fix for add-on profile upgrade

kubernetes-sigs · Oct 25, 2023 · 361be05 · 361be05
1 parent 6987809
commit 361be05
Show file tree

Hide file tree

Showing 3 changed files with 143 additions and 0 deletions.
diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook.go b/api/v1beta1/azuremanagedcontrolplane_webhook.go
@@ -244,6 +244,10 @@ func (mw *azureManagedControlPlaneWebhook) ValidateUpdate(ctx context.Context, o
 		allErrs = append(allErrs, errs...)
 	}
 
+	if errs := m.validateAddonProfilesUpdate(old); len(errs) > 0 {
+		allErrs = append(allErrs, errs...)
+	}
+
 	if errs := m.validateAPIServerAccessProfileUpdate(old); len(errs) > 0 {
 		allErrs = append(allErrs, errs...)
 	}
@@ -523,6 +527,26 @@ func (m *AzureManagedControlPlane) validateAPIServerAccessProfileUpdate(old *Azu
 	return allErrs
 }
 
+// validateAddonProfilesUpdate validates update to AddonProfiles.
+func (m *AzureManagedControlPlane) validateAddonProfilesUpdate(old *AzureManagedControlPlane) field.ErrorList {
+	var allErrs field.ErrorList
+	newAddonProfileMap := make(map[string]bool)
+	if len(old.Spec.AddonProfiles) != 0 {
+		for _, addonProfile := range m.Spec.AddonProfiles {
+			newAddonProfileMap[addonProfile.Name] = true
+		}
+		for i, addonProfile := range old.Spec.AddonProfiles {
+			if _, ok := newAddonProfileMap[addonProfile.Name]; !ok {
+				allErrs = append(allErrs, field.Invalid(
+					field.NewPath("Spec", "AddonProfiles"),
+					m.Spec.AddonProfiles,
+					fmt.Sprintf("cannot remove addonProfile %s, to disable an addon profile, update spec.AddonProfiles[%v].Enabled to false", addonProfile.Name, i)))
+			}
+		}
+	}
+	return allErrs
+}
+
 // validateVirtualNetworkUpdate validates update to VirtualNetwork.
 func (m *AzureManagedControlPlane) validateVirtualNetworkUpdate(old *AzureManagedControlPlane) field.ErrorList {
 	var allErrs field.ErrorList

diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go
@@ -1036,6 +1036,102 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) {
 			amcp:    createAzureManagedControlPlane("192.168.0.10", "1.999.9", generateSSHPublicKey(true)),
 			wantErr: true,
 		},
+		{
+			name: "AzureManagedControlPlane AddonProfiles is mutable",
+			oldAMCP: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+					AddonProfiles: []AddonProfile{
+						{
+							Name:    "first-addon-profile",
+							Enabled: true,
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "AzureManagedControlPlane AddonProfiles can be disabled",
+			oldAMCP: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					AddonProfiles: []AddonProfile{
+						{
+							Name:    "first-addon-profile",
+							Enabled: true,
+						},
+					},
+					Version: "v1.18.0",
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+					AddonProfiles: []AddonProfile{
+						{
+							Name:    "first-addon-profile",
+							Enabled: false,
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "AzureManagedControlPlane AddonProfiles cannot update to empty array",
+			oldAMCP: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					AddonProfiles: []AddonProfile{
+						{
+							Name:    "first-addon-profile",
+							Enabled: true,
+						},
+					},
+					Version: "v1.18.0",
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "AzureManagedControlPlane AddonProfiles cannot be completely removed",
+			oldAMCP: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					AddonProfiles: []AddonProfile{
+						{
+							Name:    "first-addon-profile",
+							Enabled: true,
+						},
+						{
+							Name:    "second-addon-profile",
+							Enabled: true,
+						},
+					},
+					Version: "v1.18.0",
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					AddonProfiles: []AddonProfile{
+						{
+							Name:    "first-addon-profile",
+							Enabled: true,
+						},
+					},
+					Version: "v1.18.0",
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "AzureManagedControlPlane SubscriptionID is immutable",
 			oldAMCP: &AzureManagedControlPlane{

diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go
@@ -484,6 +484,21 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{
 			return nil, azure.WithTransientError(errors.Errorf("Unable to update existing managed cluster in non-terminal state. Managed cluster must be in one of the following provisioning states: Canceled, Failed, or Succeeded. Actual state: %s", ps), 20*time.Second)
 		}
 
+		if managedCluster.Properties.AddonProfiles == nil && existingMC.Properties.AddonProfiles != nil {
+			managedCluster.Properties.AddonProfiles = map[string]*armcontainerservice.ManagedClusterAddonProfile{}
+		}
+		for key, item := range existingMC.Properties.AddonProfiles {
+			if _, ok := managedCluster.Properties.AddonProfiles[key]; !ok {
+				addonProfile := &armcontainerservice.ManagedClusterAddonProfile{
+					Enabled: ptr.To(false),
+				}
+				if item.Config != nil {
+					addonProfile.Config = item.Config
+				}
+				managedCluster.Properties.AddonProfiles[key] = addonProfile
+			}
+			existingMC.Properties.AddonProfiles[key].Identity = nil
+		}
 		// Normalize the LoadBalancerProfile so the diff below doesn't get thrown off by AKS added properties.
 		if managedCluster.Properties.NetworkProfile.LoadBalancerProfile == nil {
 			// If our LoadBalancerProfile generated by the spec is nil, then don't worry about what AKS has added.
@@ -631,6 +646,14 @@ func computeDiffOfNormalizedClusters(managedCluster armcontainerservice.ManagedC
 		}
 	}
 
+	if managedCluster.Properties.AddonProfiles != nil {
+		propertiesNormalized.AddonProfiles = managedCluster.Properties.AddonProfiles
+	}
+
+	if existingMC.Properties.AddonProfiles != nil {
+		existingMCPropertiesNormalized.AddonProfiles = existingMC.Properties.AddonProfiles
+	}
+
 	if managedCluster.Properties.AutoScalerProfile != nil {
 		propertiesNormalized.AutoScalerProfile = &armcontainerservice.ManagedClusterPropertiesAutoScalerProfile{
 			BalanceSimilarNodeGroups:      managedCluster.Properties.AutoScalerProfile.BalanceSimilarNodeGroups,