Skip to content

Commit c469b38

Browse files
Jont828Jont828
committed
Implement MachinePool Machines in CAPI, CAPD, and clusterctl
1 parent fc799af commit c469b38

34 files changed

+1742
-283
lines changed

‎api/v1beta1/common_types.go

+5
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ const (
6161
// update that disallows a pre-existing Cluster to be populated with Topology information and Class.
6262
ClusterTopologyUnsafeUpdateClassNameAnnotation = "unsafe.topology.cluster.x-k8s.io/disable-update-class-name-check"
6363

64+
// FallbackMachineLabel indicates that a Machine belongs to a MachinePool that does not support MachinePool Machines.
65+
// As such, these Machines exist to create a consistent user experience and will not have an infrastructure reference. The user will
66+
// also be prevented from deleting these Machines.
67+
FallbackMachineLabel = "machinepool.cluster.x-k8s.io/fallback-machine"
68+
6469
// ProviderNameLabel is the label set on components in the provider manifest.
6570
// This label allows to easily identify all the components belonging to a provider; the clusterctl
6671
// tool uses this label for implementing provider's lifecycle operations.

‎api/v1beta1/machine_types.go

+4
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ const (
4343
// MachineDeploymentNameLabel is the label set on machines if they're controlled by MachineDeployment.
4444
MachineDeploymentNameLabel = "cluster.x-k8s.io/deployment-name"
4545

46+
// MachinePoolNameLabel is the label indicating the name of the MachinePool a Machine is controlled by.
47+
// Note: The value of this label may be a hash if the MachinePool name is longer than 63 characters.
48+
MachinePoolNameLabel = "cluster.x-k8s.io/pool-name"
49+
4650
// MachineControlPlaneNameLabel is the label set on machines if they're controlled by a ControlPlane.
4751
// Note: The value of this label may be a hash if the control plane name is longer than 63 characters.
4852
MachineControlPlaneNameLabel = "cluster.x-k8s.io/control-plane-name"

‎api/v1beta1/machine_webhook.go

+91-26
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"context"
2021
"fmt"
22+
"os"
2123
"strings"
2224
"time"
2325

@@ -27,6 +29,7 @@ import (
2729
"k8s.io/apimachinery/pkg/util/validation/field"
2830
ctrl "sigs.k8s.io/controller-runtime"
2931
"sigs.k8s.io/controller-runtime/pkg/webhook"
32+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
3033

3134
"sigs.k8s.io/cluster-api/util/version"
3235
)
@@ -35,14 +38,15 @@ const defaultNodeDeletionTimeout = 10 * time.Second
3538

3639
func (m *Machine) SetupWebhookWithManager(mgr ctrl.Manager) error {
3740
return ctrl.NewWebhookManagedBy(mgr).
38-
For(m).
41+
For(&Machine{}).
42+
WithValidator(MachineValidator(mgr.GetScheme())).
3943
Complete()
4044
}
4145

42-
// +kubebuilder:webhook:verbs=create;update,path=/validate-cluster-x-k8s-io-v1beta1-machine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=machines,versions=v1beta1,name=validation.machine.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
46+
// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-cluster-x-k8s-io-v1beta1-machine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=machines,versions=v1beta1,name=validation.machine.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
4347
// +kubebuilder:webhook:verbs=create;update,path=/mutate-cluster-x-k8s-io-v1beta1-machine,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=machines,versions=v1beta1,name=default.machine.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
4448

45-
var _ webhook.Validator = &Machine{}
49+
var _ webhook.CustomValidator = &machineValidator{}
4650
var _ webhook.Defaulter = &Machine{}
4751

4852
// Default implements webhook.Defaulter so a webhook will be registered for the type.
@@ -57,7 +61,10 @@ func (m *Machine) Default() {
5761
}
5862

5963
if m.Spec.InfrastructureRef.Namespace == "" {
60-
m.Spec.InfrastructureRef.Namespace = m.Namespace
64+
// Don't autofill namespace for MachinePool Machines since the infraRef will be populated by the MachinePool controller.
65+
if !isMachinePoolMachine(m) {
66+
m.Spec.InfrastructureRef.Namespace = m.Namespace
67+
}
6168
}
6269

6370
if m.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Version, "v") {
@@ -70,36 +77,77 @@ func (m *Machine) Default() {
7077
}
7178
}
7279

80+
// MachineValidator creates a new CustomValidator for Machines.
81+
func MachineValidator(_ *runtime.Scheme) webhook.CustomValidator {
82+
return &machineValidator{}
83+
}
84+
85+
// machineValidator implements a defaulting webhook for Machine.
86+
type machineValidator struct{}
87+
7388
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
74-
func (m *Machine) ValidateCreate() error {
89+
func (*machineValidator) ValidateCreate(_ context.Context, obj runtime.Object) error {
90+
m, ok := obj.(*Machine)
91+
if !ok {
92+
return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", obj))
93+
}
94+
7595
return m.validate(nil)
7696
}
7797

7898
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
79-
func (m *Machine) ValidateUpdate(old runtime.Object) error {
80-
oldM, ok := old.(*Machine)
99+
func (*machineValidator) ValidateUpdate(_ context.Context, oldObj runtime.Object, newObj runtime.Object) error {
100+
newM, ok := newObj.(*Machine)
101+
if !ok {
102+
return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", newObj))
103+
}
104+
105+
oldM, ok := oldObj.(*Machine)
81106
if !ok {
82-
return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", old))
107+
return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", oldObj))
83108
}
84-
return m.validate(oldM)
109+
return newM.validate(oldM)
85110
}
86111

87112
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
88-
func (m *Machine) ValidateDelete() error {
113+
func (*machineValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
114+
m, ok := obj.(*Machine)
115+
if !ok {
116+
return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", obj))
117+
}
118+
119+
req, err := admission.RequestFromContext(ctx)
120+
if err != nil {
121+
return apierrors.NewBadRequest(fmt.Sprintf("expected a admission.Request inside context: %v", err))
122+
}
123+
124+
// Fallback machines are placeholders for InfraMachinePools that do not support MachinePool Machines. These have
125+
// no bootstrap or infrastructure data and cannot be deleted by users. They instead exist to provide a consistent
126+
// user experience for MachinePool Machines.
127+
if _, isFallbackMachine := m.Labels[FallbackMachineLabel]; isFallbackMachine {
128+
// Only allow the request if it is coming from the CAPI controller service account.
129+
if req.UserInfo.Username != "system:serviceaccount:"+os.Getenv("POD_NAMESPACE")+":"+os.Getenv("POD_SERVICE_ACCOUNT") {
130+
return apierrors.NewBadRequest("this Machine is a placeholder for InfraMachinePools that do not support MachinePool Machines and cannot be deleted by users, scale down the MachinePool instead to delete")
131+
}
132+
}
133+
89134
return nil
90135
}
91136

92137
func (m *Machine) validate(old *Machine) error {
93138
var allErrs field.ErrorList
94139
specPath := field.NewPath("spec")
95140
if m.Spec.Bootstrap.ConfigRef == nil && m.Spec.Bootstrap.DataSecretName == nil {
96-
allErrs = append(
97-
allErrs,
98-
field.Required(
99-
specPath.Child("bootstrap", "data"),
100-
"expected either spec.bootstrap.dataSecretName or spec.bootstrap.configRef to be populated",
101-
),
102-
)
141+
// MachinePool Machines don't have a bootstrap configRef, so don't require it. The bootstrap config is instead owned by the MachinePool.
142+
if !isMachinePoolMachine(m) {
143+
allErrs = append(
144+
allErrs,
145+
field.Required(
146+
specPath.Child("bootstrap", "data"),
147+
"expected either spec.bootstrap.dataSecretName or spec.bootstrap.configRef to be populated",
148+
),
149+
)
150+
}
103151
}
104152

105153
if m.Spec.Bootstrap.ConfigRef != nil && m.Spec.Bootstrap.ConfigRef.Namespace != m.Namespace {
@@ -113,15 +161,18 @@ func (m *Machine) validate(old *Machine) error {
113161
)
114162
}
115163

116-
if m.Spec.InfrastructureRef.Namespace != m.Namespace {
117-
allErrs = append(
118-
allErrs,
119-
field.Invalid(
120-
specPath.Child("infrastructureRef", "namespace"),
121-
m.Spec.InfrastructureRef.Namespace,
122-
"must match metadata.namespace",
123-
),
124-
)
164+
// InfraRef can be empty for MachinePool Machines so skip the check in that case.
165+
if !isMachinePoolMachine(m) {
166+
if m.Spec.InfrastructureRef.Namespace != m.Namespace {
167+
allErrs = append(
168+
allErrs,
169+
field.Invalid(
170+
specPath.Child("infrastructureRef", "namespace"),
171+
m.Spec.InfrastructureRef.Namespace,
172+
"must match metadata.namespace",
173+
),
174+
)
175+
}
125176
}
126177

127178
if old != nil && old.Spec.ClusterName != m.Spec.ClusterName {
@@ -142,3 +193,17 @@ func (m *Machine) validate(old *Machine) error {
142193
}
143194
return apierrors.NewInvalid(GroupVersion.WithKind("Machine").GroupKind(), m.Name, allErrs)
144195
}
196+
197+
func isMachinePoolMachine(m *Machine) bool {
198+
if m.OwnerReferences == nil {
199+
return false
200+
}
201+
202+
for _, owner := range m.OwnerReferences {
203+
if owner.Kind == "MachinePool" {
204+
return true
205+
}
206+
}
207+
208+
return false
209+
}

‎api/v1beta1/machine_webhook_test.go

+96-17
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@ limitations under the License.
1717
package v1beta1
1818

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

2223
. "github.com/onsi/gomega"
24+
admissionv1 "k8s.io/api/admission/v1"
2325
corev1 "k8s.io/api/core/v1"
2426
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2527
"k8s.io/utils/pointer"
26-
27-
utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
28+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2829
)
2930

3031
func TestMachineDefault(t *testing.T) {
@@ -39,7 +40,10 @@ func TestMachineDefault(t *testing.T) {
3940
Version: pointer.String("1.17.5"),
4041
},
4142
}
42-
t.Run("for Machine", utildefaulting.DefaultValidateTest(m))
43+
scheme, err := SchemeBuilder.Build()
44+
g.Expect(err).ToNot(HaveOccurred())
45+
validator := MachineValidator(scheme)
46+
t.Run("for Machine", defaultDefaulterTestCustomValidator(m, validator))
4347
m.Default()
4448

4549
g.Expect(m.Labels[ClusterNameLabel]).To(Equal(m.Spec.ClusterName))
@@ -75,15 +79,25 @@ func TestMachineBootstrapValidation(t *testing.T) {
7579
for _, tt := range tests {
7680
t.Run(tt.name, func(t *testing.T) {
7781
g := NewWithT(t)
82+
scheme, err := SchemeBuilder.Build()
83+
g.Expect(err).ToNot(HaveOccurred())
84+
validator := MachineValidator(scheme)
85+
86+
ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
87+
AdmissionRequest: admissionv1.AdmissionRequest{
88+
Operation: admissionv1.Create,
89+
},
90+
})
91+
7892
m := &Machine{
7993
Spec: MachineSpec{Bootstrap: tt.bootstrap},
8094
}
8195
if tt.expectErr {
82-
g.Expect(m.ValidateCreate()).NotTo(Succeed())
83-
g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
96+
g.Expect(validator.ValidateCreate(ctx, m)).NotTo(Succeed())
97+
g.Expect(validator.ValidateUpdate(ctx, m, m)).NotTo(Succeed())
8498
} else {
85-
g.Expect(m.ValidateCreate()).To(Succeed())
86-
g.Expect(m.ValidateUpdate(m)).To(Succeed())
99+
g.Expect(validator.ValidateCreate(ctx, m)).To(Succeed())
100+
g.Expect(validator.ValidateUpdate(ctx, m, m)).To(Succeed())
87101
}
88102
})
89103
}
@@ -130,18 +144,27 @@ func TestMachineNamespaceValidation(t *testing.T) {
130144
for _, tt := range tests {
131145
t.Run(tt.name, func(t *testing.T) {
132146
g := NewWithT(t)
147+
scheme, err := SchemeBuilder.Build()
148+
g.Expect(err).ToNot(HaveOccurred())
149+
validator := MachineValidator(scheme)
150+
151+
ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
152+
AdmissionRequest: admissionv1.AdmissionRequest{
153+
Operation: admissionv1.Create,
154+
},
155+
})
133156

134157
m := &Machine{
135158
ObjectMeta: metav1.ObjectMeta{Namespace: tt.namespace},
136159
Spec: MachineSpec{Bootstrap: tt.bootstrap, InfrastructureRef: tt.infraRef},
137160
}
138161

139162
if tt.expectErr {
140-
g.Expect(m.ValidateCreate()).NotTo(Succeed())
141-
g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
163+
g.Expect(validator.ValidateCreate(ctx, m)).NotTo(Succeed())
164+
g.Expect(validator.ValidateUpdate(ctx, m, m)).NotTo(Succeed())
142165
} else {
143-
g.Expect(m.ValidateCreate()).To(Succeed())
144-
g.Expect(m.ValidateUpdate(m)).To(Succeed())
166+
g.Expect(validator.ValidateCreate(ctx, m)).To(Succeed())
167+
g.Expect(validator.ValidateUpdate(ctx, m, m)).To(Succeed())
145168
}
146169
})
147170
}
@@ -171,6 +194,15 @@ func TestMachineClusterNameImmutable(t *testing.T) {
171194
for _, tt := range tests {
172195
t.Run(tt.name, func(t *testing.T) {
173196
g := NewWithT(t)
197+
scheme, err := SchemeBuilder.Build()
198+
g.Expect(err).ToNot(HaveOccurred())
199+
validator := MachineValidator(scheme)
200+
201+
ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
202+
AdmissionRequest: admissionv1.AdmissionRequest{
203+
Operation: admissionv1.Create,
204+
},
205+
})
174206

175207
newMachine := &Machine{
176208
Spec: MachineSpec{
@@ -186,9 +218,9 @@ func TestMachineClusterNameImmutable(t *testing.T) {
186218
}
187219

188220
if tt.expectErr {
189-
g.Expect(newMachine.ValidateUpdate(oldMachine)).NotTo(Succeed())
221+
g.Expect(validator.ValidateUpdate(ctx, oldMachine, newMachine)).NotTo(Succeed())
190222
} else {
191-
g.Expect(newMachine.ValidateUpdate(oldMachine)).To(Succeed())
223+
g.Expect(validator.ValidateUpdate(ctx, oldMachine, newMachine)).To(Succeed())
192224
}
193225
})
194226
}
@@ -230,6 +262,15 @@ func TestMachineVersionValidation(t *testing.T) {
230262
for _, tt := range tests {
231263
t.Run(tt.name, func(t *testing.T) {
232264
g := NewWithT(t)
265+
scheme, err := SchemeBuilder.Build()
266+
g.Expect(err).ToNot(HaveOccurred())
267+
validator := MachineValidator(scheme)
268+
269+
ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
270+
AdmissionRequest: admissionv1.AdmissionRequest{
271+
Operation: admissionv1.Create,
272+
},
273+
})
233274

234275
m := &Machine{
235276
Spec: MachineSpec{
@@ -239,12 +280,50 @@ func TestMachineVersionValidation(t *testing.T) {
239280
}
240281

241282
if tt.expectErr {
242-
g.Expect(m.ValidateCreate()).NotTo(Succeed())
243-
g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
283+
g.Expect(validator.ValidateCreate(ctx, m)).NotTo(Succeed())
284+
g.Expect(validator.ValidateUpdate(ctx, m, m)).NotTo(Succeed())
244285
} else {
245-
g.Expect(m.ValidateCreate()).To(Succeed())
246-
g.Expect(m.ValidateUpdate(m)).To(Succeed())
286+
g.Expect(validator.ValidateCreate(ctx, m)).To(Succeed())
287+
g.Expect(validator.ValidateUpdate(ctx, m, m)).To(Succeed())
247288
}
248289
})
249290
}
250291
}
292+
293+
// defaultDefaulterTestCustomVAlidator returns a new testing function to be used in tests to
294+
// make sure defaulting webhooks also pass validation tests on create, update and delete.
295+
// Note: The difference to util/defaulting.DefaultValidateTest is that this function takes an additional
296+
// CustomValidator as the validation is not implemented on the object directly.
297+
func defaultDefaulterTestCustomValidator(object admission.Defaulter, customValidator admission.CustomValidator) func(*testing.T) {
298+
return func(t *testing.T) {
299+
t.Helper()
300+
301+
createCopy := object.DeepCopyObject().(admission.Defaulter)
302+
updateCopy := object.DeepCopyObject().(admission.Defaulter)
303+
deleteCopy := object.DeepCopyObject().(admission.Defaulter)
304+
defaultingUpdateCopy := updateCopy.DeepCopyObject().(admission.Defaulter)
305+
306+
ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
307+
AdmissionRequest: admissionv1.AdmissionRequest{
308+
Operation: admissionv1.Create,
309+
},
310+
})
311+
312+
t.Run("validate-on-create", func(t *testing.T) {
313+
g := NewWithT(t)
314+
createCopy.Default()
315+
g.Expect(customValidator.ValidateCreate(ctx, createCopy)).To(Succeed())
316+
})
317+
t.Run("validate-on-update", func(t *testing.T) {
318+
g := NewWithT(t)
319+
defaultingUpdateCopy.Default()
320+
updateCopy.Default()
321+
g.Expect(customValidator.ValidateUpdate(ctx, defaultingUpdateCopy, updateCopy)).To(Succeed())
322+
})
323+
t.Run("validate-on-delete", func(t *testing.T) {
324+
g := NewWithT(t)
325+
deleteCopy.Default()
326+
g.Expect(customValidator.ValidateDelete(ctx, deleteCopy)).To(Succeed())
327+
})
328+
}
329+
}

0 commit comments

Comments
 (0)