Merge pull request #4929 from nojnhuh/aso-autoscale

handle non-native autoscalers for AzureASOManagedMachinePool

exp/controllers/azureasomanagedmachinepool_controller.go

@@ -209,7 +209,7 @@ func (r *AzureASOManagedMachinePoolReconciler) reconcileNormal(ctx context.Conte
 		return ctrl.Result{Requeue: true}, nil
 	}
 
-	resources, err := mutators.ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources, mutators.SetAgentPoolDefaults(asoManagedMachinePool, machinePool))
+	resources, err := mutators.ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources, mutators.SetAgentPoolDefaults(r.Client, machinePool))
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -276,7 +276,7 @@ func (r *AzureASOManagedMachinePoolReconciler) reconcileNormal(ctx context.Conte
 	slices.Sort(providerIDs)
 	asoManagedMachinePool.Spec.ProviderIDList = providerIDs
 	asoManagedMachinePool.Status.Replicas = int32(ptr.Deref(agentPool.Status.Count, 0))
-	if machinePool.Annotations[clusterv1.ReplicasManagedByAnnotation] == infrav1exp.ReplicasManagedByAKS {
+	if _, autoscaling := machinePool.Annotations[clusterv1.ReplicasManagedByAnnotation]; autoscaling {
 		machinePool.Spec.Replicas = &asoManagedMachinePool.Status.Replicas
 	}
 

exp/mutators/azureasomanagedcontrolplane.go

@@ -25,7 +25,6 @@ import (
 	asocontainerservicev1 "github.com/Azure/azure-service-operator/v2/api/containerservice/v1api20231001"
 	asocontainerservicev1hub "github.com/Azure/azure-service-operator/v2/api/containerservice/v1api20231001/storage"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/utils/ptr"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha1"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -270,7 +269,7 @@ func agentPoolsFromManagedMachinePools(ctx context.Context, ctrlClient client.Cl
 		}
 
 		resources, err := ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources,
-			SetAgentPoolDefaults(ptr.To(asoManagedMachinePool), machinePool),
+			SetAgentPoolDefaults(ctrlClient, machinePool),
 		)
 		if err != nil {
 			return nil, err

exp/mutators/azureasomanagedmachinepool.go

@@ -27,14 +27,15 @@ import (
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 // ErrNoManagedClustersAgentPoolDefined describes an AzureASOManagedMachinePool without a ManagedClustersAgentPool.
 var ErrNoManagedClustersAgentPoolDefined = fmt.Errorf("no %s ManagedClustersAgentPools defined in AzureASOManagedMachinePool spec.resources", asocontainerservicev1.GroupVersion.Group)
 
 // SetAgentPoolDefaults propagates config from a MachinePool to an AzureASOManagedMachinePool's defined ManagedClustersAgentPool.
-func SetAgentPoolDefaults(asoManagedMachinePool *infrav1exp.AzureASOManagedMachinePool, machinePool *expv1.MachinePool) ResourcesMutator {
+func SetAgentPoolDefaults(ctrlClient client.Client, machinePool *expv1.MachinePool) ResourcesMutator {
 	return func(ctx context.Context, us []*unstructured.Unstructured) error {
 		ctx, _, done := tele.StartSpanWithLogger(ctx, "mutators.SetAgentPoolDefaults")
 		defer done()
@@ -61,7 +62,7 @@ func SetAgentPoolDefaults(asoManagedMachinePool *infrav1exp.AzureASOManagedMachi
 			return err
 		}
 
-		if err := setAgentPoolCount(ctx, machinePool, agentPoolPath, agentPool); err != nil {
+		if err := setAgentPoolCount(ctx, ctrlClient, machinePool, agentPoolPath, agentPool); err != nil {
 			return err
 		}
 
@@ -126,15 +127,28 @@ func reconcileAutoscaling(agentPool *unstructured.Unstructured, machinePool *exp
 	return nil
 }
 
-func setAgentPoolCount(ctx context.Context, machinePool *expv1.MachinePool, agentPoolPath string, agentPool *unstructured.Unstructured) error {
-	_, log, done := tele.StartSpanWithLogger(ctx, "mutators.setAgentPoolOrchestratorVersion")
+func setAgentPoolCount(ctx context.Context, ctrlClient client.Client, machinePool *expv1.MachinePool, agentPoolPath string, agentPool *unstructured.Unstructured) error {
+	_, log, done := tele.StartSpanWithLogger(ctx, "mutators.setAgentPoolCount")
 	defer done()
 
-	autoscaling := machinePool.Annotations[clusterv1.ReplicasManagedByAnnotation] == infrav1exp.ReplicasManagedByAKS
-	if machinePool.Spec.Replicas == nil || autoscaling {
+	if machinePool.Spec.Replicas == nil {
 		return nil
 	}
 
+	// When managed by any autoscaler, CAPZ should not provide any spec.count to the ManagedClustersAgentPool
+	// to prevent ASO from overwriting the autoscaler's opinion of the replica count.
+	// The MachinePool's spec.replicas is used to seed an initial value as required by AKS.
+	if _, autoscaling := machinePool.Annotations[clusterv1.ReplicasManagedByAnnotation]; autoscaling {
+		existingAgentPool := &asocontainerservicev1.ManagedClustersAgentPool{}
+		err := ctrlClient.Get(ctx, client.ObjectKey{Namespace: machinePool.GetNamespace(), Name: agentPool.GetName()}, existingAgentPool)
+		if client.IgnoreNotFound(err) != nil {
+			return err
+		}
+		if err == nil && existingAgentPool.Status.Count != nil {
+			return nil
+		}
+	}
+
 	countPath := []string{"spec", "count"}
 	capiCount := int64(*machinePool.Spec.Replicas)
 	userCount, countFound, err := unstructured.NestedInt64(agentPool.UnstructuredContent(), countPath...)

exp/mutators/azureasomanagedmachinepool_test.go

@@ -31,6 +31,8 @@ import (
 	infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 func TestSetAgentPoolDefaults(t *testing.T) {
@@ -93,7 +95,7 @@ func TestSetAgentPoolDefaults(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			g := NewGomegaWithT(t)
 
-			mutator := SetAgentPoolDefaults(test.asoManagedMachinePool, test.machinePool)
+			mutator := SetAgentPoolDefaults(nil, test.machinePool)
 			actual, err := ApplyMutators(ctx, test.asoManagedMachinePool.Spec.Resources, mutator)
 			if test.expectedErr != nil {
 				g.Expect(err).To(MatchError(test.expectedErr))
@@ -351,11 +353,12 @@ func TestSetAgentPoolCount(t *testing.T) {
 	ctx := context.Background()
 
 	tests := []struct {
-		name        string
-		machinePool *expv1.MachinePool
-		agentPool   *asocontainerservicev1.ManagedClustersAgentPool
-		expected    *asocontainerservicev1.ManagedClustersAgentPool
-		expectedErr error
+		name              string
+		machinePool       *expv1.MachinePool
+		agentPool         *asocontainerservicev1.ManagedClustersAgentPool
+		existingAgentPool *asocontainerservicev1.ManagedClustersAgentPool
+		expected          *asocontainerservicev1.ManagedClustersAgentPool
+		expectedErr       error
 	}{
 		{
 			name: "no CAPI opinion",
@@ -389,12 +392,17 @@ func TestSetAgentPoolCount(t *testing.T) {
 			},
 			agentPool: &asocontainerservicev1.ManagedClustersAgentPool{
 				Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{
+					Count: nil,
+				},
+			},
+			existingAgentPool: &asocontainerservicev1.ManagedClustersAgentPool{
+				Status: asocontainerservicev1.ManagedClusters_AgentPool_STATUS{
 					Count: ptr.To(2),
 				},
 			},
 			expected: &asocontainerservicev1.ManagedClustersAgentPool{
 				Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{
-					Count: ptr.To(2),
+					Count: nil,
 				},
 			},
 		},
@@ -407,7 +415,7 @@ func TestSetAgentPoolCount(t *testing.T) {
 			},
 			agentPool: &asocontainerservicev1.ManagedClustersAgentPool{
 				Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{
-					OrchestratorVersion: nil,
+					Count: nil,
 				},
 			},
 			expected: &asocontainerservicev1.ManagedClustersAgentPool{
@@ -467,10 +475,18 @@ func TestSetAgentPoolCount(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			g := NewGomegaWithT(t)
 
+			var c client.Client
+			if test.existingAgentPool != nil {
+				c = fakeclient.NewClientBuilder().
+					WithScheme(s).
+					WithObjects(test.existingAgentPool).
+					Build()
+			}
+
 			before := test.agentPool.DeepCopy()
 			uap := apUnstructured(g, test.agentPool)
 
-			err := setAgentPoolCount(ctx, test.machinePool, "", uap)
+			err := setAgentPoolCount(ctx, c, test.machinePool, "", uap)
 			g.Expect(s.Convert(uap, test.agentPool, nil)).To(Succeed())
 			if test.expectedErr != nil {
 				g.Expect(err).To(MatchError(test.expectedErr))