From becac55d712f45b019d965b54f4c2cf4232aca20 Mon Sep 17 00:00:00 2001 From: willie-yao Date: Wed, 31 Jan 2024 21:57:24 +0000 Subject: [PATCH 1/2] Add mp to autoscaler e2e test --- exp/internal/webhooks/machinepool.go | 108 ++++++++++- exp/internal/webhooks/machinepool_test.go | 167 +++++++++++++++++- test/e2e/autoscaler.go | 140 ++++++++++++--- test/e2e/autoscaler_test.go | 19 +- .../cluster-autoscaler.yaml | 6 + test/framework/autoscaler_helpers.go | 154 ++++++++++++++-- test/framework/machinepool_helpers.go | 66 +++++++ 7 files changed, 606 insertions(+), 54 deletions(-) diff --git a/exp/internal/webhooks/machinepool.go b/exp/internal/webhooks/machinepool.go index c13490001464..a69850699260 100644 --- a/exp/internal/webhooks/machinepool.go +++ b/exp/internal/webhooks/machinepool.go @@ -19,8 +19,11 @@ package webhooks import ( "context" "fmt" + "strconv" "strings" + "github.com/pkg/errors" + v1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" @@ -36,6 +39,10 @@ import ( ) func (webhook *MachinePool) SetupWebhookWithManager(mgr ctrl.Manager) error { + if webhook.decoder == nil { + webhook.decoder = admission.NewDecoder(mgr.GetScheme()) + } + return ctrl.NewWebhookManagedBy(mgr). For(&expv1.MachinePool{}). WithDefaulter(webhook). @@ -47,27 +54,48 @@ func (webhook *MachinePool) SetupWebhookWithManager(mgr ctrl.Manager) error { // +kubebuilder:webhook:verbs=create;update,path=/mutate-cluster-x-k8s-io-v1beta1-machinepool,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=machinepools,versions=v1beta1,name=default.machinepool.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 // MachinePool implements a validation and defaulting webhook for MachinePool. -type MachinePool struct{} +type MachinePool struct { + decoder *admission.Decoder +} var _ webhook.CustomValidator = &MachinePool{} var _ webhook.CustomDefaulter = &MachinePool{} // Default implements webhook.Defaulter so a webhook will be registered for the type. -func (webhook *MachinePool) Default(_ context.Context, obj runtime.Object) error { +func (webhook *MachinePool) Default(ctx context.Context, obj runtime.Object) error { m, ok := obj.(*expv1.MachinePool) if !ok { return apierrors.NewBadRequest(fmt.Sprintf("expected a MachinePool but got a %T", obj)) } + req, err := admission.RequestFromContext(ctx) + if err != nil { + return err + } + dryRun := false + if req.DryRun != nil { + dryRun = *req.DryRun + } + var oldMP *expv1.MachinePool + if req.Operation == v1.Update { + oldMP = &expv1.MachinePool{} + if err := webhook.decoder.DecodeRaw(req.OldObject, oldMP); err != nil { + return errors.Wrapf(err, "failed to decode oldObject to MachinePool") + } + } + if m.Labels == nil { m.Labels = make(map[string]string) } m.Labels[clusterv1.ClusterNameLabel] = m.Spec.ClusterName - if m.Spec.Replicas == nil { - m.Spec.Replicas = ptr.To[int32](1) + replicas, err := calculateMachinePoolReplicas(ctx, oldMP, m, dryRun) + if err != nil { + return err } + m.Spec.Replicas = ptr.To[int32](replicas) + if m.Spec.MinReadySeconds == nil { m.Spec.MinReadySeconds = ptr.To[int32](0) } @@ -187,3 +215,75 @@ func (webhook *MachinePool) validate(oldObj, newObj *expv1.MachinePool) error { } return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("MachinePool").GroupKind(), newObj.Name, allErrs) } + +func calculateMachinePoolReplicas(ctx context.Context, oldMP *expv1.MachinePool, newMP *expv1.MachinePool, dryRun bool) (int32, error) { + // If replicas is already set => Keep the current value. + if newMP.Spec.Replicas != nil { + return *newMP.Spec.Replicas, nil + } + + log := ctrl.LoggerFrom(ctx) + + // If both autoscaler annotations are set, use them to calculate the default value. + minSizeString, hasMinSizeAnnotation := newMP.Annotations[clusterv1.AutoscalerMinSizeAnnotation] + maxSizeString, hasMaxSizeAnnotation := newMP.Annotations[clusterv1.AutoscalerMaxSizeAnnotation] + if hasMinSizeAnnotation && hasMaxSizeAnnotation { + minSize, err := strconv.ParseInt(minSizeString, 10, 32) + if err != nil { + return 0, errors.Wrapf(err, "failed to caculate MachinePool replicas value: could not parse the value of the %q annotation", clusterv1.AutoscalerMinSizeAnnotation) + } + maxSize, err := strconv.ParseInt(maxSizeString, 10, 32) + if err != nil { + return 0, errors.Wrapf(err, "failed to caculate MachinePool replicas value: could not parse the value of the %q annotation", clusterv1.AutoscalerMaxSizeAnnotation) + } + + // If it's a new MachinePool => Use the min size. + // Note: This will result in a scale up to get into the range where autoscaler takes over. + if oldMP == nil { + if !dryRun { + log.V(2).Info(fmt.Sprintf("Replica field has been defaulted to %d based on the %s annotation (MP is a new MP)", minSize, clusterv1.AutoscalerMinSizeAnnotation)) + } + return int32(minSize), nil + } + + // Otherwise we are handing over the control for the replicas field for an existing MachinePool + // to the autoscaler. + + switch { + // If the old MachinePool doesn't have replicas set => Use the min size. + // Note: As defaulting always sets the replica field, this case should not be possible + // We only have this handling to be 100% safe against panics. + case oldMP.Spec.Replicas == nil: + if !dryRun { + log.V(2).Info(fmt.Sprintf("Replica field has been defaulted to %d based on the %s annotation (old MP didn't have replicas set)", minSize, clusterv1.AutoscalerMinSizeAnnotation)) + } + return int32(minSize), nil + // If the old MachinePool replicas are lower than min size => Use the min size. + // Note: This will result in a scale up to get into the range where autoscaler takes over. + case *oldMP.Spec.Replicas < int32(minSize): + if !dryRun { + log.V(2).Info(fmt.Sprintf("Replica field has been defaulted to %d based on the %s annotation (old MP had replicas below min size)", minSize, clusterv1.AutoscalerMinSizeAnnotation)) + } + return int32(minSize), nil + // If the old MachinePool replicas are higher than max size => Use the max size. + // Note: This will result in a scale down to get into the range where autoscaler takes over. + case *oldMP.Spec.Replicas > int32(maxSize): + if !dryRun { + log.V(2).Info(fmt.Sprintf("Replica field has been defaulted to %d based on the %s annotation (old MP had replicas above max size)", maxSize, clusterv1.AutoscalerMaxSizeAnnotation)) + } + return int32(maxSize), nil + // If the old MachinePool replicas are between min and max size => Keep the current value. + default: + if !dryRun { + log.V(2).Info(fmt.Sprintf("Replica field has been defaulted to %d based on replicas of the old MachinePool (old MP had replicas within min size / max size range)", *oldMP.Spec.Replicas)) + } + return *oldMP.Spec.Replicas, nil + } + } + + // If neither the default nor the autoscaler annotations are set => Default to 1. + if !dryRun { + log.V(2).Info("Replica field has been defaulted to 1") + } + return 1, nil +} diff --git a/exp/internal/webhooks/machinepool_test.go b/exp/internal/webhooks/machinepool_test.go index ecda5dadcd42..0f2c54fa1f82 100644 --- a/exp/internal/webhooks/machinepool_test.go +++ b/exp/internal/webhooks/machinepool_test.go @@ -17,6 +17,7 @@ limitations under the License. package webhooks import ( + "context" "strings" "testing" @@ -26,6 +27,7 @@ import ( utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" @@ -33,7 +35,7 @@ import ( "sigs.k8s.io/cluster-api/internal/webhooks/util" ) -var ctx = ctrl.SetupSignalHandler() +var ctx = admission.NewContextWithRequest(ctrl.SetupSignalHandler(), admission.Request{}) func TestMachinePoolDefault(t *testing.T) { // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. @@ -67,6 +69,169 @@ func TestMachinePoolDefault(t *testing.T) { g.Expect(mp.Spec.Template.Spec.Version).To(Equal(ptr.To("v1.20.0"))) } +func TestCalculateMachinePoolReplicas(t *testing.T) { + tests := []struct { + name string + newMP *expv1.MachinePool + oldMP *expv1.MachinePool + expectedReplicas int32 + expectErr bool + }{ + { + name: "if new MP has replicas set, keep that value", + newMP: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](5), + }, + }, + expectedReplicas: 5, + }, + { + name: "if new MP does not have replicas set and no annotations, use 1", + newMP: &expv1.MachinePool{}, + expectedReplicas: 1, + }, + { + name: "if new MP only has min size annotation, fallback to 1", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + }, + }, + }, + expectedReplicas: 1, + }, + { + name: "if new MP only has max size annotation, fallback to 1", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + expectedReplicas: 1, + }, + { + name: "if new MP has min and max size annotation and min size is invalid, fail", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "abc", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + expectErr: true, + }, + { + name: "if new MP has min and max size annotation and max size is invalid, fail", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "abc", + }, + }, + }, + expectErr: true, + }, + { + name: "if new MP has min and max size annotation and new MP is a new MP, use min size", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + expectedReplicas: 3, + }, + { + name: "if new MP has min and max size annotation and old MP doesn't have replicas set, use min size", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMP: &expv1.MachinePool{}, + expectedReplicas: 3, + }, + { + name: "if new MP has min and max size annotation and old MP replicas is below min size, use min size", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMP: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](1), + }, + }, + expectedReplicas: 3, + }, + { + name: "if new MP has min and max size annotation and old MP replicas is above max size, use max size", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMP: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](15), + }, + }, + expectedReplicas: 7, + }, + { + name: "if new MP has min and max size annotation and old MP replicas is between min and max size, use old MP replicas", + newMP: &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMP: &expv1.MachinePool{ + Spec: expv1.MachinePoolSpec{ + Replicas: ptr.To[int32](4), + }, + }, + expectedReplicas: 4, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + replicas, err := calculateMachinePoolReplicas(context.Background(), tt.oldMP, tt.newMP, false) + + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(replicas).To(Equal(tt.expectedReplicas)) + }) + } +} + func TestMachinePoolBootstrapValidation(t *testing.T) { // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. // Enabling the feature flag temporarily for this test. diff --git a/test/e2e/autoscaler.go b/test/e2e/autoscaler.go index 349373f264d6..9e55aad11c14 100644 --- a/test/e2e/autoscaler.go +++ b/test/e2e/autoscaler.go @@ -59,8 +59,9 @@ type AutoscalerSpecInput struct { // InfrastructureMachineTemplateKind should be the plural form of the InfraMachineTemplate kind. // It should be specified in lower case. // Example: dockermachinetemplates. - InfrastructureMachineTemplateKind string - AutoscalerVersion string + InfrastructureMachineTemplateKind string + InfrastructureMachinePoolTemplateKind string + AutoscalerVersion string // Allows to inject a function to be run after test namespace is created. // If not specified, this is a no-op. @@ -71,11 +72,13 @@ type AutoscalerSpecInput struct { // being deployed in the workload cluster. func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) { var ( - specName = "autoscaler" - input AutoscalerSpecInput - namespace *corev1.Namespace - cancelWatches context.CancelFunc - clusterResources *clusterctl.ApplyClusterTemplateAndWaitResult + specName = "autoscaler" + mpNodeGroupMinSize = "1" + mpNodeGroupMaxSize = "5" + input AutoscalerSpecInput + namespace *corev1.Namespace + cancelWatches context.CancelFunc + clusterResources *clusterctl.ApplyClusterTemplateAndWaitResult ) BeforeEach(func() { @@ -127,32 +130,40 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) WaitForClusterIntervals: input.E2EConfig.GetIntervals(specName, "wait-cluster"), WaitForControlPlaneIntervals: input.E2EConfig.GetIntervals(specName, "wait-control-plane"), WaitForMachineDeployments: input.E2EConfig.GetIntervals(specName, "wait-worker-nodes"), + WaitForMachinePools: input.E2EConfig.GetIntervals(specName, "wait-machine-pool-nodes"), }, clusterResources) Expect(clusterResources.Cluster.Spec.Topology).NotTo(BeNil(), "Autoscaler test expected a Classy Cluster") // Ensure the MachineDeploymentTopology has the autoscaler annotations. mdTopology := clusterResources.Cluster.Spec.Topology.Workers.MachineDeployments[0] Expect(mdTopology.Metadata.Annotations).NotTo(BeNil(), "MachineDeployment is expected to have autoscaler annotations") - nodeGroupMinSize, ok := mdTopology.Metadata.Annotations[clusterv1.AutoscalerMinSizeAnnotation] + mdNodeGroupMinSize, ok := mdTopology.Metadata.Annotations[clusterv1.AutoscalerMinSizeAnnotation] Expect(ok).To(BeTrue(), "MachineDeploymentTopology %s does not have the %q autoscaler annotation", mdTopology.Name, clusterv1.AutoscalerMinSizeAnnotation) - nodeGroupMaxSize, ok := mdTopology.Metadata.Annotations[clusterv1.AutoscalerMaxSizeAnnotation] + mdNodeGroupMaxSize, ok := mdTopology.Metadata.Annotations[clusterv1.AutoscalerMaxSizeAnnotation] Expect(ok).To(BeTrue(), "MachineDeploymentTopology %s does not have the %q autoscaler annotation", mdTopology.Name, clusterv1.AutoscalerMaxSizeAnnotation) + // Ensure the MachinePoolTopology does NOT have the autoscaler annotations so we can test MachineDeployments first. + mpTopology := clusterResources.Cluster.Spec.Topology.Workers.MachinePools[0] + Expect(mpTopology.Metadata.Annotations).To(BeNil(), "MachinePool is expected to have autoscaler annotations") + // Get a ClusterProxy so we can interact with the workload cluster workloadClusterProxy := input.BootstrapClusterProxy.GetWorkloadCluster(ctx, clusterResources.Cluster.Namespace, clusterResources.Cluster.Name) - originalReplicas := *clusterResources.MachineDeployments[0].Spec.Replicas - Expect(strconv.Itoa(int(originalReplicas))).To(Equal(nodeGroupMinSize), "MachineDeployment should have replicas as defined in %s", clusterv1.AutoscalerMinSizeAnnotation) + mdOriginalReplicas := *clusterResources.MachineDeployments[0].Spec.Replicas + Expect(strconv.Itoa(int(mdOriginalReplicas))).To(Equal(mdNodeGroupMinSize), "MachineDeployment should have replicas as defined in %s", clusterv1.AutoscalerMinSizeAnnotation) + mpOriginalReplicas := *clusterResources.MachinePools[0].Spec.Replicas + Expect(strconv.Itoa(int(mpOriginalReplicas))).To(Equal(mpNodeGroupMinSize), "MachinePool should have replicas as defined in %s", clusterv1.AutoscalerMinSizeAnnotation) By("Installing the autoscaler on the workload cluster") autoscalerWorkloadYAMLPath := input.E2EConfig.GetVariable(AutoscalerWorkloadYAMLPath) framework.ApplyAutoscalerToWorkloadCluster(ctx, framework.ApplyAutoscalerToWorkloadClusterInput{ - ArtifactFolder: input.ArtifactFolder, - InfrastructureMachineTemplateKind: input.InfrastructureMachineTemplateKind, - WorkloadYamlPath: autoscalerWorkloadYAMLPath, - ManagementClusterProxy: input.BootstrapClusterProxy, - WorkloadClusterProxy: workloadClusterProxy, - Cluster: clusterResources.Cluster, - AutoscalerVersion: input.AutoscalerVersion, + ArtifactFolder: input.ArtifactFolder, + InfrastructureMachineTemplateKind: input.InfrastructureMachineTemplateKind, + InfrastructureMachinePoolTemplateKind: input.InfrastructureMachinePoolTemplateKind, + WorkloadYamlPath: autoscalerWorkloadYAMLPath, + ManagementClusterProxy: input.BootstrapClusterProxy, + WorkloadClusterProxy: workloadClusterProxy, + Cluster: clusterResources.Cluster, + AutoscalerVersion: input.AutoscalerVersion, }, input.E2EConfig.GetIntervals(specName, "wait-controllers")...) By("Creating workload that forces the system to scale up") @@ -161,16 +172,16 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) }, input.E2EConfig.GetIntervals(specName, "wait-autoscaler")...) By("Checking the MachineDeployment is scaled up") - scaledUpReplicas := originalReplicas + 1 + mdScaledUpReplicas := mdOriginalReplicas + 1 framework.AssertMachineDeploymentReplicas(ctx, framework.AssertMachineDeploymentReplicasInput{ Getter: input.BootstrapClusterProxy.GetClient(), MachineDeployment: clusterResources.MachineDeployments[0], - Replicas: scaledUpReplicas, + Replicas: mdScaledUpReplicas, WaitForMachineDeployment: input.E2EConfig.GetIntervals(specName, "wait-autoscaler"), }) By("Disabling the autoscaler") - framework.DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.DisableAutoscalerForMachineDeploymentTopologyAndWaitInput{ + framework.DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.DisableAutoscalerForMachineTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, WaitForAnnotationsToBeDropped: input.E2EConfig.GetIntervals(specName, "wait-controllers"), @@ -178,21 +189,21 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) By("Checking we can manually scale up the MachineDeployment") // Scale up the MachineDeployment. Since autoscaler is disabled we should be able to do this. - excessReplicas := scaledUpReplicas + 1 + mdExcessReplicas := mdScaledUpReplicas + 1 framework.ScaleAndWaitMachineDeploymentTopology(ctx, framework.ScaleAndWaitMachineDeploymentTopologyInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, - Replicas: excessReplicas, + Replicas: mdExcessReplicas, WaitForMachineDeployments: input.E2EConfig.GetIntervals(specName, "wait-worker-nodes"), }) By("Checking enabling autoscaler will scale down the MachineDeployment to correct size") // Enable autoscaler on the MachineDeployment. - framework.EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.EnableAutoscalerForMachineDeploymentTopologyAndWaitInput{ + framework.EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.EnableAutoscalerForMachineTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, - NodeGroupMinSize: nodeGroupMinSize, - NodeGroupMaxSize: nodeGroupMaxSize, + NodeGroupMinSize: mdNodeGroupMinSize, + NodeGroupMaxSize: mdNodeGroupMaxSize, WaitForAnnotationsToBeAdded: input.E2EConfig.GetIntervals(specName, "wait-autoscaler"), }) @@ -202,10 +213,85 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) framework.AssertMachineDeploymentReplicas(ctx, framework.AssertMachineDeploymentReplicasInput{ Getter: input.BootstrapClusterProxy.GetClient(), MachineDeployment: clusterResources.MachineDeployments[0], - Replicas: scaledUpReplicas, + Replicas: mdScaledUpReplicas, WaitForMachineDeployment: input.E2EConfig.GetIntervals(specName, "wait-controllers"), }) + By("Disabling the autoscaler for MachineDeployments to test MachinePools") + framework.DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.DisableAutoscalerForMachineTopologyAndWaitInput{ + ClusterProxy: input.BootstrapClusterProxy, + Cluster: clusterResources.Cluster, + WaitForAnnotationsToBeDropped: input.E2EConfig.GetIntervals(specName, "wait-controllers"), + }) + + By("Deleting the MachineDeployment scale up deployment") + framework.DeleteScaleUpDeploymentAndWait(ctx, framework.DeleteScaleUpDeploymentAndWaitInput{ + ClusterProxy: workloadClusterProxy, + WaitForDelete: input.E2EConfig.GetIntervals(specName, "wait-autoscaler"), + }) + + By("Enabling autoscaler for the MachinePool") + // Enable autoscaler on the MachinePool. + framework.EnableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.EnableAutoscalerForMachineTopologyAndWaitInput{ + ClusterProxy: input.BootstrapClusterProxy, + Cluster: clusterResources.Cluster, + NodeGroupMinSize: mpNodeGroupMinSize, + NodeGroupMaxSize: mpNodeGroupMaxSize, + WaitForAnnotationsToBeAdded: input.E2EConfig.GetIntervals(specName, "wait-autoscaler"), + }) + + By("Creating workload that forces the system to scale up") + framework.AddScaleUpDeploymentAndWait(ctx, framework.AddScaleUpDeploymentAndWaitInput{ + ClusterProxy: workloadClusterProxy, + Name: "mp-scale-up", + }, input.E2EConfig.GetIntervals(specName, "wait-autoscaler")...) + + By("Checking the MachinePool is scaled up") + mpScaledUpReplicas := mpOriginalReplicas + 1 + framework.AssertMachinePoolReplicas(ctx, framework.AssertMachinePoolReplicasInput{ + Getter: input.BootstrapClusterProxy.GetClient(), + MachinePool: clusterResources.MachinePools[0], + Replicas: mpScaledUpReplicas, + WaitForMachinePool: input.E2EConfig.GetIntervals(specName, "wait-autoscaler"), + }) + + By("Disabling the autoscaler") + framework.DisableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.DisableAutoscalerForMachineTopologyAndWaitInput{ + ClusterProxy: input.BootstrapClusterProxy, + Cluster: clusterResources.Cluster, + WaitForAnnotationsToBeDropped: input.E2EConfig.GetIntervals(specName, "wait-controllers"), + }) + + By("Checking we can manually scale up the MachinePool") + // Scale up the MachinePool. Since autoscaler is disabled we should be able to do this. + mpExcessReplicas := mpScaledUpReplicas + 1 + framework.ScaleMachinePoolTopologyAndWait(ctx, framework.ScaleMachinePoolTopologyAndWaitInput{ + ClusterProxy: input.BootstrapClusterProxy, + Cluster: clusterResources.Cluster, + Replicas: mpExcessReplicas, + WaitForMachinePools: input.E2EConfig.GetIntervals(specName, "wait-worker-nodes"), + }) + + By("Checking enabling autoscaler will scale down the MachinePool to correct size") + // Enable autoscaler on the MachinePool. + framework.EnableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.EnableAutoscalerForMachineTopologyAndWaitInput{ + ClusterProxy: input.BootstrapClusterProxy, + Cluster: clusterResources.Cluster, + NodeGroupMinSize: mpNodeGroupMinSize, + NodeGroupMaxSize: mpNodeGroupMaxSize, + WaitForAnnotationsToBeAdded: input.E2EConfig.GetIntervals(specName, "wait-autoscaler"), + }) + + By("Checking the MachinePool is scaled down") + // Since we scaled up the MachinePool manually and the workload has not changed auto scaler + // should detect that there are unneeded nodes and scale down the MachinePool. + framework.AssertMachinePoolReplicas(ctx, framework.AssertMachinePoolReplicasInput{ + Getter: input.BootstrapClusterProxy.GetClient(), + MachinePool: clusterResources.MachinePools[0], + Replicas: mpScaledUpReplicas, + WaitForMachinePool: input.E2EConfig.GetIntervals(specName, "wait-controllers"), + }) + By("PASSED!") }) diff --git a/test/e2e/autoscaler_test.go b/test/e2e/autoscaler_test.go index 1654f7f26505..f18c6a9a1275 100644 --- a/test/e2e/autoscaler_test.go +++ b/test/e2e/autoscaler_test.go @@ -27,15 +27,16 @@ import ( var _ = Describe("When using the autoscaler with Cluster API using ClusterClass [ClusterClass]", func() { AutoscalerSpec(ctx, func() AutoscalerSpecInput { return AutoscalerSpecInput{ - E2EConfig: e2eConfig, - ClusterctlConfigPath: clusterctlConfigPath, - BootstrapClusterProxy: bootstrapClusterProxy, - ArtifactFolder: artifactFolder, - SkipCleanup: skipCleanup, - InfrastructureProvider: ptr.To("docker"), - InfrastructureMachineTemplateKind: "dockermachinetemplates", - Flavor: ptr.To("topology-autoscaler"), - AutoscalerVersion: "v1.29.0", + E2EConfig: e2eConfig, + ClusterctlConfigPath: clusterctlConfigPath, + BootstrapClusterProxy: bootstrapClusterProxy, + ArtifactFolder: artifactFolder, + SkipCleanup: skipCleanup, + InfrastructureProvider: ptr.To("docker"), + InfrastructureMachineTemplateKind: "dockermachinetemplates", + InfrastructureMachinePoolTemplateKind: "dockermachinepooltemplates", + Flavor: ptr.To("topology-autoscaler"), + AutoscalerVersion: "v1.29.0", } }) }) diff --git a/test/e2e/data/infrastructure-docker/main/cluster-template-topology-autoscaler/cluster-autoscaler.yaml b/test/e2e/data/infrastructure-docker/main/cluster-template-topology-autoscaler/cluster-autoscaler.yaml index 5b76d8c0c340..36466c480d94 100644 --- a/test/e2e/data/infrastructure-docker/main/cluster-template-topology-autoscaler/cluster-autoscaler.yaml +++ b/test/e2e/data/infrastructure-docker/main/cluster-template-topology-autoscaler/cluster-autoscaler.yaml @@ -29,6 +29,12 @@ spec: annotations: cluster.x-k8s.io/cluster-api-autoscaler-node-group-max-size: "5" cluster.x-k8s.io/cluster-api-autoscaler-node-group-min-size: "2" + machinePools: + - class: "default-worker" + name: "mp-0" + nodeDeletionTimeout: "30s" + failureDomains: + - fd4 variables: - name: etcdImageTag value: "" diff --git a/test/framework/autoscaler_helpers.go b/test/framework/autoscaler_helpers.go index 067e6504e7b1..10d3ae27bfcb 100644 --- a/test/framework/autoscaler_helpers.go +++ b/test/framework/autoscaler_helpers.go @@ -26,6 +26,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" authenticationv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" @@ -47,8 +48,9 @@ import ( type ApplyAutoscalerToWorkloadClusterInput struct { ClusterctlConfigPath string - ArtifactFolder string - InfrastructureMachineTemplateKind string + ArtifactFolder string + InfrastructureMachineTemplateKind string + InfrastructureMachinePoolTemplateKind string // WorkloadYamlPath should point the yaml that will be applied on the workload cluster. // The YAML file should: // - Be creating the autoscaler deployment in the workload cluster @@ -89,7 +91,7 @@ func ApplyAutoscalerToWorkloadCluster(ctx context.Context, input ApplyAutoscaler // This address should be accessible from the workload cluster. serverAddr, mgtClusterCA := getServerAddrAndCA(ctx, input.ManagementClusterProxy) // Generate a token with the required permission that can be used by the autoscaler. - token := getAuthenticationTokenForAutoscaler(ctx, input.ManagementClusterProxy, input.Cluster.Namespace, input.Cluster.Name, input.InfrastructureMachineTemplateKind) + token := getAuthenticationTokenForAutoscaler(ctx, input.ManagementClusterProxy, input.Cluster.Namespace, input.Cluster.Name, input.InfrastructureMachineTemplateKind, input.InfrastructureMachinePoolTemplateKind) workloadYaml, err := ProcessYAML(&ProcessYAMLInput{ Template: workloadYamlTemplate, @@ -133,6 +135,7 @@ func ApplyAutoscalerToWorkloadCluster(ctx context.Context, input ApplyAutoscaler // AddScaleUpDeploymentAndWaitInput is the input for AddScaleUpDeploymentAndWait. type AddScaleUpDeploymentAndWaitInput struct { ClusterProxy ClusterProxy + Name string } // AddScaleUpDeploymentAndWait create a deployment that will trigger the autoscaler to scale up and create a new machine. @@ -163,26 +166,30 @@ func AddScaleUpDeploymentAndWait(ctx context.Context, input AddScaleUpDeployment replicas := workers + 1 memoryRequired := int64(float64(memory.Value()) * 0.6) podMemory := resource.NewQuantity(memoryRequired, resource.BinarySI) + deploymentName := "scale-up" + if input.Name != "" { + deploymentName = input.Name + } scaleUpDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "scale-up", + Name: deploymentName, Namespace: metav1.NamespaceDefault, Labels: map[string]string{ - "app": "scale-up", + "app": deploymentName, }, }, Spec: appsv1.DeploymentSpec{ Replicas: ptr.To[int32](int32(replicas)), Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": "scale-up", + "app": deploymentName, }, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "app": "scale-up", + "app": deploymentName, }, }, Spec: corev1.PodSpec{ @@ -213,6 +220,37 @@ func AddScaleUpDeploymentAndWait(ctx context.Context, input AddScaleUpDeployment }, intervals...) } +// DeleteScaleUpDeploymentAndWaitInput is the input for DeleteScaleUpDeploymentAndWait. +type DeleteScaleUpDeploymentAndWaitInput struct { + ClusterProxy ClusterProxy + Name string + WaitForDelete []interface{} +} + +// DeleteScaleUpDeploymentAndWait deletes the scale up deployment and waits for it to be deleted. +func DeleteScaleUpDeploymentAndWait(ctx context.Context, input DeleteScaleUpDeploymentAndWaitInput) { + By("Retrieving the scale up deployment") + deployment := &appsv1.Deployment{} + deploymentName := "scale-up" + if input.Name != "" { + deploymentName = input.Name + } + Expect(input.ClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: metav1.NamespaceDefault}, deployment)).To(Succeed(), "failed to get the scale up pod") + + By("Deleting the scale up deployment") + Expect(input.ClusterProxy.GetClient().Delete(ctx, deployment)).To(Succeed(), "failed to delete the scale up pod") + + By("Waiting for the scale up deployment to be deleted") + deploymentList := &appsv1.DeploymentList{} + Eventually(func() error { + Expect(input.ClusterProxy.GetClient().List(ctx, deploymentList, client.InNamespace(metav1.NamespaceDefault))).To(Succeed(), "failed to list deployments") + if len(deploymentList.Items) != 0 { + return errors.Errorf("expected no deployments for %s, found %d", deploymentName, len(deploymentList.Items)) + } + return nil + }, input.WaitForDelete...).Should(BeNil()) +} + type ProcessYAMLInput struct { Template []byte ClusterctlConfigPath string @@ -249,7 +287,7 @@ func ProcessYAML(input *ProcessYAMLInput) ([]byte, error) { return out, nil } -type DisableAutoscalerForMachineDeploymentTopologyAndWaitInput struct { +type DisableAutoscalerForMachineTopologyAndWaitInput struct { ClusterProxy ClusterProxy Cluster *clusterv1.Cluster WaitForAnnotationsToBeDropped []interface{} @@ -258,7 +296,7 @@ type DisableAutoscalerForMachineDeploymentTopologyAndWaitInput struct { // DisableAutoscalerForMachineDeploymentTopologyAndWait drop the autoscaler annotations from the MachineDeploymentTopology // and waits till the annotations are dropped from the underlying MachineDeployment. It also verifies that the replicas // fields of the MachineDeployments are not affected after the annotations are dropped. -func DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input DisableAutoscalerForMachineDeploymentTopologyAndWaitInput) { +func DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input DisableAutoscalerForMachineTopologyAndWaitInput) { Expect(ctx).NotTo(BeNil(), "ctx is required for DisableAutoscalerForMachineDeploymentTopologyAndWait") Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling DisableAutoscalerForMachineDeploymentTopologyAndWait") @@ -305,7 +343,7 @@ func DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, i }, input.WaitForAnnotationsToBeDropped...).Should(Succeed(), "Auto scaler annotations are not dropped or replicas changed for the MachineDeployments") } -type EnableAutoscalerForMachineDeploymentTopologyAndWaitInput struct { +type EnableAutoscalerForMachineTopologyAndWaitInput struct { ClusterProxy ClusterProxy Cluster *clusterv1.Cluster NodeGroupMinSize string @@ -313,7 +351,7 @@ type EnableAutoscalerForMachineDeploymentTopologyAndWaitInput struct { WaitForAnnotationsToBeAdded []interface{} } -func EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input EnableAutoscalerForMachineDeploymentTopologyAndWaitInput) { +func EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input EnableAutoscalerForMachineTopologyAndWaitInput) { Expect(ctx).NotTo(BeNil(), "ctx is required for EnableAutoscalerForMachineDeploymentTopologyAndWait") Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling EnableAutoscalerForMachineDeploymentTopologyAndWait") @@ -353,9 +391,99 @@ func EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, in }, input.WaitForAnnotationsToBeAdded...).Should(Succeed(), "Auto scaler annotations are missing from the MachineDeployments") } +// DisableAutoscalerForMachinePoolTopologyAndWait drop the autoscaler annotations from the MachinePoolTopology +// and waits till the annotations are dropped from the underlying MachinePool. It also verifies that the replicas +// fields of the MachinePools are not affected after the annotations are dropped. +func DisableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input DisableAutoscalerForMachineTopologyAndWaitInput) { + Expect(ctx).NotTo(BeNil(), "ctx is required for DisableAutoscalerForMachinePoolTopologyAndWait") + Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling DisableAutoscalerForMachinePoolTopologyAndWait") + + mgmtClient := input.ClusterProxy.GetClient() + + // Get the current replicas of the MachinePools. + replicas := map[string]*int32{} + mpList := GetMachinePoolsByCluster(ctx, GetMachinePoolsByClusterInput{ + Lister: mgmtClient, + ClusterName: input.Cluster.Name, + Namespace: input.Cluster.Namespace, + }) + for _, mp := range mpList { + replicas[mp.Name] = mp.Spec.Replicas + } + + log.Logf("Dropping the %s and %s annotations from the MachinePools in ClusterTopology", clusterv1.AutoscalerMinSizeAnnotation, clusterv1.AutoscalerMaxSizeAnnotation) + patchHelper, err := patch.NewHelper(input.Cluster, mgmtClient) + Expect(err).ToNot(HaveOccurred()) + for i := range input.Cluster.Spec.Topology.Workers.MachinePools { + mp := input.Cluster.Spec.Topology.Workers.MachinePools[i] + delete(mp.Metadata.Annotations, clusterv1.AutoscalerMinSizeAnnotation) + delete(mp.Metadata.Annotations, clusterv1.AutoscalerMaxSizeAnnotation) + input.Cluster.Spec.Topology.Workers.MachinePools[i] = mp + } + Eventually(func(g Gomega) { + g.Expect(patchHelper.Patch(ctx, input.Cluster)).Should(Succeed()) + }, retryableOperationTimeout, retryableOperationInterval).Should(Succeed(), "Failed to patch Cluster topology to drop autoscaler annotations") + + log.Logf("Wait for the annotations to be dropped from the MachinePools") + Eventually(func(g Gomega) { + mpList := GetMachinePoolsByCluster(ctx, GetMachinePoolsByClusterInput{ + Lister: mgmtClient, + ClusterName: input.Cluster.Name, + Namespace: input.Cluster.Namespace, + }) + for i := range mpList { + mp := mpList[i] + g.Expect(mp.Annotations).ToNot(HaveKey(clusterv1.AutoscalerMinSizeAnnotation), fmt.Sprintf("MachinePool %s should not have %s annotation", klog.KObj(mp), clusterv1.AutoscalerMinSizeAnnotation)) + g.Expect(mp.Annotations).ToNot(HaveKey(clusterv1.AutoscalerMaxSizeAnnotation), fmt.Sprintf("MachinePool %s should not have %s annotation", klog.KObj(mp), clusterv1.AutoscalerMaxSizeAnnotation)) + // Verify that disabling auto scaler does not change the current MachinePool replicas. + g.Expect(mp.Spec.Replicas).To(Equal(replicas[mp.Name]), fmt.Sprintf("MachinePool %s replicas should not change after disabling autoscaler", klog.KObj(mp))) + } + }, input.WaitForAnnotationsToBeDropped...).Should(Succeed(), "Auto scaler annotations are not dropped or replicas changed for the MachinePools") +} + +func EnableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input EnableAutoscalerForMachineTopologyAndWaitInput) { + Expect(ctx).NotTo(BeNil(), "ctx is required for EnableAutoscalerForMachinePoolTopologyAndWait") + Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling EnableAutoscalerForMachinePoolTopologyAndWait") + + mgmtClient := input.ClusterProxy.GetClient() + + log.Logf("Add the %s and %s annotations to the MachinePools in ClusterTopology", clusterv1.AutoscalerMinSizeAnnotation, clusterv1.AutoscalerMaxSizeAnnotation) + patchHelper, err := patch.NewHelper(input.Cluster, mgmtClient) + Expect(err).ToNot(HaveOccurred()) + for i := range input.Cluster.Spec.Topology.Workers.MachinePools { + mp := input.Cluster.Spec.Topology.Workers.MachinePools[i] + if mp.Metadata.Annotations == nil { + mp.Metadata.Annotations = map[string]string{} + } + // Add the autoscaler annotation + mp.Metadata.Annotations[clusterv1.AutoscalerMinSizeAnnotation] = input.NodeGroupMinSize + mp.Metadata.Annotations[clusterv1.AutoscalerMaxSizeAnnotation] = input.NodeGroupMaxSize + // Drop the replicas from MachinePoolTopology, or else the topology controller and autoscaler with fight over control. + mp.Replicas = nil + input.Cluster.Spec.Topology.Workers.MachinePools[i] = mp + } + Eventually(func(g Gomega) { + g.Expect(patchHelper.Patch(ctx, input.Cluster)).Should(Succeed()) + }, retryableOperationTimeout, retryableOperationInterval).Should(Succeed(), "Failed to patch Cluster topology to add autoscaler annotations") + + log.Logf("Wait for the annotations to applied on the MachinePools") + Eventually(func(g Gomega) { + mpList := GetMachinePoolsByCluster(ctx, GetMachinePoolsByClusterInput{ + Lister: mgmtClient, + ClusterName: input.Cluster.Name, + Namespace: input.Cluster.Namespace, + }) + for i := range mpList { + mp := mpList[i] + g.Expect(mp.Annotations).To(HaveKey(clusterv1.AutoscalerMinSizeAnnotation), fmt.Sprintf("MachinePool %s should have %s annotation", klog.KObj(mp), clusterv1.AutoscalerMinSizeAnnotation)) + g.Expect(mp.Annotations).To(HaveKey(clusterv1.AutoscalerMaxSizeAnnotation), fmt.Sprintf("MachinePool %s should have %s annotation", klog.KObj(mp), clusterv1.AutoscalerMaxSizeAnnotation)) + } + }, input.WaitForAnnotationsToBeAdded...).Should(Succeed(), "Auto scaler annotations are missing from the MachinePools") +} + // getAuthenticationTokenForAutoscaler returns a bearer authenticationToken with minimal RBAC permissions that will be used // by the autoscaler running on the workload cluster to access the management cluster. -func getAuthenticationTokenForAutoscaler(ctx context.Context, managementClusterProxy ClusterProxy, namespace string, cluster string, infraMachineTemplateKind string) string { +func getAuthenticationTokenForAutoscaler(ctx context.Context, managementClusterProxy ClusterProxy, namespace string, cluster string, infraMachineTemplateKind string, infraMachinePoolTemplateKind string) string { name := fmt.Sprintf("cluster-%s", cluster) sa := &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ @@ -379,7 +507,7 @@ func getAuthenticationTokenForAutoscaler(ctx context.Context, managementClusterP { Verbs: []string{"get", "list"}, APIGroups: []string{"infrastructure.cluster.x-k8s.io"}, - Resources: []string{infraMachineTemplateKind}, + Resources: []string{infraMachineTemplateKind, infraMachinePoolTemplateKind, "dockermachinepools"}, }, }, } diff --git a/test/framework/machinepool_helpers.go b/test/framework/machinepool_helpers.go index 11ff4ca96c46..7fdcb556d7d1 100644 --- a/test/framework/machinepool_helpers.go +++ b/test/framework/machinepool_helpers.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -201,6 +202,46 @@ func ScaleMachinePoolAndWait(ctx context.Context, input ScaleMachinePoolAndWaitI } } +type ScaleMachinePoolTopologyAndWaitInput struct { + ClusterProxy ClusterProxy + Cluster *clusterv1.Cluster + Replicas int32 + WaitForMachinePools []interface{} +} + +// ScaleMachinePoolTopologyAndWait scales a machine pool and waits for its instances to scale up. +func ScaleMachinePoolTopologyAndWait(ctx context.Context, input ScaleMachinePoolTopologyAndWaitInput) { + Expect(ctx).NotTo(BeNil(), "ctx is required for UpgradeMachinePoolAndWait") + Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling UpgradeMachinePoolAndWait") + Expect(input.Cluster).ToNot(BeNil(), "Invalid argument. input.Cluster can't be nil when calling UpgradeMachinePoolAndWait") + + mpTopology := input.Cluster.Spec.Topology.Workers.MachinePools[0] + if mpTopology.Replicas != nil { + log.Logf("Scaling machine pool topology %s from %d to %d replicas", mpTopology.Name, *mpTopology.Replicas, input.Replicas) + } else { + log.Logf("Scaling machine pool topology %s to %d replicas", mpTopology.Name, input.Replicas) + } + patchHelper, err := patch.NewHelper(input.Cluster, input.ClusterProxy.GetClient()) + Expect(err).ToNot(HaveOccurred()) + mpTopology.Replicas = ptr.To[int32](input.Replicas) + input.Cluster.Spec.Topology.Workers.MachinePools[0] = mpTopology + Eventually(func() error { + return patchHelper.Patch(ctx, input.Cluster) + }, retryableOperationTimeout, retryableOperationInterval).Should(Succeed(), "Failed to scale machine pool topology %s", mpTopology.Name) + + log.Logf("Waiting for correct number of replicas to exist") + mpList := &expv1.MachinePoolList{} + Eventually(func() error { + return input.ClusterProxy.GetClient().List(ctx, mpList, + client.InNamespace(input.Cluster.Namespace), + client.MatchingLabels{ + clusterv1.ClusterNameLabel: input.Cluster.Name, + clusterv1.ClusterTopologyMachinePoolNameLabel: mpTopology.Name, + }, + ) + }, retryableOperationTimeout, retryableOperationInterval).Should(Succeed(), "Failed to list MachinePools object for Cluster %s", klog.KRef(input.Cluster.Namespace, input.Cluster.Name)) +} + // WaitForMachinePoolInstancesToBeUpgradedInput is the input for WaitForMachinePoolInstancesToBeUpgraded. type WaitForMachinePoolInstancesToBeUpgradedInput struct { Getter Getter @@ -289,3 +330,28 @@ func getMachinePoolInstanceVersions(ctx context.Context, input GetMachinesPoolIn return versions } + +type AssertMachinePoolReplicasInput struct { + Getter Getter + MachinePool *expv1.MachinePool + Replicas int32 + WaitForMachinePool []interface{} +} + +func AssertMachinePoolReplicas(ctx context.Context, input AssertMachinePoolReplicasInput) { + Expect(ctx).NotTo(BeNil(), "ctx is required for AssertMachinePoolReplicas") + Expect(input.Getter).ToNot(BeNil(), "Invalid argument. input.Getter can't be nil when calling AssertMachinePoolReplicas") + Expect(input.MachinePool).ToNot(BeNil(), "Invalid argument. input.MachinePool can't be nil when calling AssertMachinePoolReplicas") + + Eventually(func(g Gomega) { + // Get the MachinePool + mp := &expv1.MachinePool{} + key := client.ObjectKey{ + Namespace: input.MachinePool.Namespace, + Name: input.MachinePool.Name, + } + g.Expect(input.Getter.Get(ctx, key, mp)).To(Succeed(), fmt.Sprintf("failed to get MachinePool %s", klog.KObj(input.MachinePool))) + g.Expect(mp.Spec.Replicas).Should(Not(BeNil()), fmt.Sprintf("MachinePool %s replicas should not be nil", klog.KObj(mp))) + g.Expect(*mp.Spec.Replicas).Should(Equal(input.Replicas), fmt.Sprintf("MachinePool %s replicas should match expected replicas", klog.KObj(mp))) + }, input.WaitForMachinePool...).Should(Succeed()) +} From 4c61ef608b309908c5b4b258d6b37886fc44b439 Mon Sep 17 00:00:00 2001 From: willie-yao Date: Fri, 5 Apr 2024 21:16:01 +0000 Subject: [PATCH 2/2] Reviews --- exp/internal/webhooks/machinepool_test.go | 3 +- test/e2e/autoscaler.go | 29 ++++++---- test/e2e/autoscaler_test.go | 1 + test/framework/autoscaler_helpers.go | 64 +++++++++++++---------- test/framework/machinepool_helpers.go | 16 ++++-- 5 files changed, 70 insertions(+), 43 deletions(-) diff --git a/exp/internal/webhooks/machinepool_test.go b/exp/internal/webhooks/machinepool_test.go index 0f2c54fa1f82..9d1af2607ed2 100644 --- a/exp/internal/webhooks/machinepool_test.go +++ b/exp/internal/webhooks/machinepool_test.go @@ -35,7 +35,7 @@ import ( "sigs.k8s.io/cluster-api/internal/webhooks/util" ) -var ctx = admission.NewContextWithRequest(ctrl.SetupSignalHandler(), admission.Request{}) +var ctx = ctrl.SetupSignalHandler() func TestMachinePoolDefault(t *testing.T) { // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. @@ -58,6 +58,7 @@ func TestMachinePoolDefault(t *testing.T) { }, } webhook := &MachinePool{} + ctx = admission.NewContextWithRequest(ctx, admission.Request{}) t.Run("for MachinePool", util.CustomDefaultValidateTest(ctx, mp, webhook)) g.Expect(webhook.Default(ctx, mp)).To(Succeed()) diff --git a/test/e2e/autoscaler.go b/test/e2e/autoscaler.go index 9e55aad11c14..27130b904677 100644 --- a/test/e2e/autoscaler.go +++ b/test/e2e/autoscaler.go @@ -61,6 +61,7 @@ type AutoscalerSpecInput struct { // Example: dockermachinetemplates. InfrastructureMachineTemplateKind string InfrastructureMachinePoolTemplateKind string + InfrastructureMachinePoolKind string AutoscalerVersion string // Allows to inject a function to be run after test namespace is created. @@ -72,7 +73,9 @@ type AutoscalerSpecInput struct { // being deployed in the workload cluster. func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) { var ( - specName = "autoscaler" + specName = "autoscaler" + // We need to set the min size to 1 because the MachinePool is initially created without the + // annotations and the replicas field set. The MachinePool webhook will then set 1 in that case. mpNodeGroupMinSize = "1" mpNodeGroupMaxSize = "5" input AutoscalerSpecInput @@ -144,14 +147,19 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) // Ensure the MachinePoolTopology does NOT have the autoscaler annotations so we can test MachineDeployments first. mpTopology := clusterResources.Cluster.Spec.Topology.Workers.MachinePools[0] - Expect(mpTopology.Metadata.Annotations).To(BeNil(), "MachinePool is expected to have autoscaler annotations") + if mpTopology.Metadata.Annotations != nil { + _, ok = mpTopology.Metadata.Annotations[clusterv1.AutoscalerMinSizeAnnotation] + Expect(ok).To(BeFalse(), "MachinePoolTopology %s does have the %q autoscaler annotation", mpTopology.Name, clusterv1.AutoscalerMinSizeAnnotation) + _, ok = mpTopology.Metadata.Annotations[clusterv1.AutoscalerMaxSizeAnnotation] + Expect(ok).To(BeFalse(), "MachinePoolTopology %s does have the %q autoscaler annotation", mpTopology.Name, clusterv1.AutoscalerMaxSizeAnnotation) + } // Get a ClusterProxy so we can interact with the workload cluster workloadClusterProxy := input.BootstrapClusterProxy.GetWorkloadCluster(ctx, clusterResources.Cluster.Namespace, clusterResources.Cluster.Name) mdOriginalReplicas := *clusterResources.MachineDeployments[0].Spec.Replicas Expect(strconv.Itoa(int(mdOriginalReplicas))).To(Equal(mdNodeGroupMinSize), "MachineDeployment should have replicas as defined in %s", clusterv1.AutoscalerMinSizeAnnotation) mpOriginalReplicas := *clusterResources.MachinePools[0].Spec.Replicas - Expect(strconv.Itoa(int(mpOriginalReplicas))).To(Equal(mpNodeGroupMinSize), "MachinePool should have replicas as defined in %s", clusterv1.AutoscalerMinSizeAnnotation) + Expect(int(mpOriginalReplicas)).To(Equal(1), "MachinePool should default to 1 replica via the MachinePool webhook") By("Installing the autoscaler on the workload cluster") autoscalerWorkloadYAMLPath := input.E2EConfig.GetVariable(AutoscalerWorkloadYAMLPath) @@ -159,6 +167,7 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) ArtifactFolder: input.ArtifactFolder, InfrastructureMachineTemplateKind: input.InfrastructureMachineTemplateKind, InfrastructureMachinePoolTemplateKind: input.InfrastructureMachinePoolTemplateKind, + InfrastructureMachinePoolKind: input.InfrastructureMachinePoolKind, WorkloadYamlPath: autoscalerWorkloadYAMLPath, ManagementClusterProxy: input.BootstrapClusterProxy, WorkloadClusterProxy: workloadClusterProxy, @@ -181,7 +190,7 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) }) By("Disabling the autoscaler") - framework.DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.DisableAutoscalerForMachineTopologyAndWaitInput{ + framework.DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.DisableAutoscalerForMachineDeploymentTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, WaitForAnnotationsToBeDropped: input.E2EConfig.GetIntervals(specName, "wait-controllers"), @@ -199,7 +208,7 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) By("Checking enabling autoscaler will scale down the MachineDeployment to correct size") // Enable autoscaler on the MachineDeployment. - framework.EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.EnableAutoscalerForMachineTopologyAndWaitInput{ + framework.EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.EnableAutoscalerForMachineDeploymentTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, NodeGroupMinSize: mdNodeGroupMinSize, @@ -218,7 +227,7 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) }) By("Disabling the autoscaler for MachineDeployments to test MachinePools") - framework.DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.DisableAutoscalerForMachineTopologyAndWaitInput{ + framework.DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx, framework.DisableAutoscalerForMachineDeploymentTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, WaitForAnnotationsToBeDropped: input.E2EConfig.GetIntervals(specName, "wait-controllers"), @@ -232,7 +241,7 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) By("Enabling autoscaler for the MachinePool") // Enable autoscaler on the MachinePool. - framework.EnableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.EnableAutoscalerForMachineTopologyAndWaitInput{ + framework.EnableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.EnableAutoscalerForMachinePoolTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, NodeGroupMinSize: mpNodeGroupMinSize, @@ -243,7 +252,6 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) By("Creating workload that forces the system to scale up") framework.AddScaleUpDeploymentAndWait(ctx, framework.AddScaleUpDeploymentAndWaitInput{ ClusterProxy: workloadClusterProxy, - Name: "mp-scale-up", }, input.E2EConfig.GetIntervals(specName, "wait-autoscaler")...) By("Checking the MachinePool is scaled up") @@ -256,7 +264,7 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) }) By("Disabling the autoscaler") - framework.DisableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.DisableAutoscalerForMachineTopologyAndWaitInput{ + framework.DisableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.DisableAutoscalerForMachinePoolTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, WaitForAnnotationsToBeDropped: input.E2EConfig.GetIntervals(specName, "wait-controllers"), @@ -270,11 +278,12 @@ func AutoscalerSpec(ctx context.Context, inputGetter func() AutoscalerSpecInput) Cluster: clusterResources.Cluster, Replicas: mpExcessReplicas, WaitForMachinePools: input.E2EConfig.GetIntervals(specName, "wait-worker-nodes"), + Getter: input.BootstrapClusterProxy.GetClient(), }) By("Checking enabling autoscaler will scale down the MachinePool to correct size") // Enable autoscaler on the MachinePool. - framework.EnableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.EnableAutoscalerForMachineTopologyAndWaitInput{ + framework.EnableAutoscalerForMachinePoolTopologyAndWait(ctx, framework.EnableAutoscalerForMachinePoolTopologyAndWaitInput{ ClusterProxy: input.BootstrapClusterProxy, Cluster: clusterResources.Cluster, NodeGroupMinSize: mpNodeGroupMinSize, diff --git a/test/e2e/autoscaler_test.go b/test/e2e/autoscaler_test.go index f18c6a9a1275..a1c20b432a37 100644 --- a/test/e2e/autoscaler_test.go +++ b/test/e2e/autoscaler_test.go @@ -35,6 +35,7 @@ var _ = Describe("When using the autoscaler with Cluster API using ClusterClass InfrastructureProvider: ptr.To("docker"), InfrastructureMachineTemplateKind: "dockermachinetemplates", InfrastructureMachinePoolTemplateKind: "dockermachinepooltemplates", + InfrastructureMachinePoolKind: "dockermachinepools", Flavor: ptr.To("topology-autoscaler"), AutoscalerVersion: "v1.29.0", } diff --git a/test/framework/autoscaler_helpers.go b/test/framework/autoscaler_helpers.go index 10d3ae27bfcb..c1f3f5097080 100644 --- a/test/framework/autoscaler_helpers.go +++ b/test/framework/autoscaler_helpers.go @@ -26,11 +26,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" authenticationv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/clientcmd" @@ -51,6 +51,7 @@ type ApplyAutoscalerToWorkloadClusterInput struct { ArtifactFolder string InfrastructureMachineTemplateKind string InfrastructureMachinePoolTemplateKind string + InfrastructureMachinePoolKind string // WorkloadYamlPath should point the yaml that will be applied on the workload cluster. // The YAML file should: // - Be creating the autoscaler deployment in the workload cluster @@ -91,7 +92,7 @@ func ApplyAutoscalerToWorkloadCluster(ctx context.Context, input ApplyAutoscaler // This address should be accessible from the workload cluster. serverAddr, mgtClusterCA := getServerAddrAndCA(ctx, input.ManagementClusterProxy) // Generate a token with the required permission that can be used by the autoscaler. - token := getAuthenticationTokenForAutoscaler(ctx, input.ManagementClusterProxy, input.Cluster.Namespace, input.Cluster.Name, input.InfrastructureMachineTemplateKind, input.InfrastructureMachinePoolTemplateKind) + token := getAuthenticationTokenForAutoscaler(ctx, input.ManagementClusterProxy, input.Cluster.Namespace, input.Cluster.Name, input.InfrastructureMachineTemplateKind, input.InfrastructureMachinePoolTemplateKind, input.InfrastructureMachinePoolKind) workloadYaml, err := ProcessYAML(&ProcessYAMLInput{ Template: workloadYamlTemplate, @@ -135,7 +136,6 @@ func ApplyAutoscalerToWorkloadCluster(ctx context.Context, input ApplyAutoscaler // AddScaleUpDeploymentAndWaitInput is the input for AddScaleUpDeploymentAndWait. type AddScaleUpDeploymentAndWaitInput struct { ClusterProxy ClusterProxy - Name string } // AddScaleUpDeploymentAndWait create a deployment that will trigger the autoscaler to scale up and create a new machine. @@ -166,30 +166,26 @@ func AddScaleUpDeploymentAndWait(ctx context.Context, input AddScaleUpDeployment replicas := workers + 1 memoryRequired := int64(float64(memory.Value()) * 0.6) podMemory := resource.NewQuantity(memoryRequired, resource.BinarySI) - deploymentName := "scale-up" - if input.Name != "" { - deploymentName = input.Name - } scaleUpDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: deploymentName, + Name: "scale-up", Namespace: metav1.NamespaceDefault, Labels: map[string]string{ - "app": deploymentName, + "app": "scale-up", }, }, Spec: appsv1.DeploymentSpec{ Replicas: ptr.To[int32](int32(replicas)), Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": deploymentName, + "app": "scale-up", }, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "app": deploymentName, + "app": "scale-up", }, }, Spec: corev1.PodSpec{ @@ -235,20 +231,16 @@ func DeleteScaleUpDeploymentAndWait(ctx context.Context, input DeleteScaleUpDepl if input.Name != "" { deploymentName = input.Name } - Expect(input.ClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: metav1.NamespaceDefault}, deployment)).To(Succeed(), "failed to get the scale up pod") + Expect(input.ClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: metav1.NamespaceDefault}, deployment)).To(Succeed(), "failed to get the scale up deployment") By("Deleting the scale up deployment") - Expect(input.ClusterProxy.GetClient().Delete(ctx, deployment)).To(Succeed(), "failed to delete the scale up pod") + Expect(input.ClusterProxy.GetClient().Delete(ctx, deployment)).To(Succeed(), "failed to delete the scale up deployment") By("Waiting for the scale up deployment to be deleted") - deploymentList := &appsv1.DeploymentList{} - Eventually(func() error { - Expect(input.ClusterProxy.GetClient().List(ctx, deploymentList, client.InNamespace(metav1.NamespaceDefault))).To(Succeed(), "failed to list deployments") - if len(deploymentList.Items) != 0 { - return errors.Errorf("expected no deployments for %s, found %d", deploymentName, len(deploymentList.Items)) - } - return nil - }, input.WaitForDelete...).Should(BeNil()) + Eventually(func(g Gomega) { + err := input.ClusterProxy.GetClient().Get(ctx, client.ObjectKey{Name: deploymentName, Namespace: metav1.NamespaceDefault}, deployment) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }, input.WaitForDelete...).Should(Succeed()) } type ProcessYAMLInput struct { @@ -287,7 +279,7 @@ func ProcessYAML(input *ProcessYAMLInput) ([]byte, error) { return out, nil } -type DisableAutoscalerForMachineTopologyAndWaitInput struct { +type DisableAutoscalerForMachineDeploymentTopologyAndWaitInput struct { ClusterProxy ClusterProxy Cluster *clusterv1.Cluster WaitForAnnotationsToBeDropped []interface{} @@ -296,7 +288,7 @@ type DisableAutoscalerForMachineTopologyAndWaitInput struct { // DisableAutoscalerForMachineDeploymentTopologyAndWait drop the autoscaler annotations from the MachineDeploymentTopology // and waits till the annotations are dropped from the underlying MachineDeployment. It also verifies that the replicas // fields of the MachineDeployments are not affected after the annotations are dropped. -func DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input DisableAutoscalerForMachineTopologyAndWaitInput) { +func DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input DisableAutoscalerForMachineDeploymentTopologyAndWaitInput) { Expect(ctx).NotTo(BeNil(), "ctx is required for DisableAutoscalerForMachineDeploymentTopologyAndWait") Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling DisableAutoscalerForMachineDeploymentTopologyAndWait") @@ -343,7 +335,7 @@ func DisableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, i }, input.WaitForAnnotationsToBeDropped...).Should(Succeed(), "Auto scaler annotations are not dropped or replicas changed for the MachineDeployments") } -type EnableAutoscalerForMachineTopologyAndWaitInput struct { +type EnableAutoscalerForMachineDeploymentTopologyAndWaitInput struct { ClusterProxy ClusterProxy Cluster *clusterv1.Cluster NodeGroupMinSize string @@ -351,7 +343,7 @@ type EnableAutoscalerForMachineTopologyAndWaitInput struct { WaitForAnnotationsToBeAdded []interface{} } -func EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input EnableAutoscalerForMachineTopologyAndWaitInput) { +func EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, input EnableAutoscalerForMachineDeploymentTopologyAndWaitInput) { Expect(ctx).NotTo(BeNil(), "ctx is required for EnableAutoscalerForMachineDeploymentTopologyAndWait") Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling EnableAutoscalerForMachineDeploymentTopologyAndWait") @@ -391,10 +383,16 @@ func EnableAutoscalerForMachineDeploymentTopologyAndWait(ctx context.Context, in }, input.WaitForAnnotationsToBeAdded...).Should(Succeed(), "Auto scaler annotations are missing from the MachineDeployments") } +type DisableAutoscalerForMachinePoolTopologyAndWaitInput struct { + ClusterProxy ClusterProxy + Cluster *clusterv1.Cluster + WaitForAnnotationsToBeDropped []interface{} +} + // DisableAutoscalerForMachinePoolTopologyAndWait drop the autoscaler annotations from the MachinePoolTopology // and waits till the annotations are dropped from the underlying MachinePool. It also verifies that the replicas // fields of the MachinePools are not affected after the annotations are dropped. -func DisableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input DisableAutoscalerForMachineTopologyAndWaitInput) { +func DisableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input DisableAutoscalerForMachinePoolTopologyAndWaitInput) { Expect(ctx).NotTo(BeNil(), "ctx is required for DisableAutoscalerForMachinePoolTopologyAndWait") Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling DisableAutoscalerForMachinePoolTopologyAndWait") @@ -441,7 +439,15 @@ func DisableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input D }, input.WaitForAnnotationsToBeDropped...).Should(Succeed(), "Auto scaler annotations are not dropped or replicas changed for the MachinePools") } -func EnableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input EnableAutoscalerForMachineTopologyAndWaitInput) { +type EnableAutoscalerForMachinePoolTopologyAndWaitInput struct { + ClusterProxy ClusterProxy + Cluster *clusterv1.Cluster + NodeGroupMinSize string + NodeGroupMaxSize string + WaitForAnnotationsToBeAdded []interface{} +} + +func EnableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input EnableAutoscalerForMachinePoolTopologyAndWaitInput) { Expect(ctx).NotTo(BeNil(), "ctx is required for EnableAutoscalerForMachinePoolTopologyAndWait") Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling EnableAutoscalerForMachinePoolTopologyAndWait") @@ -483,7 +489,7 @@ func EnableAutoscalerForMachinePoolTopologyAndWait(ctx context.Context, input En // getAuthenticationTokenForAutoscaler returns a bearer authenticationToken with minimal RBAC permissions that will be used // by the autoscaler running on the workload cluster to access the management cluster. -func getAuthenticationTokenForAutoscaler(ctx context.Context, managementClusterProxy ClusterProxy, namespace string, cluster string, infraMachineTemplateKind string, infraMachinePoolTemplateKind string) string { +func getAuthenticationTokenForAutoscaler(ctx context.Context, managementClusterProxy ClusterProxy, namespace string, cluster string, infraMachineTemplateKind, infraMachinePoolTemplateKind, infraMachinePoolKind string) string { name := fmt.Sprintf("cluster-%s", cluster) sa := &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ @@ -507,7 +513,7 @@ func getAuthenticationTokenForAutoscaler(ctx context.Context, managementClusterP { Verbs: []string{"get", "list"}, APIGroups: []string{"infrastructure.cluster.x-k8s.io"}, - Resources: []string{infraMachineTemplateKind, infraMachinePoolTemplateKind, "dockermachinepools"}, + Resources: []string{infraMachineTemplateKind, infraMachinePoolTemplateKind, infraMachinePoolKind}, }, }, } diff --git a/test/framework/machinepool_helpers.go b/test/framework/machinepool_helpers.go index 7fdcb556d7d1..7aaf40fcefd5 100644 --- a/test/framework/machinepool_helpers.go +++ b/test/framework/machinepool_helpers.go @@ -207,13 +207,16 @@ type ScaleMachinePoolTopologyAndWaitInput struct { Cluster *clusterv1.Cluster Replicas int32 WaitForMachinePools []interface{} + Getter Getter } // ScaleMachinePoolTopologyAndWait scales a machine pool and waits for its instances to scale up. func ScaleMachinePoolTopologyAndWait(ctx context.Context, input ScaleMachinePoolTopologyAndWaitInput) { - Expect(ctx).NotTo(BeNil(), "ctx is required for UpgradeMachinePoolAndWait") - Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling UpgradeMachinePoolAndWait") - Expect(input.Cluster).ToNot(BeNil(), "Invalid argument. input.Cluster can't be nil when calling UpgradeMachinePoolAndWait") + Expect(ctx).NotTo(BeNil(), "ctx is required for ScaleMachinePoolTopologyAndWait") + Expect(input.ClusterProxy).ToNot(BeNil(), "Invalid argument. input.ClusterProxy can't be nil when calling ScaleMachinePoolTopologyAndWait") + Expect(input.Cluster).ToNot(BeNil(), "Invalid argument. input.Cluster can't be nil when calling ScaleMachinePoolTopologyAndWait") + Expect(input.Cluster.Spec.Topology.Workers).ToNot(BeNil(), "Invalid argument. input.Cluster must have MachinePool topologies") + Expect(input.Cluster.Spec.Topology.Workers.MachinePools).NotTo(BeEmpty(), "Invalid argument. input.Cluster must have at least one MachinePool topology") mpTopology := input.Cluster.Spec.Topology.Workers.MachinePools[0] if mpTopology.Replicas != nil { @@ -240,6 +243,13 @@ func ScaleMachinePoolTopologyAndWait(ctx context.Context, input ScaleMachinePool }, ) }, retryableOperationTimeout, retryableOperationInterval).Should(Succeed(), "Failed to list MachinePools object for Cluster %s", klog.KRef(input.Cluster.Namespace, input.Cluster.Name)) + + Expect(mpList.Items).To(HaveLen(1)) + mp := mpList.Items[0] + WaitForMachinePoolNodesToExist(ctx, WaitForMachinePoolNodesToExistInput{ + Getter: input.Getter, + MachinePool: &mp, + }, input.WaitForMachinePools...) } // WaitForMachinePoolInstancesToBeUpgradedInput is the input for WaitForMachinePoolInstancesToBeUpgraded.