Skip to content

Commit

Permalink
Reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
@willie-yao
willie-yao committed Apr 5, 2024
1 parent becac55 commit 4c61ef6
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 43 deletions.
3 changes: 2 additions & 1 deletion exp/internal/webhooks/machinepool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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())

Expand Down
29 changes: 19 additions & 10 deletions test/e2e/autoscaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -144,21 +147,27 @@ 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)
framework.ApplyAutoscalerToWorkloadCluster(ctx, framework.ApplyAutoscalerToWorkloadClusterInput{
ArtifactFolder: input.ArtifactFolder,
InfrastructureMachineTemplateKind: input.InfrastructureMachineTemplateKind,
InfrastructureMachinePoolTemplateKind: input.InfrastructureMachinePoolTemplateKind,
InfrastructureMachinePoolKind: input.InfrastructureMachinePoolKind,
WorkloadYamlPath: autoscalerWorkloadYAMLPath,
ManagementClusterProxy: input.BootstrapClusterProxy,
WorkloadClusterProxy: workloadClusterProxy,
Expand All @@ -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"),
Expand All @@ -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,
Expand All @@ -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"),
Expand All @@ -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,
Expand All @@ -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")
Expand All @@ -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"),
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions test/e2e/autoscaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
Expand Down
64 changes: 35 additions & 29 deletions test/framework/autoscaler_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{}
Expand All @@ -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")

Expand Down Expand Up @@ -343,15 +335,15 @@ 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
NodeGroupMaxSize string
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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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{
Expand All @@ -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},
},
},
}
Expand Down
Loading

0 comments on commit 4c61ef6

Please sign in to comment.