Skip to content

Commit

Permalink
Merge pull request #4798 from nojnhuh/v2-machinepools
Browse files Browse the repository at this point in the history 
ASOAPI: propagate machinepool values to ManagedClustersAgentPools
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 2, 2024
2 parents daf5081 + 368425e commit 39189af
Show file tree
Hide file tree
Showing 11 changed files with 856 additions and 20 deletions.
9 changes: 7 additions & 2 deletions exp/api/v1alpha1/azureasomanagedmachinepool_types.go
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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

Expand Down
113 changes: 113 additions & 0 deletions exp/controllers/azureasomanagedmachinepool_controller_test.go
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}
Expand All @@ -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
}
Expand Down
78 changes: 75 additions & 3 deletions exp/mutators/azureasomanagedcontrolplane_test.go
View file
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

0 comments on commit 39189af

Please sign in to comment.