diff --git a/api/v1beta1/machineset_types.go b/api/v1beta1/machineset_types.go index 843b343a4582..f10e44f4f28d 100644 --- a/api/v1beta1/machineset_types.go +++ b/api/v1beta1/machineset_types.go @@ -41,9 +41,22 @@ type MachineSetSpec struct { // Replicas is the number of desired replicas. // This is a pointer to distinguish between explicit zero and unspecified. - // Defaults to 1. + // + // Defaults to: + // * if the Kubernetes autoscaler min size and max size annotations are set: + // - if it's a new MachineSet, use min size + // - if the replicas field of the old MachineSet is < min size, use min size + // - if the replicas field of the old MachineSet is > max size, use max size + // - if the replicas field of the old MachineSet is in the (min size, max size) range, keep the value from the oldMS + // * otherwise use 1 + // Note: Defaulting will be run whenever the replicas field is not set: + // * A new MachineSet is created with replicas not set. + // * On an existing MachineSet the replicas field was first set and is now unset. + // Those cases are especially relevant for the following Kubernetes autoscaler use cases: + // * A new MachineSet is created and replicas should be managed by the autoscaler + // * An existing MachineSet which initially wasn't controlled by the autoscaler + // should be later controlled by the autoscaler // +optional - // +kubebuilder:default=1 Replicas *int32 `json:"replicas,omitempty"` // MinReadySeconds is the minimum number of seconds for which a Node for a newly created machine should be ready before considering the replica available. diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index d49f8303a812..073de884ba90 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -2877,7 +2877,7 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_MachineSetSpec(ref common.Referenc }, "replicas": { SchemaProps: spec.SchemaProps{ - Description: "Replicas is the number of desired replicas. This is a pointer to distinguish between explicit zero and unspecified. Defaults to 1.", + Description: "Replicas is the number of desired replicas. This is a pointer to distinguish between explicit zero and unspecified.\n\nDefaults to: * if the Kubernetes autoscaler min size and max size annotations are set:\n - if it's a new MachineSet, use min size\n - if the replicas field of the old MachineSet is < min size, use min size\n - if the replicas field of the old MachineSet is > max size, use max size\n - if the replicas field of the old MachineSet is in the (min size, max size) range, keep the value from the oldMS\n* otherwise use 1 Note: Defaulting will be run whenever the replicas field is not set: * A new MachineSet is created with replicas not set. * On an existing MachineSet the replicas field was first set and is now unset. Those cases are especially relevant for the following Kubernetes autoscaler use cases: * A new MachineSet is created and replicas should be managed by the autoscaler * An existing MachineSet which initially wasn't controlled by the autoscaler\n should be later controlled by the autoscaler", Type: []string{"integer"}, Format: "int32", }, diff --git a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml index d670ba9aed76..e01642f078de 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml @@ -487,10 +487,22 @@ spec: format: int32 type: integer replicas: - default: 1 - description: Replicas is the number of desired replicas. This is a - pointer to distinguish between explicit zero and unspecified. Defaults - to 1. + description: "Replicas is the number of desired replicas. This is + a pointer to distinguish between explicit zero and unspecified. + \n Defaults to: * if the Kubernetes autoscaler min size and max + size annotations are set: - if it's a new MachineSet, use min size + - if the replicas field of the old MachineSet is < min size, use + min size - if the replicas field of the old MachineSet is > max + size, use max size - if the replicas field of the old MachineSet + is in the (min size, max size) range, keep the value from the oldMS + * otherwise use 1 Note: Defaulting will be run whenever the replicas + field is not set: * A new MachineSet is created with replicas not + set. * On an existing MachineSet the replicas field was first set + and is now unset. Those cases are especially relevant for the following + Kubernetes autoscaler use cases: * A new MachineSet is created and + replicas should be managed by the autoscaler * An existing MachineSet + which initially wasn't controlled by the autoscaler should be later + controlled by the autoscaler" format: int32 type: integer selector: diff --git a/docs/book/src/tasks/automated-machine-management/autoscaling.md b/docs/book/src/tasks/automated-machine-management/autoscaling.md index ca8dbca37e5e..442152b71865 100644 --- a/docs/book/src/tasks/automated-machine-management/autoscaling.md +++ b/docs/book/src/tasks/automated-machine-management/autoscaling.md @@ -12,17 +12,17 @@ from the [Autoscaler project documentation](https://github.com/kubernetes/autosc diff --git a/internal/controllers/machinehealthcheck/machinehealthcheck_controller_test.go b/internal/controllers/machinehealthcheck/machinehealthcheck_controller_test.go index 3cbea665c728..1e716bc02c1b 100644 --- a/internal/controllers/machinehealthcheck/machinehealthcheck_controller_test.go +++ b/internal/controllers/machinehealthcheck/machinehealthcheck_controller_test.go @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/api/v1beta1/index" @@ -1249,7 +1250,9 @@ func TestMachineHealthCheck_Reconcile(t *testing.T) { }, }, } - g.Expect((&webhooks.MachineSet{}).Default(ctx, machineSet)).Should(Succeed()) + + reqCtx := admission.NewContextWithRequest(ctx, admission.Request{}) + g.Expect((&webhooks.MachineSet{}).Default(reqCtx, machineSet)).Should(Succeed()) g.Expect(env.Create(ctx, machineSet)).To(Succeed()) // Ensure machines have been created. diff --git a/internal/controllers/topology/cluster/reconcile_state_test.go b/internal/controllers/topology/cluster/reconcile_state_test.go index d9171c490d10..cdbda35e6315 100644 --- a/internal/controllers/topology/cluster/reconcile_state_test.go +++ b/internal/controllers/topology/cluster/reconcile_state_test.go @@ -3241,9 +3241,8 @@ func newFakeMachineDeploymentTopologyState(name string, infrastructureMachineTem scheme := runtime.NewScheme() _ = clusterv1.AddToScheme(scheme) - if err := (&webhooks.MachineDeployment{ - Decoder: admission.NewDecoder(scheme), - }).Default(admission.NewContextWithRequest(ctx, admission.Request{}), mdState.Object); err != nil { + if err := (&webhooks.MachineDeployment{}). + Default(admission.NewContextWithRequest(ctx, admission.Request{}), mdState.Object); err != nil { panic(err) } return mdState diff --git a/internal/webhooks/machinedeployment.go b/internal/webhooks/machinedeployment.go index 0e8e86377f4f..ed7a9ca417f3 100644 --- a/internal/webhooks/machinedeployment.go +++ b/internal/webhooks/machinedeployment.go @@ -42,8 +42,8 @@ import ( ) func (webhook *MachineDeployment) SetupWebhookWithManager(mgr ctrl.Manager) error { - if webhook.Decoder == nil { - webhook.Decoder = admission.NewDecoder(mgr.GetScheme()) + if webhook.decoder == nil { + webhook.decoder = admission.NewDecoder(mgr.GetScheme()) } return ctrl.NewWebhookManagedBy(mgr). @@ -58,7 +58,7 @@ func (webhook *MachineDeployment) SetupWebhookWithManager(mgr ctrl.Manager) erro // MachineDeployment implements a validation and defaulting webhook for MachineDeployment. type MachineDeployment struct { - Decoder *admission.Decoder + decoder *admission.Decoder } var _ webhook.CustomDefaulter = &MachineDeployment{} @@ -83,7 +83,7 @@ func (webhook *MachineDeployment) Default(ctx context.Context, obj runtime.Objec var oldMD *clusterv1.MachineDeployment if req.Operation == v1.Update { oldMD = &clusterv1.MachineDeployment{} - if err := webhook.Decoder.DecodeRaw(req.OldObject, oldMD); err != nil { + if err := webhook.decoder.DecodeRaw(req.OldObject, oldMD); err != nil { return errors.Wrapf(err, "failed to decode oldObject to MachineDeployment") } } diff --git a/internal/webhooks/machinedeployment_test.go b/internal/webhooks/machinedeployment_test.go index ba7f9991eb6b..9668d064959d 100644 --- a/internal/webhooks/machinedeployment_test.go +++ b/internal/webhooks/machinedeployment_test.go @@ -52,7 +52,7 @@ func TestMachineDeploymentDefault(t *testing.T) { scheme := runtime.NewScheme() g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) webhook := &MachineDeployment{ - Decoder: admission.NewDecoder(scheme), + decoder: admission.NewDecoder(scheme), } reqCtx := admission.NewContextWithRequest(ctx, admission.Request{ @@ -403,7 +403,7 @@ func TestMachineDeploymentValidation(t *testing.T) { scheme := runtime.NewScheme() g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) webhook := MachineDeployment{ - Decoder: admission.NewDecoder(scheme), + decoder: admission.NewDecoder(scheme), } if tt.expectErr { @@ -475,7 +475,7 @@ func TestMachineDeploymentVersionValidation(t *testing.T) { scheme := runtime.NewScheme() g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) webhook := MachineDeployment{ - Decoder: admission.NewDecoder(scheme), + decoder: admission.NewDecoder(scheme), } if tt.expectErr { @@ -537,7 +537,7 @@ func TestMachineDeploymentClusterNameImmutable(t *testing.T) { scheme := runtime.NewScheme() g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) webhook := MachineDeployment{ - Decoder: admission.NewDecoder(scheme), + decoder: admission.NewDecoder(scheme), } warnings, err := webhook.ValidateUpdate(ctx, oldMD, newMD) @@ -589,7 +589,7 @@ func TestMachineDeploymentTemplateMetadataValidation(t *testing.T) { scheme := runtime.NewScheme() g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) webhook := MachineDeployment{ - Decoder: admission.NewDecoder(scheme), + decoder: admission.NewDecoder(scheme), } if tt.expectErr { diff --git a/internal/webhooks/machineset.go b/internal/webhooks/machineset.go index b9156544aa3f..f0f38f84e42e 100644 --- a/internal/webhooks/machineset.go +++ b/internal/webhooks/machineset.go @@ -19,14 +19,18 @@ package webhooks import ( "context" "fmt" + "strconv" "strings" + "github.com/pkg/errors" + v1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -39,6 +43,10 @@ import ( ) func (webhook *MachineSet) SetupWebhookWithManager(mgr ctrl.Manager) error { + if webhook.decoder == nil { + webhook.decoder = admission.NewDecoder(mgr.GetScheme()) + } + return ctrl.NewWebhookManagedBy(mgr). For(&clusterv1.MachineSet{}). WithDefaulter(webhook). @@ -50,23 +58,49 @@ func (webhook *MachineSet) SetupWebhookWithManager(mgr ctrl.Manager) error { // +kubebuilder:webhook:verbs=create;update,path=/mutate-cluster-x-k8s-io-v1beta1-machineset,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=machinesets,versions=v1beta1,name=default.machineset.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 // MachineSet implements a validation and defaulting webhook for MachineSet. -type MachineSet struct{} +type MachineSet struct { + decoder *admission.Decoder +} var _ webhook.CustomDefaulter = &MachineSet{} var _ webhook.CustomValidator = &MachineSet{} // Default sets default MachineSet field values. -func (webhook *MachineSet) Default(_ context.Context, obj runtime.Object) error { +func (webhook *MachineSet) Default(ctx context.Context, obj runtime.Object) error { m, ok := obj.(*clusterv1.MachineSet) if !ok { return apierrors.NewBadRequest(fmt.Sprintf("expected a MachineSet 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 oldMS *clusterv1.MachineSet + if req.Operation == v1.Update { + oldMS = &clusterv1.MachineSet{} + if err := webhook.decoder.DecodeRaw(req.OldObject, oldMS); err != nil { + return errors.Wrapf(err, "failed to decode oldObject to MachineSet") + } + } + if m.Labels == nil { m.Labels = make(map[string]string) } m.Labels[clusterv1.ClusterNameLabel] = m.Spec.ClusterName + replicas, err := calculateMachineSetReplicas(ctx, oldMS, m, dryRun) + if err != nil { + return err + } + m.Spec.Replicas = pointer.Int32(replicas) + if m.Spec.DeletePolicy == "" { randomPolicy := string(clusterv1.RandomMachineSetDeletePolicy) m.Spec.DeletePolicy = randomPolicy @@ -219,3 +253,104 @@ func validateSkippedMachineSetPreflightChecks(o client.Object) *field.Error { } return nil } + +// calculateMachineSetReplicas calculates the default value of the replicas field. +// The value will be calculated based on the following logic: +// * if replicas is already set on newMS, keep the current value +// * if the autoscaler min size and max size annotations are set: +// - if it's a new MachineSet, use min size +// - if the replicas field of the old MachineSet is < min size, use min size +// - if the replicas field of the old MachineSet is > max size, use max size +// - if the replicas field of the old MachineSet is in the (min size, max size) range, keep the value from the oldMS +// +// * otherwise use 1 +// +// The goal of this logic is to provide a smoother UX for clusters using the Kubernetes autoscaler. +// Note: Autoscaler only takes over control of the replicas field if the replicas value is in the (min size, max size) range. +// +// We are supporting the following use cases: +// * A new MS is created and replicas should be managed by the autoscaler +// - Either via the default annotation or via the min size and max size annotations the replicas field +// is defaulted to a value which is within the (min size, max size) range so the autoscaler can take control. +// +// * An existing MS which initially wasn't controlled by the autoscaler should be later controlled by the autoscaler +// - To adopt an existing MS users can use the default, min size and max size annotations to enable the autoscaler +// and to ensure the replicas field is within the (min size, max size) range. Without the annotations handing over +// control to the autoscaler by unsetting the replicas field would lead to the field being set to 1. This is very +// disruptive for existing Machines and if 1 is outside the (min size, max size) range the autoscaler won't take +// control. +// +// Notes: +// - While the min size and max size annotations of the autoscaler provide the best UX, other autoscalers can use the +// DefaultReplicasAnnotation if they have similar use cases. +func calculateMachineSetReplicas(ctx context.Context, oldMS *clusterv1.MachineSet, newMS *clusterv1.MachineSet, dryRun bool) (int32, error) { + // If replicas is already set => Keep the current value. + if newMS.Spec.Replicas != nil { + return *newMS.Spec.Replicas, nil + } + + log := ctrl.LoggerFrom(ctx) + + // If both autoscaler annotations are set, use them to calculate the default value. + minSizeString, hasMinSizeAnnotation := newMS.Annotations[clusterv1.AutoscalerMinSizeAnnotation] + maxSizeString, hasMaxSizeAnnotation := newMS.Annotations[clusterv1.AutoscalerMaxSizeAnnotation] + if hasMinSizeAnnotation && hasMaxSizeAnnotation { + minSize, err := strconv.ParseInt(minSizeString, 10, 32) + if err != nil { + return 0, errors.Wrapf(err, "failed to caculate MachineSet 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 MachineSet replicas value: could not parse the value of the %q annotation", clusterv1.AutoscalerMaxSizeAnnotation) + } + + // If it's a new MachineSet => Use the min size. + // Note: This will result in a scale up to get into the range where autoscaler takes over. + if oldMS == nil { + if !dryRun { + log.V(2).Info(fmt.Sprintf("Replica field has been defaulted to %d based on the %s annotation (MS is a new MS)", minSize, clusterv1.AutoscalerMinSizeAnnotation)) + } + return int32(minSize), nil + } + + // Otherwise we are handing over the control for the replicas field for an existing MachineSet + // to the autoscaler. + + switch { + // If the old MachineSet 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 oldMS.Spec.Replicas == nil: + if !dryRun { + log.V(2).Info(fmt.Sprintf("Replica field has been defaulted to %d based on the %s annotation (old MS didn't have replicas set)", minSize, clusterv1.AutoscalerMinSizeAnnotation)) + } + return int32(minSize), nil + // If the old MachineSet 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 *oldMS.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 MS had replicas below min size)", minSize, clusterv1.AutoscalerMinSizeAnnotation)) + } + return int32(minSize), nil + // If the old MachineSet 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 *oldMS.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 MS had replicas above max size)", maxSize, clusterv1.AutoscalerMaxSizeAnnotation)) + } + return int32(maxSize), nil + // If the old MachineSet 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 MachineSet (old MS had replicas within min size / max size range)", *oldMS.Spec.Replicas)) + } + return *oldMS.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/internal/webhooks/machineset_test.go b/internal/webhooks/machineset_test.go index 8ee8ab916601..d9779b799a71 100644 --- a/internal/webhooks/machineset_test.go +++ b/internal/webhooks/machineset_test.go @@ -17,12 +17,15 @@ limitations under the License. package webhooks import ( + "context" "strings" "testing" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/internal/webhooks/util" @@ -42,10 +45,16 @@ func TestMachineSetDefault(t *testing.T) { }, }, } - webhook := &MachineSet{} - t.Run("for MachineSet", util.CustomDefaultValidateTest(ctx, ms, webhook)) - g.Expect(webhook.Default(ctx, ms)).To(Succeed()) + scheme := runtime.NewScheme() + g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) + webhook := &MachineSet{ + decoder: admission.NewDecoder(scheme), + } + + reqCtx := admission.NewContextWithRequest(ctx, admission.Request{}) + t.Run("for MachineSet", util.CustomDefaultValidateTest(reqCtx, ms, webhook)) + g.Expect(webhook.Default(reqCtx, ms)).To(Succeed()) g.Expect(ms.Labels[clusterv1.ClusterNameLabel]).To(Equal(ms.Spec.ClusterName)) g.Expect(ms.Spec.DeletePolicy).To(Equal(string(clusterv1.RandomMachineSetDeletePolicy))) @@ -54,6 +63,169 @@ func TestMachineSetDefault(t *testing.T) { g.Expect(*ms.Spec.Template.Spec.Version).To(Equal("v1.19.10")) } +func TestCalculateMachineSetReplicas(t *testing.T) { + tests := []struct { + name string + newMS *clusterv1.MachineSet + oldMS *clusterv1.MachineSet + expectedReplicas int32 + expectErr bool + }{ + { + name: "if new MS has replicas set, keep that value", + newMS: &clusterv1.MachineSet{ + Spec: clusterv1.MachineSetSpec{ + Replicas: pointer.Int32(5), + }, + }, + expectedReplicas: 5, + }, + { + name: "if new MS does not have replicas set and no annotations, use 1", + newMS: &clusterv1.MachineSet{}, + expectedReplicas: 1, + }, + { + name: "if new MS only has min size annotation, fallback to 1", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + }, + }, + }, + expectedReplicas: 1, + }, + { + name: "if new MS only has max size annotation, fallback to 1", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + expectedReplicas: 1, + }, + { + name: "if new MS has min and max size annotation and min size is invalid, fail", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "abc", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + expectErr: true, + }, + { + name: "if new MS has min and max size annotation and max size is invalid, fail", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "abc", + }, + }, + }, + expectErr: true, + }, + { + name: "if new MS has min and max size annotation and new MS is a new MS, use min size", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + expectedReplicas: 3, + }, + { + name: "if new MS has min and max size annotation and old MS doesn't have replicas set, use min size", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMS: &clusterv1.MachineSet{}, + expectedReplicas: 3, + }, + { + name: "if new MS has min and max size annotation and old MS replicas is below min size, use min size", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMS: &clusterv1.MachineSet{ + Spec: clusterv1.MachineSetSpec{ + Replicas: pointer.Int32(1), + }, + }, + expectedReplicas: 3, + }, + { + name: "if new MS has min and max size annotation and old MS replicas is above max size, use max size", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMS: &clusterv1.MachineSet{ + Spec: clusterv1.MachineSetSpec{ + Replicas: pointer.Int32(15), + }, + }, + expectedReplicas: 7, + }, + { + name: "if new MS has min and max size annotation and old MS replicas is between min and max size, use old MS replicas", + newMS: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.AutoscalerMinSizeAnnotation: "3", + clusterv1.AutoscalerMaxSizeAnnotation: "7", + }, + }, + }, + oldMS: &clusterv1.MachineSet{ + Spec: clusterv1.MachineSetSpec{ + Replicas: pointer.Int32(4), + }, + }, + expectedReplicas: 4, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + replicas, err := calculateMachineSetReplicas(context.Background(), tt.oldMS, tt.newMS, false) + + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(replicas).To(Equal(tt.expectedReplicas)) + }) + } +} + func TestMachineSetLabelSelectorMatchValidation(t *testing.T) { tests := []struct { name string diff --git a/webhooks/alias.go b/webhooks/alias.go index ee0646bdc19e..b1688d95c80c 100644 --- a/webhooks/alias.go +++ b/webhooks/alias.go @@ -19,7 +19,6 @@ package webhooks import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/cluster-api/internal/webhooks" ) @@ -57,15 +56,11 @@ func (webhook *Machine) SetupWebhookWithManager(mgr ctrl.Manager) error { } // MachineDeployment implements a validating and defaulting webhook for MachineDeployment. -type MachineDeployment struct { - Decoder *admission.Decoder -} +type MachineDeployment struct{} // SetupWebhookWithManager sets up MachineDeployment webhooks. func (webhook *MachineDeployment) SetupWebhookWithManager(mgr ctrl.Manager) error { - return (&webhooks.MachineDeployment{ - Decoder: webhook.Decoder, - }).SetupWebhookWithManager(mgr) + return (&webhooks.MachineDeployment{}).SetupWebhookWithManager(mgr) } // MachineSet implements a validating and defaulting webhook for MachineSet.