Skip to content

Commit

Permalink
Updated the check for updating machinepool when using autoscalar
Browse files Browse the repository at this point in the history
  • Loading branch information
LochanRn committed Jul 27, 2022
1 parent a9ad6fb commit 75fa84f
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 deletions.
2 changes: 2 additions & 0 deletions api/v1beta1/conditions_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ const (
ManagedClusterRunningCondition clusterv1.ConditionType = "ManagedClusterRunning"
// AgentPoolsReadyCondition means the AKS agent pools exist and are ready to be used.
AgentPoolsReadyCondition clusterv1.ConditionType = "AgentPoolsReady"
// AutoScalerUpdatedMachinePoolsCondition means the Azure autoscaler scaledup or scaledDown the machinepools.
AutoScalerUpdatedMachinePoolReplicas clusterv1.ConditionType = "AutoScalerUpdatedMachinePoolReplicas"
)

// Azure Services Conditions and Reasons.
Expand Down
14 changes: 12 additions & 2 deletions azure/scope/managedmachinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,16 @@ func (s *ManagedMachinePoolScope) DeleteLongRunningOperationState(name, service
futures.Delete(s.ControlPlane, name, service)
}

// UpdateMachinePoolReplicas updates capi machinepool replica count.
func (s *ManagedMachinePoolScope) UpdateMachinePoolReplicas(replicas *int32) error {
patch := client.MergeFrom(s.MachinePool.DeepCopy())
s.MachinePool.Spec.Replicas = replicas
if err := s.Client.Patch(context.Background(), s.MachinePool, patch); err != nil {
return errors.Wrap(err, "failed to patch Machinepool")
}
return nil
}

// UpdateDeleteStatus updates a condition on the AzureManagedControlPlane status after a DELETE operation.
func (s *ManagedMachinePoolScope) UpdateDeleteStatus(condition clusterv1.ConditionType, service string, err error) {
switch {
Expand All @@ -231,7 +241,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:
Expand All @@ -243,7 +253,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:
Expand Down
21 changes: 19 additions & 2 deletions azure/services/agentpools/agentpools.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ 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"
infrav1beta1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
"sigs.k8s.io/cluster-api-provider-azure/util/maps"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

const serviceName = "agentpools"
Expand All @@ -43,6 +45,8 @@ type ManagedMachinePoolScope interface {
SetAgentPoolProviderIDList([]string)
SetAgentPoolReplicas(int32)
SetAgentPoolReady(bool)
UpdatePutStatus(condition clusterv1.ConditionType, service string, err error)
UpdateMachinePoolReplicas(replicas *int32) error
}

// Service provides operations on Azure resources.
Expand Down Expand Up @@ -96,7 +100,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
}
} else {
ps := *existingPool.ManagedClusterAgentPoolProfileProperties.ProvisioningState
if ps != string(infrav1alpha4.Canceled) && ps != string(infrav1alpha4.Failed) && ps != string(infrav1alpha4.Succeeded) {
if ps != string(infrav1beta1.Canceled) && ps != string(infrav1beta1.Failed) && ps != string(infrav1beta1.Succeeded) {
msg := fmt.Sprintf("Unable to update existing agent pool in non terminal state. Agent pool must be in one of the following provisioning states: canceled, failed, or succeeded. Actual state: %s", ps)
log.V(2).Info(msg)
return azure.WithTransientError(errors.New(msg), 20*time.Second)
Expand Down Expand Up @@ -125,6 +129,19 @@ 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 to.Int32(existingProfile.Count) != to.Int32(normalizedProfile.Count) {
if err := s.scope.UpdateMachinePoolReplicas(existingProfile.Count); err != nil {
return errors.Wrap(err, "unable to update machine pool replica to match autoscaler replica count")
}
s.scope.UpdatePutStatus(infrav1beta1.AutoScalerUpdatedMachinePoolReplicas, "", nil)
}
normalizedProfile.Count = existingProfile.Count
}

// Diff and check if we require an update
diff := cmp.Diff(normalizedProfile, existingProfile)
if diff != "" {
Expand Down

0 comments on commit 75fa84f

Please sign in to comment.