Implement MachinePool Machines in CAPI, CAPD, and clusterctl

api/v1beta1/common_types.go

@@ -61,6 +61,11 @@ const (
 	// update that disallows a pre-existing Cluster to be populated with Topology information and Class.
 	ClusterTopologyUnsafeUpdateClassNameAnnotation = "unsafe.topology.cluster.x-k8s.io/disable-update-class-name-check"
 
+	// FallbackMachineLabel indicates that a Machine belongs to a MachinePool that does not support MachinePool Machines.
+	// As such, these Machines exist to create a consistent user experience and will not have an infrastructure reference. The user will
+	// also be prevented from deleting these Machines.
+	FallbackMachineLabel = "machinepool.cluster.x-k8s.io/fallback-machine"
+
 	// ProviderNameLabel is the label set on components in the provider manifest.
 	// This label allows to easily identify all the components belonging to a provider; the clusterctl
 	// tool uses this label for implementing provider's lifecycle operations.

api/v1beta1/machine_types.go

@@ -43,6 +43,10 @@ const (
 	// MachineDeploymentNameLabel is the label set on machines if they're controlled by MachineDeployment.
 	MachineDeploymentNameLabel = "cluster.x-k8s.io/deployment-name"
 
+	// MachinePoolNameLabel is the label indicating the name of the MachinePool a Machine is controlled by.
+	// Note: The value of this label may be a hash if the MachinePool name is longer than 63 characters.
+	MachinePoolNameLabel = "cluster.x-k8s.io/pool-name"
+
 	// MachineControlPlaneNameLabel is the label set on machines if they're controlled by a ControlPlane.
 	// Note: The value of this label may be a hash if the control plane name is longer than 63 characters.
 	MachineControlPlaneNameLabel = "cluster.x-k8s.io/control-plane-name"

api/v1beta1/machine_webhook.go

@@ -17,7 +17,9 @@ limitations under the License.
 package v1beta1
 
 import (
+	"context"
 	"fmt"
+	"os"
 	"strings"
 	"time"
 
@@ -27,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"sigs.k8s.io/cluster-api/util/version"
 )
@@ -35,14 +38,15 @@ const defaultNodeDeletionTimeout = 10 * time.Second
 
 func (m *Machine) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
-		For(m).
+		For(&Machine{}).
+		WithValidator(MachineValidator(mgr.GetScheme())).
 		Complete()
 }
 
-// +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
+// +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
 // +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
 
-var _ webhook.Validator = &Machine{}
+var _ webhook.CustomValidator = &machineValidator{}
 var _ webhook.Defaulter = &Machine{}
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
@@ -57,7 +61,10 @@ func (m *Machine) Default() {
 	}
 
 	if m.Spec.InfrastructureRef.Namespace == "" {
-		m.Spec.InfrastructureRef.Namespace = m.Namespace
+		// Don't autofill namespace for MachinePool Machines since the infraRef will be populated by the MachinePool controller.
+		if !isMachinePoolMachine(m) {
+			m.Spec.InfrastructureRef.Namespace = m.Namespace
+		}
 	}
 
 	if m.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Version, "v") {
@@ -70,36 +77,77 @@ func (m *Machine) Default() {
 	}
 }
 
