From a8e8b31bbbe60a6ee8ee84a7078d7a0f7983d696 Mon Sep 17 00:00:00 2001 From: LochanRn Date: Sat, 2 Jul 2022 00:55:18 +0530 Subject: [PATCH] Updated the check for updating machinepool when using autoscalar --- azure/const.go | 4 ++ azure/scope/managedmachinepool.go | 58 +++++++++++++++---- azure/services/agentpools/agentpools.go | 24 ++++++++ config/rbac/role.yaml | 2 + .../azuremachinepool_controller.go | 2 +- .../azuremanagedmachinepool_controller.go | 3 + 6 files changed, 81 insertions(+), 12 deletions(-) diff --git a/azure/const.go b/azure/const.go index 9c4ac179ab2..c45b0d1d461 100644 --- a/azure/const.go +++ b/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" ) diff --git a/azure/scope/managedmachinepool.go b/azure/scope/managedmachinepool.go index 1d7b6e3fb66..5b2f32fec78 100644 --- a/azure/scope/managedmachinepool.go +++ b/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 +} diff --git a/azure/services/agentpools/agentpools.go b/azure/services/agentpools/agentpools.go index ba2a9ceed96..cf1e0158fc8 100644 --- a/azure/services/agentpools/agentpools.go +++ b/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" infrav1 "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. @@ -127,6 +132,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 != "" { diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index d952703cbb5..1983ac5bb8e 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -70,6 +70,8 @@ rules: verbs: - get - list + - patch + - update - watch - apiGroups: - cluster.x-k8s.io diff --git a/exp/controllers/azuremachinepool_controller.go b/exp/controllers/azuremachinepool_controller.go index 1fdeec1912b..76d63db194a 100644 --- a/exp/controllers/azuremachinepool_controller.go +++ b/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 diff --git a/exp/controllers/azuremanagedmachinepool_controller.go b/exp/controllers/azuremanagedmachinepool_controller.go index 7e2318677e5..32b6db1e223 100644 --- a/exp/controllers/azuremanagedmachinepool_controller.go +++ b/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