Merge pull request #2594 from k8s-infra-cherrypick-robot/cherry-pick-2444-to-release-1.4

[release-1.4] When creating AKS clusters using autoscaler enabled, do not make an update api call to agentpool service based on difference in node count

Author: k8s-ci-robot

azure/const.go

@@ -28,4 +28,8 @@ const (
 	// See https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/
 	// for annotation formatting rules.
 	RGTagsLastAppliedAnnotation = "sigs.k8s.io/cluster-api-provider-azure-last-applied-tags-rg"
+
+	// ReplicasManagedByAutoscalerAnnotation is set to true in the corresponding capi machine pool
+	// when an external autoscaler manages the node count of the associated machine pool.
+	ReplicasManagedByAutoscalerAnnotation = "cluster.x-k8s.io/replicas-managed-by-autoscaler"
 )

azure/scope/managedmachinepool.go

@@ -69,21 +69,28 @@ func NewManagedMachinePoolScope(ctx context.Context, params ManagedMachinePoolSc
 		return nil, errors.Wrap(err, "failed to init patch helper")
 	}
 
+	capiMachinePoolPatchHelper, err := patch.NewHelper(params.MachinePool, params.Client)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to init patch helper")
+	}
+
 	return &ManagedMachinePoolScope{
-		Client:               params.Client,
-		Cluster:              params.Cluster,
-		ControlPlane:         params.ControlPlane,
-		MachinePool:          params.MachinePool,
-		InfraMachinePool:     params.InfraMachinePool,
-		patchHelper:          helper,
-		ManagedClusterScoper: params.ManagedControlPlaneScope,
+		Client:                     params.Client,
+		Cluster:                    params.Cluster,
+		ControlPlane:               params.ControlPlane,
+		MachinePool:                params.MachinePool,
+		InfraMachinePool:           params.InfraMachinePool,
+		patchHelper:                helper,
+		capiMachinePoolPatchHelper: capiMachinePoolPatchHelper,
+		ManagedClusterScoper:       params.ManagedControlPlaneScope,
 	}, nil
 }
 
 // ManagedMachinePoolScope defines the basic context for an actuator to operate upon.
 type ManagedMachinePoolScope struct {
-	Client      client.Client
-	patchHelper *patch.Helper
+	Client                     client.Client
+	patchHelper                *patch.Helper
+	capiMachinePoolPatchHelper *patch.Helper
 
 	azure.ManagedClusterScoper
 	Cluster          *clusterv1.Cluster
@@ -231,7 +238,7 @@ func (s *ManagedMachinePoolScope) UpdateDeleteStatus(condition clusterv1.Conditi
 	}
 }
 
