Skip to content

Commit

Permalink
Merge pull request #4176 from spectrocloud/fix-addon-profile-upgdate
Browse files Browse the repository at this point in the history 
fix for add-on profile update for AKS clusters
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 26, 2023
2 parents 91b6545 + d71965d commit 379a040
Show file tree
Hide file tree
Showing 4 changed files with 251 additions and 0 deletions.
24 changes: 24 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
Expand Down Expand Up @@ -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 := map[string]struct{}{}
if len(old.Spec.AddonProfiles) != 0 {
for _, addonProfile := range m.Spec.AddonProfiles {
newAddonProfileMap[addonProfile.Name] = struct{}{}
}
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 this AddonProfile, 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
Expand Down
96 changes: 96 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
23 changes: 23 additions & 0 deletions azure/services/managedclusters/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
108 changes: 108 additions & 0 deletions azure/services/managedclusters/spec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,99 @@ func TestParameters(t *testing.T) {
g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.OidcIssuerProfile.Enabled).To(BeFalse())
},
},
{
name: "do not update addon profile",
existing: getExistingClusterWithAddonProfile(),
spec: &ManagedClusterSpec{
Name: "test-managedcluster",
ResourceGroup: "test-rg",
Location: "test-location",
Tags: map[string]string{
"test-tag": "test-value",
},
Version: "v1.22.0",
LoadBalancerSKU: "standard",
OIDCIssuerProfile: &OIDCIssuerProfile{
Enabled: ptr.To(true),
},
AddonProfiles: []AddonProfile{
{
Name: "first-addon-profile",
Enabled: true,
},
{
Name: "second-addon-profile",
Enabled: true,
},
},
},
expect: func(g *WithT, result interface{}) {
g.Expect(result).To(BeNil())
},
},
{
name: "update needed when addon profile enabled changes",
existing: getExistingClusterWithAddonProfile(),
spec: &ManagedClusterSpec{
Name: "test-managedcluster",
ResourceGroup: "test-rg",
Location: "test-location",
Tags: map[string]string{
"test-tag": "test-value",
},
Version: "v1.22.0",
LoadBalancerSKU: "standard",
OIDCIssuerProfile: &OIDCIssuerProfile{
Enabled: ptr.To(true),
},
AddonProfiles: []AddonProfile{
{
Name: "first-addon-profile",
Enabled: true,
},
},
},
expect: func(g *WithT, result interface{}) {
g.Expect(result).To(BeAssignableToTypeOf(armcontainerservice.ManagedCluster{}))
g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.AddonProfiles["first-addon-profile"].Enabled).To(BeTrue())
g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.AddonProfiles["second-addon-profile"].Enabled).To(BeFalse())
},
},
{
name: "update when we delete an addon profile",
existing: getExistingClusterWithAddonProfile(),
spec: &ManagedClusterSpec{
Name: "test-managedcluster",
ResourceGroup: "test-rg",
Location: "test-location",
Tags: map[string]string{
"test-tag": "test-value",
},
Version: "v1.22.0",
LoadBalancerSKU: "standard",
OIDCIssuerProfile: &OIDCIssuerProfile{
Enabled: ptr.To(true),
},
AddonProfiles: []AddonProfile{
{
Name: "first-addon-profile",
Enabled: true,
},
{
Name: "second-addon-profile",
Enabled: true,
},
{
Name: "third-addon-profile",
Enabled: true,
},
},
},
expect: func(g *WithT, result interface{}) {
g.Expect(result).To(BeAssignableToTypeOf(armcontainerservice.ManagedCluster{}))
g.Expect(*result.(armcontainerservice.ManagedCluster).Properties.AddonProfiles["third-addon-profile"].Enabled).To(BeTrue())
},
},
}
for _, tc := range testcases {
tc := tc
Expand Down Expand Up @@ -569,6 +662,21 @@ func getExistingCluster() armcontainerservice.ManagedCluster {
return mc
}

func getExistingClusterWithAddonProfile() armcontainerservice.ManagedCluster {
mc := getSampleManagedCluster()
mc.Properties.ProvisioningState = ptr.To("Succeeded")
mc.ID = ptr.To("test-id")
mc.Properties.AddonProfiles = map[string]*armcontainerservice.ManagedClusterAddonProfile{
"first-addon-profile": {
Enabled: ptr.To(true),
},
"second-addon-profile": {
Enabled: ptr.To(true),
},
}
return mc
}

func getExistingClusterWithUserAssignedIdentity() armcontainerservice.ManagedCluster {
mc := getSampleManagedCluster()
mc.Properties.ProvisioningState = ptr.To("Succeeded")
Expand Down

0 comments on commit 379a040

Please sign in to comment.