diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 5cdf1dcca56..5b1c3b2d8df 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -414,6 +414,7 @@ func (s *ManagedControlPlaneScope) ManagedClusterAnnotations() map[string]string return s.ControlPlane.Annotations } +// IsLocalAcountsDisabled checks if local accounts have been disabled. func (s *ManagedControlPlaneScope) IsLocalAcountsDisabled() bool { if s.IsAadEnabled() && s.ControlPlane.Spec.DisableLocalAccounts != nil && @@ -423,6 +424,7 @@ func (s *ManagedControlPlaneScope) IsLocalAcountsDisabled() bool { return false } +// IsAadEnabled checks if aad is enabled. func (s *ManagedControlPlaneScope) IsAadEnabled() bool { if s.ControlPlane.Spec.AADProfile != nil && s.ControlPlane.Spec.AADProfile.Managed { return true @@ -430,6 +432,46 @@ func (s *ManagedControlPlaneScope) IsAadEnabled() bool { return false } +// SetAutoUpgradeVersionStatus sets the auto upgrade version in status +func (s *ManagedControlPlaneScope) SetAutoUpgradeVersionStatus(version string) { + s.ControlPlane.Status.AutoUpgradeVersion = version +} + +// IsManagedVersionUpgrade checks if auto upgrade profile is set and if the upgradeChannel is of the type patch, stable or rapid. +func (s *ManagedControlPlaneScope) IsManagedVersionUpgrade() bool { + return s.IsPatchAutoUpgrade() || s.IsStableAutoUpgrade() || s.IsRapidAutoUpgrade() +} + +// IsAutoUpgradeStable checks if auto upgrade channel is stable. +func (s *ManagedControlPlaneScope) IsStableAutoUpgrade() bool { + if s.ControlPlane.Spec.AutoUpgradeProfile != nil { + if upgradeChannel := s.ControlPlane.Spec.AutoUpgradeProfile.UpgradeChannel; upgradeChannel == infrav1exp.UpgradeChannelStable { + return true + } + } + return false +} + +// IsAutoUpgradeStable checks if auto upgrade channel is rapid. +func (s *ManagedControlPlaneScope) IsRapidAutoUpgrade() bool { + if s.ControlPlane.Spec.AutoUpgradeProfile != nil { + if upgradeChannel := s.ControlPlane.Spec.AutoUpgradeProfile.UpgradeChannel; upgradeChannel == infrav1exp.UpgradeChannelRapid { + return true + } + } + return false +} + +// IsAutoUpgradeStable checks if auto upgrade channel is patch. +func (s *ManagedControlPlaneScope) IsPatchAutoUpgrade() bool { + if s.ControlPlane.Spec.AutoUpgradeProfile != nil { + if upgradeChannel := s.ControlPlane.Spec.AutoUpgradeProfile.UpgradeChannel; upgradeChannel == infrav1exp.UpgradeChannelPatch { + return true + } + } + return false +} + // ManagedClusterSpec returns the managed cluster spec. func (s *ManagedControlPlaneScope) ManagedClusterSpec(ctx context.Context) azure.ResourceSpecGetter { managedClusterSpec := managedclusters.ManagedClusterSpec{ diff --git a/azure/scope/managedcontrolplane_test.go b/azure/scope/managedcontrolplane_test.go index 969401d2c23..acb384f98a1 100644 --- a/azure/scope/managedcontrolplane_test.go +++ b/azure/scope/managedcontrolplane_test.go @@ -792,3 +792,218 @@ func TestIsLocalAcountsDisabled(t *testing.T) { }) } } + +func Test_IsManagedVersionUpgrade(t *testing.T) { + scheme := runtime.NewScheme() + _ = capiv1exp.AddToScheme(scheme) + _ = infrav1.AddToScheme(scheme) + + cases := []struct { + Name string + Input ManagedControlPlaneScopeParams + Expected bool + }{ + { + Name: "Upgrade channel not set", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: false, + }, + { + Name: "Upgradechannel is set rapid", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + AutoUpgradeProfile: &infrav1.ManagedClusterAutoUpgradeProfile{ + UpgradeChannel: infrav1.UpgradeChannelRapid, + }, + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: true, + }, + { + Name: "Upgradechannel is set patch", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + AutoUpgradeProfile: &infrav1.ManagedClusterAutoUpgradeProfile{ + UpgradeChannel: infrav1.UpgradeChannelPatch, + }, + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: true, + }, + { + Name: "Upgradechannel is set stable", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + AutoUpgradeProfile: &infrav1.ManagedClusterAutoUpgradeProfile{ + UpgradeChannel: infrav1.UpgradeChannelStable, + }, + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: true, + }, + { + Name: "Upgradechannel is set none", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + AutoUpgradeProfile: &infrav1.ManagedClusterAutoUpgradeProfile{ + UpgradeChannel: infrav1.UpgradeChannelNone, + }, + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: false, + }, + { + Name: "Upgradechannel is set node-image", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + AutoUpgradeProfile: &infrav1.ManagedClusterAutoUpgradeProfile{ + UpgradeChannel: infrav1.UpgradeChannelNodeImage, + }, + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: false, + }, + } + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(c.Input.ControlPlane).Build() + c.Input.Client = fakeClient + s, err := NewManagedControlPlaneScope(context.TODO(), c.Input) + g.Expect(err).To(Succeed()) + autoUpgrade := s.IsManagedVersionUpgrade() + g.Expect(autoUpgrade).To(Equal(c.Expected)) + }) + } +} diff --git a/azure/services/managedclusters/managedclusters.go b/azure/services/managedclusters/managedclusters.go index 6978968412f..4902558a5e2 100644 --- a/azure/services/managedclusters/managedclusters.go +++ b/azure/services/managedclusters/managedclusters.go @@ -18,6 +18,7 @@ package managedclusters import ( "context" + "fmt" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-05-01/containerservice" "github.com/Azure/go-autorest/autorest/to" @@ -51,6 +52,8 @@ type ManagedClusterScope interface { SetUserKubeConfigData([]byte) IsAadEnabled() bool IsLocalAcountsDisabled() bool + SetAutoUpgradeVersionStatus(version string) + IsManagedVersionUpgrade() bool } // Service provides operations on azure resources. @@ -108,6 +111,11 @@ func (s *Service) Reconcile(ctx context.Context) error { return errors.Wrap(err, "error while reconciling adminKubeConfigData") } + if s.Scope.IsManagedVersionUpgrade() && managedCluster.KubernetesVersion != nil { + kubernetesVersion := fmt.Sprintf("v%s", *managedCluster.KubernetesVersion) + s.Scope.SetAutoUpgradeVersionStatus(kubernetesVersion) + } + s.Scope.SetAdminKubeConfigData(adminKubeConfigData) s.Scope.SetUserKubeConfigData(userKubeConfigData) } diff --git a/azure/services/managedclusters/managedclusters_test.go b/azure/services/managedclusters/managedclusters_test.go index 6a185a626ef..549e380902f 100644 --- a/azure/services/managedclusters/managedclusters_test.go +++ b/azure/services/managedclusters/managedclusters_test.go @@ -85,6 +85,34 @@ func TestReconcile(t *testing.T) { m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return([]byte("credentials"), nil) s.SetAdminKubeConfigData([]byte("credentials")) s.SetUserKubeConfigData(userKubeConfigData) + s.IsManagedVersionUpgrade().Return(false) + s.UpdatePutStatus(infrav1.ManagedClusterRunningCondition, serviceName, nil) + }, + }, + { + name: "create managed cluster succeeds, update autoupgrade status", + expectedError: "", + expect: func(m *mock_managedclusters.MockCredentialGetterMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + var userKubeConfigData []byte + s.ManagedClusterSpec(gomockinternal.AContext()).Return(fakeManagedClusterSpec) + r.CreateResource(gomockinternal.AContext(), fakeManagedClusterSpec, serviceName).Return(containerservice.ManagedCluster{ + ManagedClusterProperties: &containerservice.ManagedClusterProperties{ + Fqdn: pointer.String("my-managedcluster-fqdn"), + ProvisioningState: pointer.String("Succeeded"), + KubernetesVersion: pointer.String("1.27.3"), + }, + }, nil) + s.SetControlPlaneEndpoint(clusterv1.APIEndpoint{ + Host: "my-managedcluster-fqdn", + Port: 443, + }) + s.IsAadEnabled().Return(false) + s.IsLocalAcountsDisabled().Return(false) + m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return([]byte("credentials"), nil) + s.SetAdminKubeConfigData([]byte("credentials")) + s.SetUserKubeConfigData(userKubeConfigData) + s.IsManagedVersionUpgrade().Return(true) + s.SetAutoUpgradeVersionStatus("v1.27.3") s.UpdatePutStatus(infrav1.ManagedClusterRunningCondition, serviceName, nil) }, }, @@ -113,6 +141,7 @@ func TestReconcile(t *testing.T) { m.GetUserCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return([]byte("credentials-user"), nil) s.SetAdminKubeConfigData([]byte("credentials")) s.SetUserKubeConfigData([]byte("credentials-user")) + s.IsManagedVersionUpgrade().Return(false) s.UpdatePutStatus(infrav1.ManagedClusterRunningCondition, serviceName, nil) }, }, diff --git a/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go b/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go index 0d0126f8866..1badc3806c1 100644 --- a/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go +++ b/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go @@ -221,6 +221,20 @@ func (mr *MockManagedClusterScopeMockRecorder) IsLocalAcountsDisabled() *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsLocalAcountsDisabled", reflect.TypeOf((*MockManagedClusterScope)(nil).IsLocalAcountsDisabled)) } +// IsManagedVersionUpgrade mocks base method. +func (m *MockManagedClusterScope) IsManagedVersionUpgrade() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsManagedVersionUpgrade") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsManagedVersionUpgrade indicates an expected call of IsManagedVersionUpgrade. +func (mr *MockManagedClusterScopeMockRecorder) IsManagedVersionUpgrade() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsManagedVersionUpgrade", reflect.TypeOf((*MockManagedClusterScope)(nil).IsManagedVersionUpgrade)) +} + // MakeEmptyKubeConfigSecret mocks base method. func (m *MockManagedClusterScope) MakeEmptyKubeConfigSecret() v1.Secret { m.ctrl.T.Helper() @@ -261,6 +275,18 @@ func (mr *MockManagedClusterScopeMockRecorder) SetAdminKubeConfigData(arg0 inter return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAdminKubeConfigData", reflect.TypeOf((*MockManagedClusterScope)(nil).SetAdminKubeConfigData), arg0) } +// SetAutoUpgradeVersionStatus mocks base method. +func (m *MockManagedClusterScope) SetAutoUpgradeVersionStatus(version string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetAutoUpgradeVersionStatus", version) +} + +// SetAutoUpgradeVersionStatus indicates an expected call of SetAutoUpgradeVersionStatus. +func (mr *MockManagedClusterScopeMockRecorder) SetAutoUpgradeVersionStatus(version interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAutoUpgradeVersionStatus", reflect.TypeOf((*MockManagedClusterScope)(nil).SetAutoUpgradeVersionStatus), version) +} + // SetControlPlaneEndpoint mocks base method. func (m *MockManagedClusterScope) SetControlPlaneEndpoint(arg0 v1beta10.APIEndpoint) { m.ctrl.T.Helper() diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index 1e0e8f1255d..f8b140b2523 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -26,10 +26,12 @@ import ( "github.com/Azure/go-autorest/autorest/to" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" + "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/converters" expinfrav1 "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/util/versions" ) // ManagedClusterSpec contains properties to create a managed cluster. @@ -215,6 +217,28 @@ func (s *ManagedClusterSpec) CustomHeaders() map[string]string { return s.Headers } +// GetManagedClusterVersion gets the desired managed k8s version. +// If autoupgrade channels is set to patch, stable or rapid, clusters can be upgraded to higher version by AKS. +// If autoupgrade is triggered, existing kubernetes version will be higher than the user desired kubernetes version. +// CAPZ should honour the upgrade and it should not downgrade to the lower desired version. +func (s *ManagedClusterSpec) GetManagedClusterVersion(existing interface{}) (string, error) { + version := s.Version + if existing != nil && version != "" { + existingMC, ok := existing.(containerservice.ManagedCluster) + if !ok { + return version, fmt.Errorf("%T is not a containerservice.ManagedCluster", existing) + } + if v, err := versions.GetHigherK8sVersion( + version, + ptr.Deref(existingMC.KubernetesVersion, version)); err != nil { + return "", err + } else { + version = v + } + } + return version, nil +} + // Parameters returns the parameters for the managed clusters. func (s *ManagedClusterSpec) Parameters(existing interface{}) (params interface{}, err error) { decodedSSHPublicKey, err := base64.StdEncoding.DecodeString(s.SSHPublicKey) @@ -230,7 +254,6 @@ func (s *ManagedClusterSpec) Parameters(existing interface{}) (params interface{ NodeResourceGroup: &s.NodeResourceGroup, EnableRBAC: to.BoolPtr(true), DNSPrefix: s.DNSPrefix, - KubernetesVersion: &s.Version, LinuxProfile: &containerservice.LinuxProfile{ AdminUsername: to.StringPtr(azure.DefaultAKSUserName), SSH: &containerservice.SSHConfiguration{ @@ -253,6 +276,12 @@ func (s *ManagedClusterSpec) Parameters(existing interface{}) (params interface{ }, } + if kubernetesVersion, err := s.GetManagedClusterVersion(existing); err != nil { + return nil, err + } else { + managedCluster.KubernetesVersion = &kubernetesVersion + } + if s.FqdnSubdomain != nil { managedCluster.FqdnSubdomain = s.FqdnSubdomain } @@ -561,6 +590,8 @@ func computeDiffOfNormalizedClusters(managedCluster containerservice.ManagedClus clusterNormalized.AutoUpgradeProfile = &containerservice.ManagedClusterAutoUpgradeProfile{ UpgradeChannel: managedCluster.AutoUpgradeProfile.UpgradeChannel, } + } else { + clusterNormalized.AutoUpgradeProfile = &containerservice.ManagedClusterAutoUpgradeProfile{} } if existingMC.AutoUpgradeProfile != nil { diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go index e6256e900a0..0347d9f4107 100644 --- a/azure/services/managedclusters/spec_test.go +++ b/azure/services/managedclusters/spec_test.go @@ -110,6 +110,23 @@ func TestParameters(t *testing.T) { g.Expect(result).To(BeNil()) }, }, + { + name: "managedcluster exists, no update needed", + existing: getExistingCluster(), + spec: &ManagedClusterSpec{ + Name: "test-managedcluster", + ResourceGroup: "test-rg", + Location: "test-location", + Tags: map[string]string{ + "test-tag": "test-value", + }, + Version: "v1.21.0", + LoadBalancerSKU: "Standard", + }, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + }, { name: "managedcluster exists and an update is needed", existing: getExistingCluster(), @@ -158,7 +175,6 @@ func getSampleManagedCluster() containerservice.ManagedCluster { return containerservice.ManagedCluster{ ManagedClusterProperties: &containerservice.ManagedClusterProperties{ KubernetesVersion: to.StringPtr("v1.22.0"), - DNSPrefix: to.StringPtr("test-managedcluster"), AgentPoolProfiles: &[]containerservice.ManagedClusterAgentPoolProfile{ converters.AgentPoolToManagedClusterAgentPoolProfile(azure.AgentPoolSpec{ Name: "test-agentpool-0", diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml index 79b598017e1..5ab7c8e8ac0 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml @@ -820,6 +820,11 @@ spec: description: AzureManagedControlPlaneStatus defines the observed state of AzureManagedControlPlane. properties: + autoUpgradeVersion: + description: AutoUpgradeVersion is the Kubernetes version populated + after autoupgrade based on the upgrade channel. + minLength: 2 + type: string conditions: description: Conditions defines current service state of the AzureManagedControlPlane. items: diff --git a/exp/api/v1alpha3/zz_generated.conversion.go b/exp/api/v1alpha3/zz_generated.conversion.go index fe1ecea044b..f2af706016e 100644 --- a/exp/api/v1alpha3/zz_generated.conversion.go +++ b/exp/api/v1alpha3/zz_generated.conversion.go @@ -789,6 +789,7 @@ func Convert_v1alpha3_AzureManagedControlPlaneStatus_To_v1beta1_AzureManagedCont } func autoConvert_v1beta1_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(in *v1beta1.AzureManagedControlPlaneStatus, out *AzureManagedControlPlaneStatus, s conversion.Scope) error { + // WARNING: in.AutoUpgradeVersion requires manual conversion: does not exist in peer-type out.Ready = in.Ready out.Initialized = in.Initialized // WARNING: in.Conditions requires manual conversion: does not exist in peer-type diff --git a/exp/api/v1alpha4/zz_generated.conversion.go b/exp/api/v1alpha4/zz_generated.conversion.go index 7c316cfa13d..3ed27aae260 100644 --- a/exp/api/v1alpha4/zz_generated.conversion.go +++ b/exp/api/v1alpha4/zz_generated.conversion.go @@ -1093,6 +1093,7 @@ func Convert_v1alpha4_AzureManagedControlPlaneStatus_To_v1beta1_AzureManagedCont } func autoConvert_v1beta1_AzureManagedControlPlaneStatus_To_v1alpha4_AzureManagedControlPlaneStatus(in *v1beta1.AzureManagedControlPlaneStatus, out *AzureManagedControlPlaneStatus, s conversion.Scope) error { + // WARNING: in.AutoUpgradeVersion requires manual conversion: does not exist in peer-type out.Ready = in.Ready out.Initialized = in.Initialized // WARNING: in.Conditions requires manual conversion: does not exist in peer-type diff --git a/exp/api/v1beta1/azuremanagedcontrolplane_types.go b/exp/api/v1beta1/azuremanagedcontrolplane_types.go index f8396523776..4f8afccfb33 100644 --- a/exp/api/v1beta1/azuremanagedcontrolplane_types.go +++ b/exp/api/v1beta1/azuremanagedcontrolplane_types.go @@ -283,6 +283,12 @@ type ManagedControlPlaneSubnet struct { // AzureManagedControlPlaneStatus defines the observed state of AzureManagedControlPlane. type AzureManagedControlPlaneStatus struct { + + // AutoUpgradeVersion is the Kubernetes version populated after autoupgrade based on the upgrade channel. + // +kubebuilder:validation:MinLength:=2 + // +optional + AutoUpgradeVersion string `json:"autoUpgradeVersion,omitempty"` + // Ready is true when the provider resource is ready. // +optional Ready bool `json:"ready,omitempty"` diff --git a/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go b/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go index fd79f3d86b0..434b476d520 100644 --- a/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go +++ b/exp/api/v1beta1/azuremanagedcontrolplane_webhook.go @@ -30,8 +30,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/util/versions" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -242,8 +243,7 @@ func (m *AzureManagedControlPlane) ValidateUpdate(oldRaw runtime.Object, client } } - if old.Spec.DisableLocalAccounts == nil && - m.Spec.DisableLocalAccounts != nil && + if m.Spec.DisableLocalAccounts != nil && m.Spec.AADProfile == nil { allErrs = append(allErrs, field.Invalid( @@ -252,6 +252,14 @@ func (m *AzureManagedControlPlane) ValidateUpdate(oldRaw runtime.Object, client "DisableLocalAccounts can be set only for AAD enabled clusters")) } + if old.Spec.DisableLocalAccounts != nil && m.Spec.DisableLocalAccounts == nil { + allErrs = append(allErrs, + field.Invalid( + field.NewPath("Spec", "DisableLocalAccounts"), + m.Spec.DisableLocalAccounts, + "field cannot be disabled")) + } + if old.Spec.OutboundType != nil { // Prevent OutboundType modification if it was already set to some value if m.Spec.OutboundType == nil { @@ -271,7 +279,7 @@ func (m *AzureManagedControlPlane) ValidateUpdate(oldRaw runtime.Object, client } } - if pointer.StringDeref(m.Spec.DNSPrefix, "") != pointer.StringDeref(old.Spec.DNSPrefix, "") { + if ptr.Deref[string](m.Spec.DNSPrefix, "") != ptr.Deref[string](old.Spec.DNSPrefix, "") { allErrs = append(allErrs, field.Invalid( field.NewPath("Spec.DNSPrefix"), @@ -279,7 +287,7 @@ func (m *AzureManagedControlPlane) ValidateUpdate(oldRaw runtime.Object, client "field is immutable")) } - if pointer.StringDeref(m.Spec.DockerBridgeCidr, "") != pointer.StringDeref(old.Spec.DockerBridgeCidr, "") { + if ptr.Deref[string](m.Spec.DockerBridgeCidr, "") != ptr.Deref[string](old.Spec.DockerBridgeCidr, "") { allErrs = append(allErrs, field.Invalid( field.NewPath("Spec.DockerBridgeCidr"), @@ -287,7 +295,7 @@ func (m *AzureManagedControlPlane) ValidateUpdate(oldRaw runtime.Object, client "field is immutable")) } - if pointer.StringDeref(m.Spec.FqdnSubdomain, "") != pointer.StringDeref(old.Spec.FqdnSubdomain, "") { + if ptr.Deref[string](m.Spec.FqdnSubdomain, "") != ptr.Deref[string](old.Spec.FqdnSubdomain, "") { allErrs = append(allErrs, field.Invalid( field.NewPath("Spec.FqdnSubdomain"), @@ -295,6 +303,20 @@ func (m *AzureManagedControlPlane) ValidateUpdate(oldRaw runtime.Object, client "field is immutable")) } + if hv, _ := versions.GetHigherK8sVersion(m.Spec.Version, old.Spec.Version); hv != m.Spec.Version { + allErrs = append(allErrs, field.Invalid(field.NewPath("Spec", "Version"), + m.Spec.Version, "fields version cannot be downgraded"), + ) + } + + if old.Status.AutoUpgradeVersion != "" && m.Spec.Version != old.Spec.Version { + if hv, _ := versions.GetHigherK8sVersion(m.Spec.Version, old.Status.AutoUpgradeVersion); hv != m.Spec.Version { + allErrs = append(allErrs, field.Invalid(field.NewPath("Spec", "Version"), + m.Spec.Version, "fields version cannot be downgraded"), + ) + } + } + if errs := m.validateAPIServerAccessProfileUpdate(old); len(errs) > 0 { allErrs = append(allErrs, errs...) } @@ -345,7 +367,6 @@ func (m *AzureManagedControlPlane) validateDisableLocalAccounts(_ client.Client) } func (m *AzureManagedControlPlane) validateDNSPrefix(_ client.Client) error { - if m.Spec.DNSPrefix == nil { return nil } @@ -355,32 +376,12 @@ func (m *AzureManagedControlPlane) validateDNSPrefix(_ client.Client) error { // 2. Alphanumerics and hyphens: [a-zA-Z0-9-] // 3. Start and end with alphanumeric: ^[a-zA-Z0-9].*[a-zA-Z0-9]$ regex := regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9-]{0,52}[a-zA-Z0-9]$`) - if regex.MatchString(pointer.StringDeref(m.Spec.DNSPrefix, "")) { + if regex.MatchString(ptr.Deref[string](m.Spec.DNSPrefix, "")) { return nil } return errors.New("DNSPrefix is invalid") } -// func (m *AzureManagedControlPlane) validateFqdnSubdomain(_ client.Client) error { - -// if m.Spec.FqdnSubdomain == nil { -// return nil -// } - -// // Regex pattern for FQDN subdomain validation -// // 1. Between 1 and 63 characters long: {1,63} -// // 2. Alphanumerics and hyphens. -// // 3. Start and end with alphanumeric. -// // 4. Parts separated by dots (.) -// pattern := `^(?i)[a-z0-9]([a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*(?:\.aks\.example)$` - -// regex := regexp.MustCompile(pattern) -// if regex.MatchString(pointer.StringDeref(m.Spec.FqdnSubdomain, "")) { -// return nil -// } -// return errors.Errorf("FqdnSubdomain is invalid %s", pointer.StringDeref(m.Spec.FqdnSubdomain, "")) -// } - // validateDNSServiceIP validates the DNSServiceIP. func (m *AzureManagedControlPlane) validateDNSServiceIP(_ client.Client) error { if m.Spec.DNSServiceIP != nil { @@ -409,7 +410,6 @@ func (m *AzureManagedControlPlane) validateVersion(_ client.Client) error { if !kubeSemver.MatchString(m.Spec.Version) { return errors.New("must be a valid semantic version") } - return nil } @@ -593,10 +593,10 @@ func (m *AzureManagedControlPlane) validateAPIServerAccessProfileDNSZoneUpdate(o // You can only update from byo or system to none. No other combination of update values is supported. if m.Spec.APIServerAccessProfile != nil && old.Spec.APIServerAccessProfile != nil && - pointer.BoolDeref(m.Spec.APIServerAccessProfile.EnablePrivateCluster, false) && - pointer.BoolDeref(old.Spec.APIServerAccessProfile.EnablePrivateCluster, false) && + ptr.Deref(m.Spec.APIServerAccessProfile.EnablePrivateCluster, false) && + ptr.Deref(old.Spec.APIServerAccessProfile.EnablePrivateCluster, false) && m.Spec.APIServerAccessProfile.PrivateDNSZone != old.Spec.APIServerAccessProfile.PrivateDNSZone && - pointer.StringDeref(m.Spec.APIServerAccessProfile.PrivateDNSZone, "") == "None" { + ptr.Deref[string](m.Spec.APIServerAccessProfile.PrivateDNSZone, "") == "None" { return nil } diff --git a/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go index 9f6343d4828..ec08aa9fa0b 100644 --- a/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go +++ b/exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go @@ -471,6 +471,117 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { amcp: createAzureManagedControlPlane("192.168.0.0", "1.999.9", generateSSHPublicKey(true)), wantErr: true, }, + { + name: "AzureManagedControlPlane invalid version downgrade change", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.0", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.17.0", + }, + }, + wantErr: true, + }, + { + name: "AzureManagedControlPlane invalid version downgrade change", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.0", + }, + Status: AzureManagedControlPlaneStatus{ + AutoUpgradeVersion: "v1.18.3", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.1", + }, + }, + wantErr: true, + }, + { + name: "AzureManagedControlPlane invalid version downgrade change", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.0", + }, + Status: AzureManagedControlPlaneStatus{ + AutoUpgradeVersion: "1.19.3", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.6", + }, + }, + wantErr: true, + }, + { + name: "AzureManagedControlPlane no version change", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.0", + }, + Status: AzureManagedControlPlaneStatus{ + AutoUpgradeVersion: "1.19.3", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.0", + }, + }, + wantErr: false, + }, + { + name: "AzureManagedControlPlane valid version upgrade change", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.0", + }, + Status: AzureManagedControlPlaneStatus{ + AutoUpgradeVersion: "1.19.3", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.19.5", + }, + }, + wantErr: false, + }, + { + name: "AzureManagedControlPlane valid version change", + oldAMCP: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.18.0", + }, + Status: AzureManagedControlPlaneStatus{ + AutoUpgradeVersion: "1.19.3", + }, + }, + amcp: &AzureManagedControlPlane{ + Spec: AzureManagedControlPlaneSpec{ + DNSServiceIP: to.StringPtr("192.168.0.0"), + Version: "v1.19.3", + }, + }, + wantErr: false, + }, { name: "AzureManagedControlPlane SubscriptionID is immutable", oldAMCP: &AzureManagedControlPlane{ @@ -1117,6 +1228,35 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { }, wantErr: true, }, + { + name: "DisableLocalAccounts cannot be disabled AAD clusters", + oldAMCP: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + Spec: AzureManagedControlPlaneSpec{ + Version: "v1.18.0", + AADProfile: &AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + DisableLocalAccounts: pointer.BoolPtr(true), + }, + }, + amcp: &AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + Spec: AzureManagedControlPlaneSpec{ + Version: "v1.18.0", + AADProfile: &AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + }, + }, + wantErr: true, + }, { name: "OutboundType update", oldAMCP: &AzureManagedControlPlane{ diff --git a/go.mod b/go.mod index d0b4c6d2e01..86dda86eed5 100644 --- a/go.mod +++ b/go.mod @@ -41,9 +41,9 @@ require ( k8s.io/apimachinery v0.25.0 k8s.io/client-go v0.25.0 k8s.io/component-base v0.25.0 - k8s.io/klog/v2 v2.70.1 + k8s.io/klog/v2 v2.80.1 k8s.io/kubectl v0.25.0 - k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed + k8s.io/utils v0.0.0-20230726121419-3b25d923346b sigs.k8s.io/cluster-api v1.1.1 sigs.k8s.io/cluster-api/test v1.1.4 sigs.k8s.io/controller-runtime v0.13.1 diff --git a/go.sum b/go.sum index 7714df6dbf0..37d6030d3a1 100644 --- a/go.sum +++ b/go.sum @@ -1919,8 +1919,8 @@ k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= k8s.io/klog/v2 v2.4.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= k8s.io/klog/v2 v2.30.0/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= -k8s.io/klog/v2 v2.70.1 h1:7aaoSdahviPmR+XkS7FyxlkkXs6tHISSG03RxleQAVQ= -k8s.io/klog/v2 v2.70.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= +k8s.io/klog/v2 v2.80.1 h1:atnLQ121W371wYYFawwYx1aEY2eUfs4l3J72wtgAwV4= +k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6/go.mod h1:UuqjUnNftUyPE5H64/qeyjQoUZhGpeFDVdxjTeEVN2o= k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd/go.mod h1:WOJ3KddDSol4tAGcJo0Tvi+dK12EcqSLqcWsryKMpfM= k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e/go.mod h1:vHXdDvt9+2spS2Rx9ql3I8tycm3H9FDfdUoIuKCefvw= @@ -1936,8 +1936,8 @@ k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/ k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= k8s.io/utils v0.0.0-20211116205334-6203023598ed/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= -k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed h1:jAne/RjBTyawwAy0utX5eqigAwz/lQhTmy+Hr/Cpue4= -k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= +k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI= +k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go v1.1.0 h1:tfWM1RT7PzUwWphqHU6ptPU3ZhwVnSw/9nEGf519rYg= oras.land/oras-go v1.1.0/go.mod h1:1A7vR/0KknT2UkJVWh+xMi95I/AhK8ZrxrnUSmXN0bQ= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/spectro/generated/core-global.yaml b/spectro/generated/core-global.yaml index d2070bba34a..18148548248 100644 --- a/spectro/generated/core-global.yaml +++ b/spectro/generated/core-global.yaml @@ -9992,6 +9992,11 @@ spec: description: AzureManagedControlPlaneStatus defines the observed state of AzureManagedControlPlane. properties: + autoUpgradeVersion: + description: AutoUpgradeVersion is the Kubernetes version populated + after autoupgrade based on the upgrade channel. + minLength: 2 + type: string conditions: description: Conditions defines current service state of the AzureManagedControlPlane. items: diff --git a/util/versions/version.go b/util/versions/version.go new file mode 100644 index 00000000000..0b83c8acec0 --- /dev/null +++ b/util/versions/version.go @@ -0,0 +1,22 @@ +package versions + +import ( + semverv4 "github.com/blang/semver" + "github.com/pkg/errors" +) + +// GetHigherK8sVersion returns the higher k8s version out of a and b +func GetHigherK8sVersion(a, b string) (string, error) { + v1, err := semverv4.ParseTolerant(a) + if err != nil { + return "", errors.Wrap(err, "error parsing k8s version") + } + v2, err := semverv4.ParseTolerant(b) + if err != nil { + return "", errors.Wrap(err, "error parsing k8s version") + } + if v1.GTE(v2) { + return a, nil + } + return b, nil +} diff --git a/util/versions/version_test.go b/util/versions/version_test.go new file mode 100644 index 00000000000..e18991176bb --- /dev/null +++ b/util/versions/version_test.go @@ -0,0 +1,86 @@ +package versions + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestGetHigherK8sVersion(t *testing.T) { + cases := []struct { + name string + a string + b string + output string + expectErr bool + }{ + { + name: "a is greater than b", + a: "v1.17.8", + b: "v1.18.8", + output: "v1.18.8", + expectErr: false, + }, + { + name: "b is greater than a", + a: "v1.18.9", + b: "v1.18.8", + output: "v1.18.9", + expectErr: false, + }, + { + name: "b is greater than a", + a: "v1.18", + b: "v1.18.8", + output: "v1.18.8", + expectErr: false, + }, + { + name: "a is equal to b", + a: "v1.18.8", + b: "v1.18.8", + output: "v1.18.8", + expectErr: false, + }, + { + name: "a is greater than b and a is major.minor", + a: "v1.18", + b: "v1.17.8", + output: "v1.18", + expectErr: false, + }, + { + name: "a is greater than b and a is major.minor", + a: "1.18", + b: "1.17.8", + output: "1.18", + expectErr: false, + }, + { + name: "a is invalid", + a: "1.18.", + b: "v1.17.8", + output: "", + expectErr: true, + }, + { + name: "b is invalid", + a: "1.18.1", + b: "v1.17.8.", + output: "", + expectErr: true, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + g := NewWithT(t) + output, err := GetHigherK8sVersion(c.a, c.b) + g.Expect(output).To(Equal(c.output)) + if c.expectErr { + g.Expect(err).NotTo(BeNil()) + } else { + g.Expect(err).To(BeNil()) + } + }) + } +}