Merge pull request #3658 from k8s-infra-cherrypick-robot/cherry-pick-…

…3639-to-release-1.9 [release-1.9] Fix managed clusters and agent pools diffs
kubernetes-sigs · Jun 26, 2023 · b21ee6f · b21ee6f
2 parents fbf5ae2 + 464da0d
commit b21ee6f
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 7 deletions.
diff --git a/azure/services/agentpools/spec.go b/azure/services/agentpools/spec.go
@@ -237,6 +237,14 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing interface{}) (p
 			normalizedProfile.Count = existingProfile.Count
 		}
 
+		// We do a just-in-time merge of existent kubernetes.azure.com-prefixed labels
+		// So that we don't unintentionally delete them
+		// See https://github.com/Azure/AKS/issues/3152
+		if normalizedProfile.NodeLabels != nil {
+			nodeLabels = mergeSystemNodeLabels(normalizedProfile.NodeLabels, existingPool.NodeLabels)
+			normalizedProfile.NodeLabels = nodeLabels
+		}
+
 		// Compute a diff to check if we require an update
 		diff := cmp.Diff(normalizedProfile, existingProfile)
 		if diff == "" {
@@ -245,12 +253,6 @@ func (s *AgentPoolSpec) Parameters(ctx context.Context, existing interface{}) (p
 			return nil, nil
 		}
 		log.V(4).Info("found a diff between the desired spec and the existing agentpool", "difference", diff)
-		// We do a just-in-time merge of existent kubernetes.azure.com-prefixed labels
-		// So that we don't unintentionally delete them
-		// See https://github.com/Azure/AKS/issues/3152
-		if normalizedProfile.NodeLabels != nil {
-			nodeLabels = mergeSystemNodeLabels(normalizedProfile.NodeLabels, existingPool.NodeLabels)
-		}
 	}
 
 	var availabilityZones *[]string

diff --git a/azure/services/agentpools/spec_test.go b/azure/services/agentpools/spec_test.go
@@ -260,6 +260,27 @@ func TestParameters(t *testing.T) {
 			expected:      sdkFakeAgentPool(),
 			expectedError: nil,
 		},
+		{
+			name: "difference in system node labels shouldn't trigger update",
+			spec: fakeAgentPool(
+				func(pool *AgentPoolSpec) {
+					pool.NodeLabels = map[string]*string{
+						"fake-label": pointer.String("fake-value"),
+					}
+				},
+			),
+			existing: sdkFakeAgentPool(
+				func(pool *containerservice.AgentPool) {
+					pool.NodeLabels = map[string]*string{
+						"fake-label":                            pointer.String("fake-value"),
+						"kubernetes.azure.com/scalesetpriority": pointer.String("spot"),
+					}
+				},
+				sdkWithProvisioningState("Succeeded"),
+			),
+			expected:      nil,
+			expectedError: nil,
+		},
 		{
 			name: "parameters with an existing agent pool and update needed on node taints",
 			spec: fakeAgentPool(),

diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go
@@ -382,11 +382,14 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{
 
 	if s.APIServerAccessProfile != nil {
 		managedCluster.APIServerAccessProfile = &containerservice.ManagedClusterAPIServerAccessProfile{
-			AuthorizedIPRanges:             &s.APIServerAccessProfile.AuthorizedIPRanges,
 			EnablePrivateCluster:           s.APIServerAccessProfile.EnablePrivateCluster,
 			PrivateDNSZone:                 s.APIServerAccessProfile.PrivateDNSZone,
 			EnablePrivateClusterPublicFQDN: s.APIServerAccessProfile.EnablePrivateClusterPublicFQDN,
 		}
+
+		if len(s.APIServerAccessProfile.AuthorizedIPRanges) > 0 {
+			managedCluster.APIServerAccessProfile.AuthorizedIPRanges = &s.APIServerAccessProfile.AuthorizedIPRanges
+		}
 	}
 
 	if s.OutboundType != nil {

diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go
@@ -154,6 +154,29 @@ func TestParameters(t *testing.T) {
 				g.Expect(result).To(BeNil())
 			},
 		},
+		{
+			name:     "no update needed if both clusters have no authorized IP ranges",
+			existing: getExistingClusterWithAPIServerAccessProfile(),
+			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",
+				APIServerAccessProfile: &APIServerAccessProfile{
+					AuthorizedIPRanges: func() []string {
+						var arr []string
+						return arr
+					}(),
+				},
+			},
+			expect: func(g *WithT, result interface{}) {
+				g.Expect(result).To(BeNil())
+			},
+		},
 	}
 	for _, tc := range testcases {
 		tc := tc
@@ -173,6 +196,14 @@ func TestParameters(t *testing.T) {
 	}
 }
 
+func getExistingClusterWithAPIServerAccessProfile() containerservice.ManagedCluster {
+	mc := getExistingCluster()
+	mc.APIServerAccessProfile = &containerservice.ManagedClusterAPIServerAccessProfile{
+		EnablePrivateCluster: pointer.Bool(false),
+	}
+	return mc
+}
+
 func getExistingCluster() containerservice.ManagedCluster {
 	mc := getSampleManagedCluster()
 	mc.ProvisioningState = pointer.String("Succeeded")