Skip to content

Commit becac55

Browse files
willie-yaowillie-yao
committed
Add mp to autoscaler e2e test
1 parent 7eea1d9 commit becac55

File tree

7 files changed

+606
-54
lines changed

7 files changed

+606
-54
lines changed

‎exp/internal/webhooks/machinepool.go

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@ package webhooks
1919
import (
2020
"context"
2121
"fmt"
22+
"strconv"
2223
"strings"
2324

25+
"github.com/pkg/errors"
26+
v1 "k8s.io/api/admission/v1"
2427
apierrors "k8s.io/apimachinery/pkg/api/errors"
2528
"k8s.io/apimachinery/pkg/runtime"
2629
"k8s.io/apimachinery/pkg/util/validation/field"
@@ -36,6 +39,10 @@ import (
3639
)
3740

3841
func (webhook *MachinePool) SetupWebhookWithManager(mgr ctrl.Manager) error {
42+
if webhook.decoder == nil {
43+
webhook.decoder = admission.NewDecoder(mgr.GetScheme())
44+
}
45+
3946
return ctrl.NewWebhookManagedBy(mgr).
4047
For(&expv1.MachinePool{}).
4148
WithDefaulter(webhook).
@@ -47,27 +54,48 @@ func (webhook *MachinePool) SetupWebhookWithManager(mgr ctrl.Manager) error {
4754
// +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
4855

4956
// MachinePool implements a validation and defaulting webhook for MachinePool.
50-
type MachinePool struct{}
57+
type MachinePool struct {
58+
decoder *admission.Decoder
59+
}
5160

5261
var _ webhook.CustomValidator = &MachinePool{}
5362
var _ webhook.CustomDefaulter = &MachinePool{}
5463

5564
// Default implements webhook.Defaulter so a webhook will be registered for the type.
56-
func (webhook *MachinePool) Default(_ context.Context, obj runtime.Object) error {
65+
func (webhook *MachinePool) Default(ctx context.Context, obj runtime.Object) error {
5766
m, ok := obj.(*expv1.MachinePool)
5867
if !ok {
5968
return apierrors.NewBadRequest(fmt.Sprintf("expected a MachinePool but got a %T", obj))
6069
}
6170

71+
req, err := admission.RequestFromContext(ctx)
72+
if err != nil {
73+
return err
74+
}
75+
dryRun := false
76+
if req.DryRun != nil {
77+
dryRun = *req.DryRun
78+
}
79+
var oldMP *expv1.MachinePool
80+
if req.Operation == v1.Update {
81+
oldMP = &expv1.MachinePool{}
82+
if err := webhook.decoder.DecodeRaw(req.OldObject, oldMP); err != nil {
83+
return errors.Wrapf(err, "failed to decode oldObject to MachinePool")
84+
}
85+
}
86+
6287
if m.Labels == nil {
6388
m.Labels = make(map[string]string)
6489
}
6590
m.Labels[clusterv1.ClusterNameLabel] = m.Spec.ClusterName
6691

67-
if m.Spec.Replicas == nil {
68-
m.Spec.Replicas = ptr.To[int32](1)
92+
replicas, err := calculateMachinePoolReplicas(ctx, oldMP, m, dryRun)
93+
if err != nil {
94+
return err
6995
}
7096

97+
m.Spec.Replicas = ptr.To[int32](replicas)
98+
7199
if m.Spec.MinReadySeconds == nil {
72100
m.Spec.MinReadySeconds = ptr.To[int32](0)
73101
}
@@ -187,3 +215,75 @@ func (webhook *MachinePool) validate(oldObj, newObj *expv1.MachinePool) error {
187215
}
188216
return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("MachinePool").GroupKind(), newObj.Name, allErrs)
189217
}
218+
219+
func calculateMachinePoolReplicas(ctx context.Context, oldMP *expv1.MachinePool, newMP *expv1.MachinePool, dryRun bool) (int32, error) {
220+
// If replicas is already set => Keep the current value.
221+
if newMP.Spec.Replicas != nil {
222+
return *newMP.Spec.Replicas, nil
223+
}
224+
225+
log := ctrl.LoggerFrom(ctx)
226+
227+
// If both autoscaler annotations are set, use them to calculate the default value.
228+
minSizeString, hasMinSizeAnnotation := newMP.Annotations[clusterv1.AutoscalerMinSizeAnnotation]
229+
maxSizeString, hasMaxSizeAnnotation := newMP.Annotations[clusterv1.AutoscalerMaxSizeAnnotation]
230+
if hasMinSizeAnnotation && hasMaxSizeAnnotation {
231+
minSize, err := strconv.ParseInt(minSizeString, 10, 32)
232+
if err != nil {
233+
return 0, errors.Wrapf(err, "failed to caculate MachinePool replicas value: could not parse the value of the %q annotation", clusterv1.AutoscalerMinSizeAnnotation)
234+
}
235+
maxSize, err := strconv.ParseInt(maxSizeString, 10, 32)
236+
if err != nil {
237+
return 0, errors.Wrapf(err, "failed to caculate MachinePool replicas value: could not parse the value of the %q annotation", clusterv1.AutoscalerMaxSizeAnnotation)
238+
}
239+
240+
// If it's a new MachinePool => Use the min size.
241+
// Note: This will result in a scale up to get into the range where autoscaler takes over.
242+
if oldMP == nil {
243+
if !dryRun {
244+
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))
245+
}
246+
return int32(minSize), nil
247+
}
248+
249+
// Otherwise we are handing over the control for the replicas field for an existing MachinePool
250+
// to the autoscaler.
251+
252+
switch {
253+
// If the old MachinePool doesn't have replicas set => Use the min size.
254+
// Note: As defaulting always sets the replica field, this case should not be possible
255+
// We only have this handling to be 100% safe against panics.
256+
case oldMP.Spec.Replicas == nil:
257+
if !dryRun {
258+
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))
259+
}
260+
return int32(minSize), nil
261+
// If the old MachinePool replicas are lower than min size => Use the min size.
262+
// Note: This will result in a scale up to get into the range where autoscaler takes over.
263+
case *oldMP.Spec.Replicas < int32(minSize):
264+
if !dryRun {
265+
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))
266+
}
267+
return int32(minSize), nil
268+
// If the old MachinePool replicas are higher than max size => Use the max size.
269+
// Note: This will result in a scale down to get into the range where autoscaler takes over.
270+
case *oldMP.Spec.Replicas > int32(maxSize):
271+
if !dryRun {
272+
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))
273+
}
274+
return int32(maxSize), nil
275+
// If the old MachinePool replicas are between min and max size => Keep the current value.
276+
default:
277+
if !dryRun {
278+
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))
279+
}
280+
return *oldMP.Spec.Replicas, nil
281+
}
282+
}
283+
284+
// If neither the default nor the autoscaler annotations are set => Default to 1.
285+
if !dryRun {
286+
log.V(2).Info("Replica field has been defaulted to 1")
287+
}
288+
return 1, nil
289+
}

