Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASOAPI: propagate machinepool values to ManagedClustersAgentPools #4798

Merged
merged 2 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions exp/api/v1alpha1/azureasomanagedmachinepool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 5 additions & 3 deletions exp/controllers/azureasomanagedmachinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@
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",
Expand All @@ -209,7 +208,7 @@
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
}
Expand All @@ -223,7 +222,7 @@
}
}
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)

Check warning on line 225 in exp/controllers/azureasomanagedmachinepool_controller.go

View check run for this annotation

Codecov / codecov/patch

exp/controllers/azureasomanagedmachinepool_controller.go#L225

Added line #L225 was not covered by tests
}

resourceReconciler := r.newResourceReconciler(asoManagedMachinePool, resources)
Expand Down Expand Up @@ -276,6 +275,9 @@
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

Expand Down
113 changes: 113 additions & 0 deletions exp/controllers/azureasomanagedmachinepool_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 15 additions & 2 deletions exp/mutators/azureasomanagedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
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"
Expand Down Expand Up @@ -237,7 +239,7 @@
}

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{}
Expand All @@ -253,7 +255,18 @@

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

Check warning on line 260 in exp/mutators/azureasomanagedcontrolplane.go

View check run for this annotation

Codecov / codecov/patch

exp/mutators/azureasomanagedcontrolplane.go#L260

Added line #L260 was not covered by tests
}
if machinePool == nil {
log.V(2).Info("Waiting for MachinePool Controller to set OwnerRef on AzureASOManagedMachinePool")
return nil, nil

Check warning on line 264 in exp/mutators/azureasomanagedcontrolplane.go

View check run for this annotation

Codecov / codecov/patch

exp/mutators/azureasomanagedcontrolplane.go#L263-L264

Added lines #L263 - L264 were not covered by tests
}

resources, err := ApplyMutators(ctx, asoManagedMachinePool.Spec.Resources,
SetAgentPoolDefaults(ptr.To(asoManagedMachinePool), machinePool),
)
if err != nil {
return nil, err
}
Expand Down
78 changes: 75 additions & 3 deletions exp/mutators/azureasomanagedcontrolplane_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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}}
Expand Down Expand Up @@ -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
}
Loading