Skip to content

Commit 10741fd

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#10083 from willie-yao/cc-mp-autoscaler
🌱 Add MachinePools to autoscaler e2e test
2 parents b96861a + 4c61ef6 commit 10741fd

File tree

7 files changed

+622
-43
lines changed

7 files changed

+622
-43
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 & 0 deletions
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

@@ -25,6 +26,7 @@ import (
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
"k8s.io/utils/ptr"
2728
ctrl "sigs.k8s.io/controller-runtime"
29+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2830

2931
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3032
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
@@ -50,6 +52,7 @@ func TestMachinePoolDefault(t *testing.T) {
5052
},
5153
}
5254
webhook := &MachinePool{}
55+
ctx = admission.NewContextWithRequest(ctx, admission.Request{})
5356
t.Run("for MachinePool", util.CustomDefaultValidateTest(ctx, mp, webhook))
5457
g.Expect(webhook.Default(ctx, mp)).To(Succeed())
5558

@@ -61,6 +64,169 @@ func TestMachinePoolDefault(t *testing.T) {
6164
g.Expect(mp.Spec.Template.Spec.Version).To(Equal(ptr.To("v1.20.0")))
6265
}
6366

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

0 commit comments

Comments
 (0)