‎exp/internal/webhooks/machinepool_test.go

Lines changed: 166 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package webhooks
1818

1919
import (
20+
"context"
2021
"strings"
2122
"testing"
2223

@@ -26,14 +27,15 @@ import (
2627
utilfeature "k8s.io/component-base/featuregate/testing"
2728
"k8s.io/utils/ptr"
2829
ctrl "sigs.k8s.io/controller-runtime"
30+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2931

3032
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3133
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
3234
"sigs.k8s.io/cluster-api/feature"
3335
"sigs.k8s.io/cluster-api/internal/webhooks/util"
3436
)
3537

36-
var ctx = ctrl.SetupSignalHandler()
38+
var ctx = admission.NewContextWithRequest(ctrl.SetupSignalHandler(), admission.Request{})
3739

3840
func TestMachinePoolDefault(t *testing.T) {
3941
// NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool.
@@ -67,6 +69,169 @@ func TestMachinePoolDefault(t *testing.T) {
6769
g.Expect(mp.Spec.Template.Spec.Version).To(Equal(ptr.To("v1.20.0")))
6870
}
6971

72+
func TestCalculateMachinePoolReplicas(t *testing.T) {
73+
tests := []struct {
74+
name string
75+
newMP *expv1.MachinePool
76+
oldMP *expv1.MachinePool
77+
expectedReplicas int32
78+
expectErr bool
79+
}{
80+
{
81+
name: "if new MP has replicas set, keep that value",
82+
newMP: &expv1.MachinePool{
83+
Spec: expv1.MachinePoolSpec{
84+
Replicas: ptr.To[int32](5),
85+
},
86+
},
87+
expectedReplicas: 5,
88+
},
89+
{
90+
name: "if new MP does not have replicas set and no annotations, use 1",
91+
newMP: &expv1.MachinePool{},
92+
expectedReplicas: 1,
93+
},
94+
{
95+
name: "if new MP only has min size annotation, fallback to 1",
96+
newMP: &expv1.MachinePool{
97+
ObjectMeta: metav1.ObjectMeta{
98+
Annotations: map[string]string{
99+
clusterv1.AutoscalerMinSizeAnnotation: "3",
100+
},
101+
},
102+
},
103+
expectedReplicas: 1,
104+
},
105+
{
106+
name: "if new MP only has max size annotation, fallback to 1",
107+
newMP: &expv1.MachinePool{
108+
ObjectMeta: metav1.ObjectMeta{
109+
Annotations: map[string]string{
110+
clusterv1.AutoscalerMaxSizeAnnotation: "7",
111+
},
112+
},
113+
},
114+
expectedReplicas: 1,
115+
},
116+
{
117+
name: "if new MP has min and max size annotation and min size is invalid, fail",
118+
newMP: &expv1.MachinePool{
119+
ObjectMeta: metav1.ObjectMeta{
120+
Annotations: map[string]string{
121+
clusterv1.AutoscalerMinSizeAnnotation: "abc",
122+
clusterv1.AutoscalerMaxSizeAnnotation: "7",
123+
},
124+
},
125+
},
126+
expectErr: true,
127+
},
128+
{
129+
name: "if new MP has min and max size annotation and max size is invalid, fail",
130+
newMP: &expv1.MachinePool{
131+
ObjectMeta: metav1.ObjectMeta{
132+
Annotations: map[string]string{
133+
clusterv1.AutoscalerMinSizeAnnotation: "3",
134+
clusterv1.AutoscalerMaxSizeAnnotation: "abc",
135+
},
136+
},
137+
},
138+
expectErr: true,
139+
},
140+
{
141+
name: "if new MP has min and max size annotation and new MP is a new MP, use min size",
142+
newMP: &expv1.MachinePool{
143+
ObjectMeta: metav1.ObjectMeta{
144+
Annotations: map[string]string{
145+
clusterv1.AutoscalerMinSizeAnnotation: "3",
146+
clusterv1.AutoscalerMaxSizeAnnotation: "7",
147+
},
148+
},
149+
},
150+
expectedReplicas: 3,
151+
},
152+
{
153+
name: "if new MP has min and max size annotation and old MP doesn't have replicas set, use min size",
154+
newMP: &expv1.MachinePool{
155+
ObjectMeta: metav1.ObjectMeta{
156+
Annotations: map[string]string{
157+
clusterv1.AutoscalerMinSizeAnnotation: "3",
158+
clusterv1.AutoscalerMaxSizeAnnotation: "7",
159+
},
160+
},
161+
},
162+
oldMP: &expv1.MachinePool{},
163+
expectedReplicas: 3,
164+
},
165+
{
166+
name: "if new MP has min and max size annotation and old MP replicas is below min size, use min size",
167+
newMP: &expv1.MachinePool{
168+
ObjectMeta: metav1.ObjectMeta{
169+
Annotations: map[string]string{
170+
clusterv1.AutoscalerMinSizeAnnotation: "3",
171+
clusterv1.AutoscalerMaxSizeAnnotation: "7",
172+
},
173+
},
174+
},
175+
oldMP: &expv1.MachinePool{
176+
Spec: expv1.MachinePoolSpec{
177+
Replicas: ptr.To[int32](1),
178+
},
179+
},
180+
expectedReplicas: 3,
181+
},
182+
{
183+
name: "if new MP has min and max size annotation and old MP replicas is above max size, use max size",
184+
newMP: &expv1.MachinePool{
185+
ObjectMeta: metav1.ObjectMeta{
186+
Annotations: map[string]string{
187+
clusterv1.AutoscalerMinSizeAnnotation: "3",
188+
clusterv1.AutoscalerMaxSizeAnnotation: "7",
189+
},
190+
},
191+
},
192+
oldMP: &expv1.MachinePool{
193+
Spec: expv1.MachinePoolSpec{
194+
Replicas: ptr.To[int32](15),
195+
},
196+
},
197+
expectedReplicas: 7,
198+
},
199+
{
200+
name: "if new MP has min and max size annotation and old MP replicas is between min and max size, use old MP replicas",
201+
newMP: &expv1.MachinePool{
202+
ObjectMeta: metav1.ObjectMeta{
203+
Annotations: map[string]string{
204+
clusterv1.AutoscalerMinSizeAnnotation: "3",
205+
clusterv1.AutoscalerMaxSizeAnnotation: "7",
206+
},
207+
},
208+
},
209+
oldMP: &expv1.MachinePool{
210+
Spec: expv1.MachinePoolSpec{
211+
Replicas: ptr.To[int32](4),
212+
},
213+
},
214+
expectedReplicas: 4,
215+
},
216+
}
217+
218+
for _, tt := range tests {
219+
t.Run(tt.name, func(t *testing.T) {
220+
g := NewWithT(t)
221+
222+
replicas, err := calculateMachinePoolReplicas(context.Background(), tt.oldMP, tt.newMP, false)
223+
224+
if tt.expectErr {
225+
g.Expect(err).To(HaveOccurred())
226+
return
227+
}
228+
229+
g.Expect(err).ToNot(HaveOccurred())
230+
g.Expect(replicas).To(Equal(tt.expectedReplicas))
231+
})
232+
}
233+
}
234+
70235
func TestMachinePoolBootstrapValidation(t *testing.T) {
71236
// NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool.
72237
// Enabling the feature flag temporarily for this test.

0 commit comments

Comments
 (0)