diff --git a/exp/api/v1alpha1/azureasomanagedmachinepool_types.go b/exp/api/v1alpha1/azureasomanagedmachinepool_types.go index 6c4fdadc640..240a66eec34 100644 --- a/exp/api/v1alpha1/azureasomanagedmachinepool_types.go +++ b/exp/api/v1alpha1/azureasomanagedmachinepool_types.go @@ -20,8 +20,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// AzureASOManagedMachinePoolKind is the kind for AzureASOManagedMachinePool. -const AzureASOManagedMachinePoolKind = "AzureASOManagedMachinePool" +const ( + // AzureASOManagedMachinePoolKind is the kind for AzureASOManagedMachinePool. + AzureASOManagedMachinePoolKind = "AzureASOManagedMachinePool" + + // ReplicasManagedByAKS is the value of the CAPI replica manager annotation that maps to the AKS built-in autoscaler. + ReplicasManagedByAKS = "aks" +) // AzureASOManagedMachinePoolSpec defines the desired state of AzureASOManagedMachinePool. type AzureASOManagedMachinePoolSpec struct { diff --git a/exp/controllers/azureasomanagedmachinepool_controller.go b/exp/controllers/azureasomanagedmachinepool_controller.go index f3ee9bb7257..b24222c5711 100644 --- a/exp/controllers/azureasomanagedmachinepool_controller.go +++ b/exp/controllers/azureasomanagedmachinepool_controller.go @@ -196,7 +196,6 @@ func (r *AzureASOManagedMachinePoolReconciler) Reconcile(ctx context.Context, re return r.reconcileNormal(ctx, asoManagedMachinePool, machinePool, cluster) } -//nolint:unparam // these parameters will be used soon enough func (r *AzureASOManagedMachinePoolReconciler) reconcileNormal(ctx context.Context, asoManagedMachinePool *infrav1exp.AzureASOManagedMachinePool, machinePool *expv1.MachinePool, cluster *clusterv1.Cluster) (ctrl.Result, error) { ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.AzureASOManagedMachinePoolReconciler.reconcileNormal", @@ -209,7 +208,7 @@ func (r *AzureASOManagedMachinePoolReconciler) reconcileNormal(ctx context.Conte return ctrl.Result{Requeue: true}, nil } - resources, err := mutators.ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources) + resources, err := mutators.ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources, mutators.SetAgentPoolDefaults(asoManagedMachinePool, machinePool)) if err != nil { return ctrl.Result{}, err } @@ -223,7 +222,7 @@ func (r *AzureASOManagedMachinePoolReconciler) reconcileNormal(ctx context.Conte } } if agentPoolName == "" { - return ctrl.Result{}, reconcile.TerminalError(fmt.Errorf("no %s ManagedClustersAgentPools defined in AzureASOManagedMachinePool spec.resources", asocontainerservicev1.GroupVersion.Group)) + return ctrl.Result{}, reconcile.TerminalError(mutators.ErrNoManagedClustersAgentPoolDefined) } resourceReconciler := r.newResourceReconciler(asoManagedMachinePool, resources) @@ -276,6 +275,9 @@ 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 { + machinePool.Spec.Replicas = &asoManagedMachinePool.Status.Replicas + } asoManagedMachinePool.Status.Ready = true diff --git a/exp/controllers/azureasomanagedmachinepool_controller_test.go b/exp/controllers/azureasomanagedmachinepool_controller_test.go index 82d3566fc2a..648363b4262 100644 --- a/exp/controllers/azureasomanagedmachinepool_controller_test.go +++ b/exp/controllers/azureasomanagedmachinepool_controller_test.go @@ -354,6 +354,9 @@ func TestAzureASOManagedMachinePoolReconcile(t *testing.T) { clusterv1.ClusterNameLabel: "cluster", }, }, + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](1), + }, } c := fakeClientBuilder(). WithObjects(asoManagedMachinePool, machinePool, cluster, asoAgentPool, asoManagedCluster). @@ -410,6 +413,116 @@ func TestAzureASOManagedMachinePoolReconcile(t *testing.T) { g.Expect(asoManagedMachinePool.Spec.ProviderIDList).To(ConsistOf("azure://node1", "azure://node2")) g.Expect(asoManagedMachinePool.Status.Replicas).To(Equal(int32(3))) g.Expect(asoManagedMachinePool.Status.Ready).To(BeTrue()) + + g.Expect(r.Get(ctx, client.ObjectKeyFromObject(machinePool), machinePool)).To(Succeed()) + g.Expect(*machinePool.Spec.Replicas).To(Equal(int32(1))) + }) + + t.Run("successfully reconciles normally with autoscaling", func(t *testing.T) { + g := NewGomegaWithT(t) + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Namespace: "ns", + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: &corev1.ObjectReference{ + APIVersion: infrav1exp.GroupVersion.Identifier(), + Kind: infrav1exp.AzureASOManagedControlPlaneKind, + }, + }, + } + asoManagedCluster := &asocontainerservicev1.ManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mc", + Namespace: cluster.Namespace, + }, + Status: asocontainerservicev1.ManagedCluster_STATUS{ + NodeResourceGroup: ptr.To("MC_rg"), + }, + } + asoAgentPool := &asocontainerservicev1.ManagedClustersAgentPool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ap", + Namespace: cluster.Namespace, + }, + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + AzureName: "pool1", + Owner: &genruntime.KnownResourceReference{ + Name: asoManagedCluster.Name, + }, + EnableAutoScaling: ptr.To(true), + }, + Status: asocontainerservicev1.ManagedClusters_AgentPool_STATUS{ + Count: ptr.To(3), + }, + } + asoManagedMachinePool := &infrav1exp.AzureASOManagedMachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ammp", + Namespace: cluster.Namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: expv1.GroupVersion.Identifier(), + Kind: "MachinePool", + Name: "mp", + }, + }, + Finalizers: []string{ + clusterv1.ClusterFinalizer, + }, + }, + Spec: infrav1exp.AzureASOManagedMachinePoolSpec{ + AzureASOManagedMachinePoolTemplateResourceSpec: infrav1exp.AzureASOManagedMachinePoolTemplateResourceSpec{ + Resources: []runtime.RawExtension{ + { + Raw: apJSON(g, asoAgentPool), + }, + }, + }, + }, + Status: infrav1exp.AzureASOManagedMachinePoolStatus{ + Ready: false, + }, + } + machinePool := &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mp", + Namespace: cluster.Namespace, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "cluster", + }, + }, + } + c := fakeClientBuilder(). + WithObjects(asoManagedMachinePool, machinePool, cluster, asoAgentPool, asoManagedCluster). + Build() + r := &AzureASOManagedMachinePoolReconciler{ + Client: c, + newResourceReconciler: func(_ *infrav1exp.AzureASOManagedMachinePool, _ []*unstructured.Unstructured) resourceReconciler { + return &fakeResourceReconciler{ + reconcileFunc: func(ctx context.Context, o client.Object) error { + return nil + }, + } + }, + Tracker: &FakeClusterTracker{ + getClientFunc: func(_ context.Context, _ types.NamespacedName) (client.Client, error) { + return fakeclient.NewClientBuilder().Build(), nil + }, + }, + } + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(asoManagedMachinePool)}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).To(Equal(ctrl.Result{})) + + g.Expect(r.Get(ctx, client.ObjectKeyFromObject(asoManagedMachinePool), asoManagedMachinePool)).To(Succeed()) + g.Expect(asoManagedMachinePool.Status.Replicas).To(Equal(int32(3))) + g.Expect(asoManagedMachinePool.Status.Ready).To(BeTrue()) + + g.Expect(r.Get(ctx, client.ObjectKeyFromObject(machinePool), machinePool)).To(Succeed()) + g.Expect(*machinePool.Spec.Replicas).To(Equal(int32(3))) }) t.Run("successfully reconciles pause", func(t *testing.T) { diff --git a/exp/mutators/azureasomanagedcontrolplane.go b/exp/mutators/azureasomanagedcontrolplane.go index 5e6a3b351e2..946b497d28e 100644 --- a/exp/mutators/azureasomanagedcontrolplane.go +++ b/exp/mutators/azureasomanagedcontrolplane.go @@ -25,10 +25,12 @@ 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" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + exputil "sigs.k8s.io/cluster-api/exp/util" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/conversion" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -237,7 +239,7 @@ func setManagedClusterAgentPoolProfiles(ctx context.Context, ctrlClient client.C } func agentPoolsFromManagedMachinePools(ctx context.Context, ctrlClient client.Client, clusterName string, namespace string) ([]conversion.Convertible, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "mutators.agentPoolsFromManagedMachinePools") + ctx, log, done := tele.StartSpanWithLogger(ctx, "mutators.agentPoolsFromManagedMachinePools") defer done() asoManagedMachinePools := &infrav1exp.AzureASOManagedMachinePoolList{} @@ -253,7 +255,18 @@ func agentPoolsFromManagedMachinePools(ctx context.Context, ctrlClient client.Cl var agentPools []conversion.Convertible for _, asoManagedMachinePool := range asoManagedMachinePools.Items { - resources, err := ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources) + machinePool, err := exputil.GetOwnerMachinePool(ctx, ctrlClient, asoManagedMachinePool.ObjectMeta) + if err != nil { + return nil, err + } + if machinePool == nil { + log.V(2).Info("Waiting for MachinePool Controller to set OwnerRef on AzureASOManagedMachinePool") + return nil, nil + } + + resources, err := ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources, + SetAgentPoolDefaults(ptr.To(asoManagedMachinePool), machinePool), + ) if err != nil { return nil, err } diff --git a/exp/mutators/azureasomanagedcontrolplane_test.go b/exp/mutators/azureasomanagedcontrolplane_test.go index 0e0a0f44a57..35c260651f8 100644 --- a/exp/mutators/azureasomanagedcontrolplane_test.go +++ b/exp/mutators/azureasomanagedcontrolplane_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/utils/ptr" 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" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/conversion" ) @@ -494,6 +495,7 @@ func TestSetManagedClusterAgentPoolProfiles(t *testing.T) { s := runtime.NewScheme() g.Expect(asocontainerservicev1.AddToScheme(s)).To(Succeed()) g.Expect(infrav1exp.AddToScheme(s)).To(Succeed()) + g.Expect(expv1.AddToScheme(s)).To(Succeed()) fakeClientBuilder := func() *fakeclient.ClientBuilder { return fakeclient.NewClientBuilder().WithScheme(s) } @@ -558,6 +560,13 @@ func TestSetManagedClusterAgentPoolProfiles(t *testing.T) { Labels: map[string]string{ clusterv1.ClusterNameLabel: "not-" + clusterName, }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: expv1.GroupVersion.Identifier(), + Kind: "MachinePool", + Name: "wrong-label", + }, + }, }, Spec: infrav1exp.AzureASOManagedMachinePoolSpec{ AzureASOManagedMachinePoolTemplateResourceSpec: infrav1exp.AzureASOManagedMachinePoolTemplateResourceSpec{ @@ -580,6 +589,13 @@ func TestSetManagedClusterAgentPoolProfiles(t *testing.T) { Labels: map[string]string{ clusterv1.ClusterNameLabel: clusterName, }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: expv1.GroupVersion.Identifier(), + Kind: "MachinePool", + Name: "wrong-namespace", + }, + }, }, Spec: infrav1exp.AzureASOManagedMachinePoolSpec{ AzureASOManagedMachinePoolTemplateResourceSpec: infrav1exp.AzureASOManagedMachinePoolTemplateResourceSpec{ @@ -602,6 +618,13 @@ func TestSetManagedClusterAgentPoolProfiles(t *testing.T) { Labels: map[string]string{ clusterv1.ClusterNameLabel: clusterName, }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: expv1.GroupVersion.Identifier(), + Kind: "MachinePool", + Name: "pool0", + }, + }, }, Spec: infrav1exp.AzureASOManagedMachinePoolSpec{ AzureASOManagedMachinePoolTemplateResourceSpec: infrav1exp.AzureASOManagedMachinePoolTemplateResourceSpec{ @@ -624,6 +647,13 @@ func TestSetManagedClusterAgentPoolProfiles(t *testing.T) { Labels: map[string]string{ clusterv1.ClusterNameLabel: clusterName, }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: expv1.GroupVersion.Identifier(), + Kind: "MachinePool", + Name: "pool1", + }, + }, }, Spec: infrav1exp.AzureASOManagedMachinePoolSpec{ AzureASOManagedMachinePoolTemplateResourceSpec: infrav1exp.AzureASOManagedMachinePoolTemplateResourceSpec{ @@ -641,17 +671,51 @@ func TestSetManagedClusterAgentPoolProfiles(t *testing.T) { }, }, } + machinePools := &expv1.MachinePoolList{ + Items: []expv1.MachinePool{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "wrong-label", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "not-" + namespace, + Name: "wrong-namespace", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "pool0", + }, + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](1), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "pool1", + }, + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](2), + }, + }, + }, + } expected := &asocontainerservicev1.ManagedCluster{ Spec: asocontainerservicev1.ManagedCluster_Spec{ AgentPoolProfiles: []asocontainerservicev1.ManagedClusterAgentPoolProfile{ - {Name: ptr.To("azpool0")}, - {Name: ptr.To("azpool1")}, + {Name: ptr.To("azpool0"), Count: ptr.To(1)}, + {Name: ptr.To("azpool1"), Count: ptr.To(2)}, }, }, } c := fakeClientBuilder(). - WithLists(asoManagedMachinePools). + WithLists(asoManagedMachinePools, machinePools). Build() cluster := &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: clusterName}} @@ -770,3 +834,11 @@ func mcUnstructured(g Gomega, mc *asocontainerservicev1.ManagedCluster) *unstruc g.Expect(s.Convert(mc, u, nil)).To(Succeed()) return u } + +func apUnstructured(g Gomega, ap *asocontainerservicev1.ManagedClustersAgentPool) *unstructured.Unstructured { + s := runtime.NewScheme() + g.Expect(asocontainerservicev1.AddToScheme(s)).To(Succeed()) + u := &unstructured.Unstructured{} + g.Expect(s.Convert(ap, u, nil)).To(Succeed()) + return u +} diff --git a/exp/mutators/azureasomanagedmachinepool.go b/exp/mutators/azureasomanagedmachinepool.go new file mode 100644 index 00000000000..4348aad1261 --- /dev/null +++ b/exp/mutators/azureasomanagedmachinepool.go @@ -0,0 +1,157 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutators + +import ( + "context" + "fmt" + "strings" + + asocontainerservicev1 "github.com/Azure/azure-service-operator/v2/api/containerservice/v1api20231001" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha1" + "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/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 { + return func(ctx context.Context, us []*unstructured.Unstructured) error { + ctx, _, done := tele.StartSpanWithLogger(ctx, "mutators.SetAgentPoolDefaults") + defer done() + + var agentPool *unstructured.Unstructured + var agentPoolPath string + for i, u := range us { + if u.GroupVersionKind().Group == asocontainerservicev1.GroupVersion.Group && + u.GroupVersionKind().Kind == "ManagedClustersAgentPool" { + agentPool = u + agentPoolPath = fmt.Sprintf("spec.resources[%d]", i) + break + } + } + if agentPool == nil { + return reconcile.TerminalError(ErrNoManagedClustersAgentPoolDefined) + } + + if err := setAgentPoolOrchestratorVersion(ctx, machinePool, agentPoolPath, agentPool); err != nil { + return err + } + + if err := reconcileAutoscaling(agentPool, machinePool); err != nil { + return err + } + + if err := setAgentPoolCount(ctx, machinePool, agentPoolPath, agentPool); err != nil { + return err + } + + return nil + } +} + +func setAgentPoolOrchestratorVersion(ctx context.Context, machinePool *expv1.MachinePool, agentPoolPath string, agentPool *unstructured.Unstructured) error { + _, log, done := tele.StartSpanWithLogger(ctx, "mutators.setAgentPoolOrchestratorVersion") + defer done() + + if machinePool.Spec.Template.Spec.Version == nil { + return nil + } + + k8sVersionPath := []string{"spec", "orchestratorVersion"} + capiK8sVersion := strings.TrimPrefix(*machinePool.Spec.Template.Spec.Version, "v") + userK8sVersion, k8sVersionFound, err := unstructured.NestedString(agentPool.UnstructuredContent(), k8sVersionPath...) + if err != nil { + return err + } + setK8sVersion := mutation{ + location: agentPoolPath + "." + strings.Join(k8sVersionPath, "."), + val: capiK8sVersion, + reason: fmt.Sprintf("because MachinePool %s's spec.template.spec.version is %s", machinePool.Name, *machinePool.Spec.Template.Spec.Version), + } + if k8sVersionFound && userK8sVersion != capiK8sVersion { + return Incompatible{ + mutation: setK8sVersion, + userVal: userK8sVersion, + } + } + logMutation(log, setK8sVersion) + return unstructured.SetNestedField(agentPool.UnstructuredContent(), capiK8sVersion, k8sVersionPath...) +} + +func reconcileAutoscaling(agentPool *unstructured.Unstructured, machinePool *expv1.MachinePool) error { + autoscaling, _, err := unstructured.NestedBool(agentPool.UnstructuredContent(), "spec", "enableAutoScaling") + if err != nil { + return err + } + + // Update the MachinePool replica manager annotation. This isn't wrapped in a mutation object because + // it's not modifying an ASO resource and users are not expected to set this manually. This behavior + // is documented by CAPI as expected of a provider. + replicaManager, ok := machinePool.Annotations[clusterv1.ReplicasManagedByAnnotation] + if autoscaling { + if !ok { + if machinePool.Annotations == nil { + machinePool.Annotations = make(map[string]string) + } + machinePool.Annotations[clusterv1.ReplicasManagedByAnnotation] = infrav1exp.ReplicasManagedByAKS + } else if replicaManager != infrav1exp.ReplicasManagedByAKS { + return fmt.Errorf("failed to enable autoscaling, replicas are already being managed by %s according to MachinePool %s's %s annotation", replicaManager, machinePool.Name, clusterv1.ReplicasManagedByAnnotation) + } + } else if !autoscaling && replicaManager == infrav1exp.ReplicasManagedByAKS { + // Removing this annotation informs the MachinePool controller that this MachinePool is no longer + // being autoscaled. + delete(machinePool.Annotations, clusterv1.ReplicasManagedByAnnotation) + } + + return nil +} + +func setAgentPoolCount(ctx context.Context, machinePool *expv1.MachinePool, agentPoolPath string, agentPool *unstructured.Unstructured) error { + _, log, done := tele.StartSpanWithLogger(ctx, "mutators.setAgentPoolOrchestratorVersion") + defer done() + + autoscaling := machinePool.Annotations[clusterv1.ReplicasManagedByAnnotation] == infrav1exp.ReplicasManagedByAKS + if machinePool.Spec.Replicas == nil || autoscaling { + return nil + } + + countPath := []string{"spec", "count"} + capiCount := int64(*machinePool.Spec.Replicas) + userCount, countFound, err := unstructured.NestedInt64(agentPool.UnstructuredContent(), countPath...) + if err != nil { + return err + } + setCount := mutation{ + location: agentPoolPath + "." + strings.Join(countPath, "."), + val: capiCount, + reason: fmt.Sprintf("because MachinePool %s's spec.replicas is %d", machinePool.Name, capiCount), + } + if countFound && userCount != capiCount { + return Incompatible{ + mutation: setCount, + userVal: userCount, + } + } + logMutation(log, setCount) + return unstructured.SetNestedField(agentPool.UnstructuredContent(), capiCount, countPath...) +} diff --git a/exp/mutators/azureasomanagedmachinepool_test.go b/exp/mutators/azureasomanagedmachinepool_test.go new file mode 100644 index 00000000000..b48688c6d58 --- /dev/null +++ b/exp/mutators/azureasomanagedmachinepool_test.go @@ -0,0 +1,484 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutators + +import ( + "context" + "errors" + "testing" + + asocontainerservicev1 "github.com/Azure/azure-service-operator/v2/api/containerservice/v1api20231001" + "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + 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" +) + +func TestSetAgentPoolDefaults(t *testing.T) { + ctx := context.Background() + g := NewGomegaWithT(t) + + tests := []struct { + name string + asoManagedMachinePool *infrav1exp.AzureASOManagedMachinePool + machinePool *expv1.MachinePool + expected []*unstructured.Unstructured + expectedErr error + }{ + { + name: "no ManagedClustersAgentPool", + asoManagedMachinePool: &infrav1exp.AzureASOManagedMachinePool{ + Spec: infrav1exp.AzureASOManagedMachinePoolSpec{ + AzureASOManagedMachinePoolTemplateResourceSpec: infrav1exp.AzureASOManagedMachinePoolTemplateResourceSpec{ + Resources: []runtime.RawExtension{}, + }, + }, + }, + expectedErr: ErrNoManagedClustersAgentPoolDefined, + }, + { + name: "success", + asoManagedMachinePool: &infrav1exp.AzureASOManagedMachinePool{ + Spec: infrav1exp.AzureASOManagedMachinePoolSpec{ + AzureASOManagedMachinePoolTemplateResourceSpec: infrav1exp.AzureASOManagedMachinePoolTemplateResourceSpec{ + Resources: []runtime.RawExtension{ + { + Raw: apJSON(g, &asocontainerservicev1.ManagedClustersAgentPool{}), + }, + }, + }, + }, + }, + machinePool: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](1), + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: ptr.To("vcapi k8s version"), + }, + }, + }, + }, + expected: []*unstructured.Unstructured{ + apUnstructured(g, &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: ptr.To("capi k8s version"), + Count: ptr.To(1), + }, + }), + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + mutator := SetAgentPoolDefaults(test.asoManagedMachinePool, test.machinePool) + actual, err := ApplyMutators(ctx, test.asoManagedMachinePool.Spec.Resources, mutator) + if test.expectedErr != nil { + g.Expect(err).To(MatchError(test.expectedErr)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + g.Expect(cmp.Diff(test.expected, actual)).To(BeEmpty()) + }) + } +} + +func TestSetAgentPoolOrchestratorVersion(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + machinePool *expv1.MachinePool + agentPool *asocontainerservicev1.ManagedClustersAgentPool + expected *asocontainerservicev1.ManagedClustersAgentPool + expectedErr error + }{ + { + name: "no CAPI opinion", + machinePool: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: nil, + }, + }, + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: ptr.To("user k8s version"), + }, + }, + expected: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: ptr.To("user k8s version"), + }, + }, + }, + { + name: "set from CAPI opinion", + machinePool: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: ptr.To("vcapi k8s version"), + }, + }, + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: nil, + }, + }, + expected: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: ptr.To("capi k8s version"), + }, + }, + }, + { + name: "user value matching CAPI ok", + machinePool: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: ptr.To("vcapi k8s version"), + }, + }, + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: ptr.To("capi k8s version"), + }, + }, + expected: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: ptr.To("capi k8s version"), + }, + }, + }, + { + name: "incompatible", + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mp", + }, + Spec: expv1.MachinePoolSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: ptr.To("vcapi k8s version"), + }, + }, + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: ptr.To("user k8s version"), + }, + }, + expectedErr: Incompatible{ + mutation: mutation{ + location: ".spec.orchestratorVersion", + val: "capi k8s version", + reason: "because MachinePool mp's spec.template.spec.version is vcapi k8s version", + }, + userVal: "user k8s version", + }, + }, + } + + s := runtime.NewScheme() + NewGomegaWithT(t).Expect(asocontainerservicev1.AddToScheme(s)).To(Succeed()) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + before := test.agentPool.DeepCopy() + uap := apUnstructured(g, test.agentPool) + + err := setAgentPoolOrchestratorVersion(ctx, test.machinePool, "", uap) + g.Expect(s.Convert(uap, test.agentPool, nil)).To(Succeed()) + if test.expectedErr != nil { + g.Expect(err).To(MatchError(test.expectedErr)) + g.Expect(cmp.Diff(before, test.agentPool)).To(BeEmpty()) // errors should never modify the resource. + } else { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cmp.Diff(test.expected, test.agentPool)).To(BeEmpty()) + } + }) + } +} + +func TestReconcileAutoscaling(t *testing.T) { + tests := []struct { + name string + autoscaling bool + machinePool *expv1.MachinePool + expected *expv1.MachinePool + expectedErr error + }{ + { + name: "autoscaling disabled removes aks annotation", + autoscaling: false, + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: infrav1exp.ReplicasManagedByAKS, + }, + }, + }, + expected: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + }, + { + name: "autoscaling disabled leaves other annotation", + autoscaling: false, + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: "not-" + infrav1exp.ReplicasManagedByAKS, + }, + }, + }, + expected: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: "not-" + infrav1exp.ReplicasManagedByAKS, + }, + }, + }, + }, + { + name: "autoscaling enabled, manager undefined adds annotation", + autoscaling: true, + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }, + expected: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: infrav1exp.ReplicasManagedByAKS, + }, + }, + }, + }, + { + name: "autoscaling enabled, manager already set", + autoscaling: true, + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: infrav1exp.ReplicasManagedByAKS, + }, + }, + }, + expected: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: infrav1exp.ReplicasManagedByAKS, + }, + }, + }, + }, + { + name: "autoscaling enabled, manager set to something else", + autoscaling: true, + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mp", + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: "not-" + infrav1exp.ReplicasManagedByAKS, + }, + }, + }, + expectedErr: errors.New("failed to enable autoscaling, replicas are already being managed by not-aks according to MachinePool mp's cluster.x-k8s.io/replicas-managed-by annotation"), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + agentPool := &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + EnableAutoScaling: ptr.To(test.autoscaling), + }, + } + + err := reconcileAutoscaling(apUnstructured(g, agentPool), test.machinePool) + + if test.expectedErr != nil { + g.Expect(err).To(MatchError(test.expectedErr)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cmp.Diff(test.expected, test.machinePool)).To(BeEmpty()) + } + }) + } +} + +func TestSetAgentPoolCount(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + machinePool *expv1.MachinePool + agentPool *asocontainerservicev1.ManagedClustersAgentPool + expected *asocontainerservicev1.ManagedClustersAgentPool + expectedErr error + }{ + { + name: "no CAPI opinion", + machinePool: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: nil, + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(2), + }, + }, + expected: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(2), + }, + }, + }, + { + name: "autoscaling enabled", + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.ReplicasManagedByAnnotation: infrav1exp.ReplicasManagedByAKS, + }, + }, + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](3), + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(2), + }, + }, + expected: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(2), + }, + }, + }, + { + name: "set from CAPI opinion", + machinePool: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](1), + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + OrchestratorVersion: nil, + }, + }, + expected: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(1), + }, + }, + }, + { + name: "user value matching CAPI ok", + machinePool: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](1), + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(1), + }, + }, + expected: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(1), + }, + }, + }, + { + name: "incompatible", + machinePool: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mp", + }, + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](1), + }, + }, + agentPool: &asocontainerservicev1.ManagedClustersAgentPool{ + Spec: asocontainerservicev1.ManagedClusters_AgentPool_Spec{ + Count: ptr.To(2), + }, + }, + expectedErr: Incompatible{ + mutation: mutation{ + location: ".spec.count", + val: int64(1), + reason: "because MachinePool mp's spec.replicas is 1", + }, + userVal: int64(2), + }, + }, + } + + s := runtime.NewScheme() + NewGomegaWithT(t).Expect(asocontainerservicev1.AddToScheme(s)).To(Succeed()) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + before := test.agentPool.DeepCopy() + uap := apUnstructured(g, test.agentPool) + + err := setAgentPoolCount(ctx, test.machinePool, "", uap) + g.Expect(s.Convert(uap, test.agentPool, nil)).To(Succeed()) + if test.expectedErr != nil { + g.Expect(err).To(MatchError(test.expectedErr)) + g.Expect(cmp.Diff(before, test.agentPool)).To(BeEmpty()) // errors should never modify the resource. + } else { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(cmp.Diff(test.expected, test.agentPool)).To(BeEmpty()) + } + }) + } +} diff --git a/templates/cluster-template-aks-aso.yaml b/templates/cluster-template-aks-aso.yaml index d3862da75c8..c5189fee1a1 100644 --- a/templates/cluster-template-aks-aso.yaml +++ b/templates/cluster-template-aks-aso.yaml @@ -95,7 +95,6 @@ spec: name: ${CLUSTER_NAME}-pool0 spec: azureName: pool0 - count: ${WORKER_MACHINE_COUNT:=2} mode: System owner: name: ${CLUSTER_NAME} @@ -137,7 +136,6 @@ spec: name: ${CLUSTER_NAME}-pool1 spec: azureName: pool1 - count: ${WORKER_MACHINE_COUNT:=2} mode: User owner: name: ${CLUSTER_NAME} diff --git a/templates/flavors/aks-aso/cluster-template.yaml b/templates/flavors/aks-aso/cluster-template.yaml index 8d059145936..47316f4333d 100644 --- a/templates/flavors/aks-aso/cluster-template.yaml +++ b/templates/flavors/aks-aso/cluster-template.yaml @@ -97,8 +97,6 @@ spec: mode: System type: VirtualMachineScaleSets vmSize: ${AZURE_NODE_MACHINE_TYPE} - # Soon this will be derived from the MachinePool - count: ${WORKER_MACHINE_COUNT:=2} --- apiVersion: cluster.x-k8s.io/v1beta1 kind: MachinePool @@ -138,5 +136,3 @@ spec: mode: User type: VirtualMachineScaleSets vmSize: ${AZURE_NODE_MACHINE_TYPE} - # Soon this will be derived from the MachinePool - count: ${WORKER_MACHINE_COUNT:=2} diff --git a/templates/test/ci/cluster-template-prow-aks-aso.yaml b/templates/test/ci/cluster-template-prow-aks-aso.yaml index c0ea2df3fe2..ff0cfb21ee0 100644 --- a/templates/test/ci/cluster-template-prow-aks-aso.yaml +++ b/templates/test/ci/cluster-template-prow-aks-aso.yaml @@ -99,7 +99,6 @@ spec: name: ${CLUSTER_NAME}-pool0 spec: azureName: pool0 - count: ${WORKER_MACHINE_COUNT:=2} mode: System owner: name: ${CLUSTER_NAME} @@ -141,7 +140,6 @@ spec: name: ${CLUSTER_NAME}-pool1 spec: azureName: pool1 - count: ${WORKER_MACHINE_COUNT:=2} mode: User owner: name: ${CLUSTER_NAME} @@ -183,7 +181,6 @@ spec: name: ${CLUSTER_NAME}-pool2 spec: azureName: pool2 - count: 1 mode: User osType: Windows owner: diff --git a/templates/test/ci/prow-aks-aso/patches/aks-pool2.yaml b/templates/test/ci/prow-aks-aso/patches/aks-pool2.yaml index 6fdfcd1ee9b..a12346f24a2 100644 --- a/templates/test/ci/prow-aks-aso/patches/aks-pool2.yaml +++ b/templates/test/ci/prow-aks-aso/patches/aks-pool2.yaml @@ -37,4 +37,3 @@ spec: type: VirtualMachineScaleSets vmSize: "${AZURE_AKS_NODE_MACHINE_TYPE:=Standard_D2s_v3}" osType: Windows - count: 1