Merge pull request kubernetes-sigs#8842 from Jont828/mpm-capd

✨ Add MachinePool Machine implementation to CAPD components

Author: k8s-ci-robot

exp/internal/controllers/machinepool_controller_phases.go

@@ -47,6 +47,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	utilconversion "sigs.k8s.io/cluster-api/util/conversion"
+	"sigs.k8s.io/cluster-api/util/labels"
 	"sigs.k8s.io/cluster-api/util/labels/format"
 	"sigs.k8s.io/cluster-api/util/patch"
 )
@@ -485,12 +486,10 @@ func computeDesiredMachine(mp *expv1.MachinePool, infraMachine *unstructured.Uns
 func (r *MachinePoolReconciler) infraMachineToMachinePoolMapper(ctx context.Context, o client.Object) []ctrl.Request {
 	log := ctrl.LoggerFrom(ctx)
 
-	labels := o.GetLabels()
-	_, machinePoolOwned := labels[clusterv1.MachinePoolNameLabel]
-	if machinePoolOwned {
-		machinePool, err := utilexp.GetMachinePoolByLabels(ctx, r.Client, o.GetNamespace(), labels)
+	if labels.IsMachinePoolOwned(o) {
+		machinePool, err := utilexp.GetMachinePoolByLabels(ctx, r.Client, o.GetNamespace(), o.GetLabels())
 		if err != nil {
-			log.Error(err, "failed to get MachinePool for InfraMachine", "infraMachine", klog.KObj(o), "labels", labels)
+			log.Error(err, "failed to get MachinePool for InfraMachine", "infraMachine", klog.KObj(o), "labels", o.GetLabels())
 			return nil
 		}
 		if machinePool != nil {

internal/webhooks/machine.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/util/labels"
 	"sigs.k8s.io/cluster-api/util/version"
 )
 
@@ -120,7 +121,7 @@ func (webhook *Machine) validate(oldM, newM *clusterv1.Machine) error {
 	specPath := field.NewPath("spec")
 	if newM.Spec.Bootstrap.ConfigRef == nil && newM.Spec.Bootstrap.DataSecretName == nil {
 		// MachinePool Machines don't have a bootstrap configRef, so don't require it. The bootstrap config is instead owned by the MachinePool.
-		if !isMachinePoolMachine(newM) {
+		if !labels.IsMachinePoolOwned(newM) {
 			allErrs = append(
 				allErrs,
 				field.Required(
@@ -171,19 +172,3 @@ func (webhook *Machine) validate(oldM, newM *clusterv1.Machine) error {
 	}
 	return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("Machine").GroupKind(), newM.Name, allErrs)
 }
-
-func isMachinePoolMachine(m *clusterv1.Machine) bool {
-	if m.Labels != nil {
-		if _, ok := m.Labels[clusterv1.MachinePoolNameLabel]; ok {
-			return true
-		}
-	}
-
-	for _, owner := range m.OwnerReferences {
-		if owner.Kind == "MachinePool" {
-			return true
-		}
-	}
-
-	return false
-}

internal/webhooks/machine_test.go

@@ -220,60 +220,6 @@ func TestMachineClusterNameImmutable(t *testing.T) {
 	}
 }
 
-func TestIsMachinePoolMachine(t *testing.T) {
-	tests := []struct {
-		name    string
-		machine clusterv1.Machine
-		isMPM   bool
-	}{
-		{
-			name: "machine is a MachinePoolMachine",
-			machine: clusterv1.Machine{
-				ObjectMeta: metav1.ObjectMeta{
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							Kind: "MachinePool",
-						},
-					},
-				},
-			},
-			isMPM: true,
-		},
-		{
-			name: "machine is not a MachinePoolMachine",
-			machine: clusterv1.Machine{
-				ObjectMeta: metav1.ObjectMeta{
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							Kind: "NotMachinePool",
-						},
-					},
-				},
-			},
-			isMPM: false,
-		},
-		{
-			name: "machine is not a MachinePoolMachine, no owner references",
-			machine: clusterv1.Machine{
-				ObjectMeta: metav1.ObjectMeta{
-					OwnerReferences: nil,
-				},
-			},
-			isMPM: false,
-		},
-	}
-
-	for i := range tests {
-		tt := tests[i]
-		t.Run(tt.name, func(t *testing.T) {
-			g := NewWithT(t)
-
-			result := isMachinePoolMachine(&tt.machine)
-			g.Expect(result).To(Equal(tt.isMPM))
-		})
-	}
-}
-
 func TestMachineVersionValidation(t *testing.T) {
 	tests := []struct {
 		name      string

test/e2e/clusterclass_rollout.go

@@ -47,6 +47,7 @@ import (
 	"sigs.k8s.io/cluster-api/test/framework"
 	"sigs.k8s.io/cluster-api/test/framework/clusterctl"
 	"sigs.k8s.io/cluster-api/util"
+	"sigs.k8s.io/cluster-api/util/labels"
 	"sigs.k8s.io/cluster-api/util/patch"
 )
 
@@ -246,6 +247,8 @@ func ClusterClassRolloutSpec(ctx context.Context, inputGetter func() ClusterClas
 		})
 		By("Verifying all Machines are replaced through rollout")
 		Eventually(func(g Gomega) {
+			// Note: This excludes MachinePool Machines as they are not replaced by rollout yet.
+			// This is tracked by https://github.com/kubernetes-sigs/cluster-api/issues/8858.
 			machinesAfterUpgrade := getMachinesByCluster(ctx, input.BootstrapClusterProxy.GetClient(), clusterResources.Cluster)
 			g.Expect(machinesAfterUpgrade.HasAny(machinesBeforeUpgrade.UnsortedList()...)).To(BeFalse(), "All Machines must be replaced through rollout")
 		}, input.E2EConfig.GetIntervals(specName, "wait-control-plane")...).Should(Succeed())
@@ -898,15 +901,19 @@ func mustMetadata(metadata *clusterv1.ObjectMeta, err error) *clusterv1.ObjectMe
 }
 
 // getMachinesByCluster gets the Machines of a Cluster and returns them as a Set of Machine names.
+// Note: This excludes MachinePool Machines as they are not replaced by rollout yet.
 func getMachinesByCluster(ctx context.Context, client client.Client, cluster *clusterv1.Cluster) sets.Set[string] {
 	machines := sets.Set[string]{}
 	machinesByCluster := framework.GetMachinesByCluster(ctx, framework.GetMachinesByClusterInput{
 		Lister:      client,
 		ClusterName: cluster.Name,
 		Namespace:   cluster.Namespace,
 	})
-	for _, m := range machinesByCluster {
-		machines.Insert(m.Name)
+	for i := range machinesByCluster {
+		m := machinesByCluster[i]
+		if !labels.IsMachinePoolOwned(&m) {
+			machines.Insert(m.Name)
+		}
 	}
 	return machines
 }
@@ -935,7 +942,7 @@ func getMDTopology(cluster *clusterv1.Cluster, md *clusterv1.MachineDeployment)
 	return nil
 }
 
-// getMPClass looks up the MachinePoolClass for a md in the ClusterClass.
+// getMPClass looks up the MachinePoolClass for a MachinePool in the ClusterClass.
 func getMPClass(cluster *clusterv1.Cluster, clusterClass *clusterv1.ClusterClass, mp *expv1.MachinePool) *clusterv1.MachinePoolClass {
 	mpTopology := getMPTopology(cluster, mp)
 

test/framework/ownerreference_helpers.go

@@ -39,6 +39,7 @@ import (
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+	infraexpv1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/patch"
 )
 
@@ -288,6 +289,8 @@ var (
 	dockerMachinePoolTemplateKind = "DockerMachinePoolTemplate"
 	dockerClusterKind             = "DockerCluster"
 	dockerClusterTemplateKind     = "DockerClusterTemplate"
+
+	dockerMachinePoolController = metav1.OwnerReference{Kind: dockerMachinePoolKind, APIVersion: infraexpv1.GroupVersion.String()}
 )
 
 // DockerInfraOwnerReferenceAssertions maps Docker Infrastructure types to functions which return an error if the passed
@@ -296,8 +299,9 @@ var (
 // That document should be updated if these references change.
 var DockerInfraOwnerReferenceAssertions = map[string]func([]metav1.OwnerReference) error{
 	dockerMachineKind: func(owners []metav1.OwnerReference) error {
-		// The DockerMachine must be owned and controlled by a Machine.
-		return HasExactOwners(owners, machineController)
+		// The DockerMachine must be owned and controlled by a Machine or a DockerMachinePool.
+		return HasOneOfExactOwners(owners, []metav1.OwnerReference{machineController}, []metav1.OwnerReference{machineController, dockerMachinePoolController})
+
 	},
 	dockerMachineTemplateKind: func(owners []metav1.OwnerReference) error {
 		// Base DockerMachineTemplates referenced in a ClusterClass must be owned by the ClusterClass.

test/infrastructure/docker/config/crd/bases/infrastructure.cluster.x-k8s.io_dockermachinepools.yaml

@@ -51,6 +51,15 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - cluster.x-k8s.io
+  resources:
+  - machines
+  verbs:
+  - delete
+  - get
+  - list
+  - watch
 - apiGroups:
   - infrastructure.cluster.x-k8s.io
   resources:

test/infrastructure/docker/config/rbac/role.yaml

@@ -17,21 +17,40 @@ limitations under the License.
 package v1alpha4
 
 import (
+	apiconversion "k8s.io/apimachinery/pkg/conversion"
 	"sigs.k8s.io/controller-runtime/pkg/conversion"
 
 	infraexpv1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/exp/api/v1beta1"
+	utilconversion "sigs.k8s.io/cluster-api/util/conversion"
 )
 
 func (src *DockerMachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	dst := dstRaw.(*infraexpv1.DockerMachinePool)
 
-	return Convert_v1alpha4_DockerMachinePool_To_v1beta1_DockerMachinePool(src, dst, nil)
+	if err := Convert_v1alpha4_DockerMachinePool_To_v1beta1_DockerMachinePool(src, dst, nil); err != nil {
+		return err
+	}
+
+	// Manually restore data.
+	restored := &infraexpv1.DockerMachinePool{}
+	if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
+		return err
+	}
+
+	dst.Status.InfrastructureMachineKind = restored.Status.InfrastructureMachineKind
+
+	return nil
 }
 
 func (dst *DockerMachinePool) ConvertFrom(srcRaw conversion.Hub) error {
 	src := srcRaw.(*infraexpv1.DockerMachinePool)
 
-	return Convert_v1beta1_DockerMachinePool_To_v1alpha4_DockerMachinePool(src, dst, nil)
+	if err := Convert_v1beta1_DockerMachinePool_To_v1alpha4_DockerMachinePool(src, dst, nil); err != nil {
+		return err
+	}
+
+	// Preserve Hub data on down-conversion except for metadata
+	return utilconversion.MarshalData(src, dst)
 }
 
 func (src *DockerMachinePoolList) ConvertTo(dstRaw conversion.Hub) error {
@@ -45,3 +64,8 @@ func (dst *DockerMachinePoolList) ConvertFrom(srcRaw conversion.Hub) error {
 
 	return Convert_v1beta1_DockerMachinePoolList_To_v1alpha4_DockerMachinePoolList(src, dst, nil)
 }
+
+func Convert_v1beta1_DockerMachinePoolStatus_To_v1alpha4_DockerMachinePoolStatus(in *infraexpv1.DockerMachinePoolStatus, out *DockerMachinePoolStatus, s apiconversion.Scope) error {
+	// NOTE: custom conversion func is required because Status.InfrastructureMachineKind has been added in v1beta1.
+	return autoConvert_v1beta1_DockerMachinePoolStatus_To_v1alpha4_DockerMachinePoolStatus(in, out, s)
+}

test/infrastructure/docker/exp/api/v1alpha4/conversion.go

@@ -82,6 +82,10 @@ type DockerMachinePoolStatus struct {
 	// Conditions defines current service state of the DockerMachinePool.
 	// +optional
 	Conditions clusterv1.Conditions `json:"conditions,omitempty"`
+
+	// InfrastructureMachineKind is the kind of the infrastructure resources behind MachinePool Machines.
+	// +optional
+	InfrastructureMachineKind string `json:"infrastructureMachineKind,omitempty"`
 }
 
 // DockerMachinePoolInstanceStatus contains status information about a DockerMachinePool.
@@ -130,13 +134,13 @@ type DockerMachinePool struct {
 }
 
 // GetConditions returns the set of conditions for this object.
-func (c *DockerMachinePool) GetConditions() clusterv1.Conditions {
-	return c.Status.Conditions
+func (d *DockerMachinePool) GetConditions() clusterv1.Conditions {
+	return d.Status.Conditions
 }
 
 // SetConditions sets the conditions on this object.
-func (c *DockerMachinePool) SetConditions(conditions clusterv1.Conditions) {
-	c.Status.Conditions = conditions
+func (d *DockerMachinePool) SetConditions(conditions clusterv1.Conditions) {
+	d.Status.Conditions = conditions
 }
 
 // +kubebuilder:object:root=true