Skip to content

Commit

Permalink
fix for add-on profile upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
@LochanRn
LochanRn committed Oct 25, 2023
1 parent 6987809 commit 005e575
Show file tree
Hide file tree
Showing 4 changed files with 244 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 := 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
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{}
}

Check warning on line 489 in azure/services/managedclusters/spec.go

View check run for this annotation

Codecov / codecov/patch

azure/services/managedclusters/spec.go#L488-L489 

Added lines #L488 - L489 were not covered by tests
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
}

Check warning on line 497 in azure/services/managedclusters/spec.go

View check run for this annotation

Codecov / codecov/patch

azure/services/managedclusters/spec.go#L496-L497 

Added lines #L496 - L497 were not covered by tests
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
101 changes: 101 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,92 @@ 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",
AddonProfiles: []AddonProfile{
{
Name: "first-addon-profile",
Enabled: true,
},
{
Name: "second-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(BeTrue())
},
},
{
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",
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",
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 +655,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 005e575

Please sign in to comment.