+// MachineValidator creates a new CustomValidator for Machines.
+func MachineValidator(_ *runtime.Scheme) webhook.CustomValidator {
+	return &machineValidator{}
+}
+
+// machineValidator implements a defaulting webhook for Machine.
+type machineValidator struct{}
+
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (m *Machine) ValidateCreate() error {
+func (*machineValidator) ValidateCreate(_ context.Context, obj runtime.Object) error {
+	m, ok := obj.(*Machine)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", obj))
+	}
+
 	return m.validate(nil)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (m *Machine) ValidateUpdate(old runtime.Object) error {
-	oldM, ok := old.(*Machine)
+func (*machineValidator) ValidateUpdate(_ context.Context, oldObj runtime.Object, newObj runtime.Object) error {
+	newM, ok := newObj.(*Machine)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", newObj))
+	}
+
+	oldM, ok := oldObj.(*Machine)
 	if !ok {
-		return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", old))
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", oldObj))
 	}
-	return m.validate(oldM)
+	return newM.validate(oldM)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (m *Machine) ValidateDelete() error {
+func (*machineValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
+	m, ok := obj.(*Machine)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", obj))
+	}
+
+	req, err := admission.RequestFromContext(ctx)
+	if err != nil {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a admission.Request inside context: %v", err))
+	}
+
+	// Fallback machines are placeholders for InfraMachinePools that do not support MachinePool Machines. These have
+	// no bootstrap or infrastructure data and cannot be deleted by users. They instead exist to provide a consistent
+	// user experience for MachinePool Machines.
+	if _, isFallbackMachine := m.Labels[FallbackMachineLabel]; isFallbackMachine {
+		// Only allow the request if it is coming from the CAPI controller service account.
+		if req.UserInfo.Username != "system:serviceaccount:"+os.Getenv("POD_NAMESPACE")+":"+os.Getenv("POD_SERVICE_ACCOUNT") {
+			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")
+		}
+	}
+
 	return nil
 }
 
 func (m *Machine) validate(old *Machine) error {
 	var allErrs field.ErrorList
 	specPath := field.NewPath("spec")
 	if m.Spec.Bootstrap.ConfigRef == nil && m.Spec.Bootstrap.DataSecretName == nil {
-		allErrs = append(
-			allErrs,
-			field.Required(
-				specPath.Child("bootstrap", "data"),
-				"expected either spec.bootstrap.dataSecretName or spec.bootstrap.configRef to be populated",
-			),
-		)
+		// MachinePool Machines don't have a bootstrap configRef, so don't require it. The bootstrap config is instead owned by the MachinePool.
+		if !isMachinePoolMachine(m) {
+			allErrs = append(
+				allErrs,
+				field.Required(
+					specPath.Child("bootstrap", "data"),
+					"expected either spec.bootstrap.dataSecretName or spec.bootstrap.configRef to be populated",
+				),
+			)
+		}
 	}
 
 	if m.Spec.Bootstrap.ConfigRef != nil && m.Spec.Bootstrap.ConfigRef.Namespace != m.Namespace {
@@ -113,15 +161,18 @@ func (m *Machine) validate(old *Machine) error {
 		)
 	}
 
-	if m.Spec.InfrastructureRef.Namespace != m.Namespace {
-		allErrs = append(
-			allErrs,
-			field.Invalid(
-				specPath.Child("infrastructureRef", "namespace"),
-				m.Spec.InfrastructureRef.Namespace,
-				"must match metadata.namespace",
-			),
-		)
+	// InfraRef can be empty for MachinePool Machines so skip the check in that case.
+	if !isMachinePoolMachine(m) {
+		if m.Spec.InfrastructureRef.Namespace != m.Namespace {
+			allErrs = append(
+				allErrs,
+				field.Invalid(
+					specPath.Child("infrastructureRef", "namespace"),
+					m.Spec.InfrastructureRef.Namespace,
+					"must match metadata.namespace",
+				),
+			)
+		}
 	}
 
 	if old != nil && old.Spec.ClusterName != m.Spec.ClusterName {
@@ -142,3 +193,17 @@ func (m *Machine) validate(old *Machine) error {
 	}
 	return apierrors.NewInvalid(GroupVersion.WithKind("Machine").GroupKind(), m.Name, allErrs)
 }
+
+func isMachinePoolMachine(m *Machine) bool {
+	if m.OwnerReferences == nil {
+		return false
+	}
+
+	for _, owner := range m.OwnerReferences {
+		if owner.Kind == "MachinePool" {
+			return true
+		}
+	}
+
+	return false
+}

api/v1beta1/machine_webhook_test.go

@@ -17,14 +17,15 @@ limitations under the License.
 package v1beta1
 
 import (
+	"context"
 	"testing"
 
 	. "github.com/onsi/gomega"
+	admissionv1 "k8s.io/api/admission/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
-
-	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 func TestMachineDefault(t *testing.T) {
@@ -39,7 +40,10 @@ func TestMachineDefault(t *testing.T) {
 			Version:   pointer.String("1.17.5"),
 		},
 	}
-	t.Run("for Machine", utildefaulting.DefaultValidateTest(m))
+	scheme, err := SchemeBuilder.Build()
+	g.Expect(err).ToNot(HaveOccurred())
+	validator := MachineValidator(scheme)
+	t.Run("for Machine", defaultDefaulterTestCustomValidator(m, validator))
 	m.Default()
 
 	g.Expect(m.Labels[ClusterNameLabel]).To(Equal(m.Spec.ClusterName))
@@ -75,15 +79,25 @@ func TestMachineBootstrapValidation(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
+			scheme, err := SchemeBuilder.Build()
+			g.Expect(err).ToNot(HaveOccurred())
+			validator := MachineValidator(scheme)
+
+			ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
+				AdmissionRequest: admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
+				},
+			})
+
 			m := &Machine{
 				Spec: MachineSpec{Bootstrap: tt.bootstrap},
 			}
 			if tt.expectErr {
-				g.Expect(m.ValidateCreate()).NotTo(Succeed())
-				g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
+				g.Expect(validator.ValidateCreate(ctx, m)).NotTo(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, m, m)).NotTo(Succeed())
 			} else {
-				g.Expect(m.ValidateCreate()).To(Succeed())
-				g.Expect(m.ValidateUpdate(m)).To(Succeed())
+				g.Expect(validator.ValidateCreate(ctx, m)).To(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, m, m)).To(Succeed())
 			}
 		})
 	}