-// UpdatePutStatus updates a condition on the AzureManagedControlPlane status after a PUT operation.
+// UpdatePutStatus updates a condition on the AzureManagedMachinePool status after a PUT operation.
 func (s *ManagedMachinePoolScope) UpdatePutStatus(condition clusterv1.ConditionType, service string, err error) {
 	switch {
 	case err == nil:
@@ -243,7 +250,7 @@ func (s *ManagedMachinePoolScope) UpdatePutStatus(condition clusterv1.ConditionT
 	}
 }
 
-// UpdatePatchStatus updates a condition on the AzureManagedControlPlane status after a PATCH operation.
+// UpdatePatchStatus updates a condition on the AzureManagedMachinePool status after a PATCH operation.
 func (s *ManagedMachinePoolScope) UpdatePatchStatus(condition clusterv1.ConditionType, service string, err error) {
 	switch {
 	case err == nil:
@@ -254,3 +261,32 @@ func (s *ManagedMachinePoolScope) UpdatePatchStatus(condition clusterv1.Conditio
 		conditions.MarkFalse(s.InfraMachinePool, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to update. err: %s", service, err.Error())
 	}
 }
+
+// PatchCAPIMachinePoolObject persists the capi machinepool configuration and status.
+func (s *ManagedMachinePoolScope) PatchCAPIMachinePoolObject(ctx context.Context) error {
+	return s.capiMachinePoolPatchHelper.Patch(
+		ctx,
+		s.MachinePool,
+	)
+}
+
+// UpdateCAPIMachinePoolReplicas updates the associated MachinePool replica count.
+func (s *ManagedMachinePoolScope) UpdateCAPIMachinePoolReplicas(replicas *int32) {
+	s.MachinePool.Spec.Replicas = replicas
+}
+
+// UpdateCAPIMachinePoolAnnotations updates the associated MachinePool annotation.
+func (s *ManagedMachinePoolScope) UpdateCAPIMachinePoolAnnotations(key, value string) {
+	s.MachinePool.Annotations[key] = value
+}
+
+// RemoveCAPIMachinePoolAnnotations removes the associated MachinePool annotation.
+func (s *ManagedMachinePoolScope) RemoveCAPIMachinePoolAnnotations(key string) {
+	delete(s.MachinePool.Annotations, key)
+}
+
+// GetCAPIMachinePoolAnnotations gets the associated MachinePool annotation.
+func (s *ManagedMachinePoolScope) GetCAPIMachinePoolAnnotation(key string) (bool, string) {
+	value, ok := s.MachinePool.Annotations[key]
+	return ok, value
+}

azure/services/agentpools/agentpools.go

@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-05-01/containerservice"
+	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	infrav1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
@@ -43,6 +44,10 @@ type ManagedMachinePoolScope interface {
 	SetAgentPoolProviderIDList([]string)
 	SetAgentPoolReplicas(int32)
 	SetAgentPoolReady(bool)
+	UpdateCAPIMachinePoolReplicas(replicas *int32)
+	UpdateCAPIMachinePoolAnnotations(key, value string)
+	GetCAPIMachinePoolAnnotation(key string) (bool, string)
+	RemoveCAPIMachinePoolAnnotations(key string)
 }
 
 // Service provides operations on Azure resources.
@@ -125,6 +130,25 @@ func (s *Service) Reconcile(ctx context.Context) error {
 			},
 		}
 
+		// When autoscaling is set, the count of the nodes differ based on the autoscaler and should not depend on the
+		// count present in MachinePool or AzureManagedMachinePool, hence we should not make an update API call based
+		// on difference in count.
+		if to.Bool(profile.EnableAutoScaling) && existingProfile.Count != nil {
+			if ok, _ := s.scope.GetCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation); !ok {
+				s.scope.UpdateCAPIMachinePoolAnnotations(azure.ReplicasManagedByAutoscalerAnnotation, "true")
+			}
+
+			if to.Int32(existingProfile.Count) != to.Int32(normalizedProfile.Count) {
+				s.scope.UpdateCAPIMachinePoolReplicas(existingProfile.Count)
+			}
+			normalizedProfile.Count = existingProfile.Count
+		}
+
+		// set ReplicasManagedByAutoscalerAnnotation to false as it is disabled by the user.
+		if ok, _ := s.scope.GetCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation); !to.Bool(profile.EnableAutoScaling) && ok {
+			s.scope.RemoveCAPIMachinePoolAnnotations(azure.ReplicasManagedByAutoscalerAnnotation)
+		}
+
 		// Diff and check if we require an update
 		diff := cmp.Diff(normalizedProfile, existingProfile)
 		if diff != "" {

config/rbac/role.yaml

@@ -70,6 +70,8 @@ rules:
   verbs:
   - get
   - list
+  - patch
+  - update
   - watch
 - apiGroups:
   - cluster.x-k8s.io

exp/controllers/azuremachinepool_controller.go

@@ -149,7 +149,7 @@ func (ampr *AzureMachinePoolReconciler) SetupWithManager(ctx context.Context, mg
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinepools/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinepoolmachines,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinepoolmachines/status,verbs=get
-// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinepools;machinepools/status,verbs=get;list;watch
+// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinepools;machinepools/status,verbs=get;list;watch;update;patch
 // +kubebuilder:rbac:groups="",resources=events,verbs=get;list;watch;create;update;patch
 // +kubebuilder:rbac:groups="",resources=secrets;,verbs=get;list;watch
 // +kubebuilder:rbac:groups=core,resources=nodes,verbs=get;list;watch

exp/controllers/azuremanagedmachinepool_controller.go

@@ -225,6 +225,9 @@ func (ammpr *AzureManagedMachinePoolReconciler) Reconcile(ctx context.Context, r
 		if err := mcpScope.PatchObject(ctx); err != nil && reterr == nil {
 			reterr = err
 		}
+		if err := mcpScope.PatchCAPIMachinePoolObject(ctx); err != nil && reterr == nil {
+			reterr = err
+		}
 	}()
 
 	// Handle deleted clusters