Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

🌱 Add MachinePools to autoscaler e2e test #10083

Merged
merged 2 commits into from
Apr 8, 2024
Merged

🌱 Add MachinePools to autoscaler e2e test #10083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
becac55
Add mp to autoscaler e2e test
willie-yao Jan 31, 2024
4c61ef6
Reviews
willie-yao Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 104 additions & 4 deletions exp/internal/webhooks/machinepool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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).
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
}
166 changes: 166 additions & 0 deletions exp/internal/webhooks/machinepool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package webhooks

import (
"context"
"strings"
"testing"

Expand All @@ -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"
Expand Down Expand Up @@ -56,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 All @@ -67,6 +70,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,
chrischdi marked this conversation as resolved.
Show resolved Hide resolved
},
{
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.
Expand Down
Loading
Loading