@@ -130,18 +144,27 @@ func TestMachineNamespaceValidation(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
+			scheme, err := SchemeBuilder.Build()
+			g.Expect(err).ToNot(HaveOccurred())
+			validator := MachineValidator(scheme)
+
+			ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
+				AdmissionRequest: admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
+				},
+			})
 
 			m := &Machine{
 				ObjectMeta: metav1.ObjectMeta{Namespace: tt.namespace},
 				Spec:       MachineSpec{Bootstrap: tt.bootstrap, InfrastructureRef: tt.infraRef},
 			}
 
 			if tt.expectErr {
-				g.Expect(m.ValidateCreate()).NotTo(Succeed())
-				g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
+				g.Expect(validator.ValidateCreate(ctx, m)).NotTo(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, m, m)).NotTo(Succeed())
 			} else {
-				g.Expect(m.ValidateCreate()).To(Succeed())
-				g.Expect(m.ValidateUpdate(m)).To(Succeed())
+				g.Expect(validator.ValidateCreate(ctx, m)).To(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, m, m)).To(Succeed())
 			}
 		})
 	}
@@ -171,6 +194,15 @@ func TestMachineClusterNameImmutable(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
+			scheme, err := SchemeBuilder.Build()
+			g.Expect(err).ToNot(HaveOccurred())
+			validator := MachineValidator(scheme)
+
+			ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
+				AdmissionRequest: admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
+				},
+			})
 
 			newMachine := &Machine{
 				Spec: MachineSpec{
@@ -186,9 +218,9 @@ func TestMachineClusterNameImmutable(t *testing.T) {
 			}
 
 			if tt.expectErr {
-				g.Expect(newMachine.ValidateUpdate(oldMachine)).NotTo(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, oldMachine, newMachine)).NotTo(Succeed())
 			} else {
-				g.Expect(newMachine.ValidateUpdate(oldMachine)).To(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, oldMachine, newMachine)).To(Succeed())
 			}
 		})
 	}
@@ -230,6 +262,15 @@ func TestMachineVersionValidation(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
+			scheme, err := SchemeBuilder.Build()
+			g.Expect(err).ToNot(HaveOccurred())
+			validator := MachineValidator(scheme)
+
+			ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
+				AdmissionRequest: admissionv1.AdmissionRequest{
+					Operation: admissionv1.Create,
+				},
+			})
 
 			m := &Machine{
 				Spec: MachineSpec{
@@ -239,12 +280,50 @@ func TestMachineVersionValidation(t *testing.T) {
 			}
 
 			if tt.expectErr {
-				g.Expect(m.ValidateCreate()).NotTo(Succeed())
-				g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
+				g.Expect(validator.ValidateCreate(ctx, m)).NotTo(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, m, m)).NotTo(Succeed())
 			} else {
-				g.Expect(m.ValidateCreate()).To(Succeed())
-				g.Expect(m.ValidateUpdate(m)).To(Succeed())
+				g.Expect(validator.ValidateCreate(ctx, m)).To(Succeed())
+				g.Expect(validator.ValidateUpdate(ctx, m, m)).To(Succeed())
 			}
 		})
 	}
 }
+
+// defaultDefaulterTestCustomVAlidator returns a new testing function to be used in tests to
+// make sure defaulting webhooks also pass validation tests on create, update and delete.
+// Note: The difference to util/defaulting.DefaultValidateTest is that this function takes an additional
+// CustomValidator as the validation is not implemented on the object directly.
+func defaultDefaulterTestCustomValidator(object admission.Defaulter, customValidator admission.CustomValidator) func(*testing.T) {
+	return func(t *testing.T) {
+		t.Helper()
+
+		createCopy := object.DeepCopyObject().(admission.Defaulter)
+		updateCopy := object.DeepCopyObject().(admission.Defaulter)
+		deleteCopy := object.DeepCopyObject().(admission.Defaulter)
+		defaultingUpdateCopy := updateCopy.DeepCopyObject().(admission.Defaulter)
+
+		ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{
+				Operation: admissionv1.Create,
+			},
+		})
+
+		t.Run("validate-on-create", func(t *testing.T) {
+			g := NewWithT(t)
+			createCopy.Default()
+			g.Expect(customValidator.ValidateCreate(ctx, createCopy)).To(Succeed())
+		})
+		t.Run("validate-on-update", func(t *testing.T) {
+			g := NewWithT(t)
+			defaultingUpdateCopy.Default()
+			updateCopy.Default()
+			g.Expect(customValidator.ValidateUpdate(ctx, defaultingUpdateCopy, updateCopy)).To(Succeed())
+		})
+		t.Run("validate-on-delete", func(t *testing.T) {
+			g := NewWithT(t)
+			deleteCopy.Default()
+			g.Expect(customValidator.ValidateDelete(ctx, deleteCopy)).To(Succeed())
+		})
+	}
+}