Implement template deletion for topology-owned MD and MS

Signed-off-by: Stefan Büringer [email protected]

Co-authored-by: fabriziopandini <[email protected]>

api/v1alpha4/machinedeployment_types.go

@@ -21,6 +21,12 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
+const (
+	// MachineDeploymentTopologyFinalizer is the finalizer used by the topology MachineDeployment controller to
+	// clean up referenced template resources if necessary when a MachineDeployment is being deleted.
+	MachineDeploymentTopologyFinalizer = "machinedeployment.topology.cluster.x-k8s.io"
+)
+
 // MachineDeploymentStrategyType defines the type of MachineDeployment rollout strategies.
 type MachineDeploymentStrategyType string
 

api/v1alpha4/machineset_types.go

@@ -24,6 +24,12 @@ import (
 	capierrors "sigs.k8s.io/cluster-api/errors"
 )
 
+const (
+	// MachineSetTopologyFinalizer is the finalizer used by the topology MachineDeployment controller to
+	// clean up referenced template resources if necessary when a MachineSet is being deleted.
+	MachineSetTopologyFinalizer = "machineset.topology.cluster.x-k8s.io"
+)
+
 // ANCHOR: MachineSetSpec
 
 // MachineSetSpec defines the desired state of MachineSet.

config/rbac/role.yaml

@@ -71,6 +71,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - cluster.x-k8s.io
+  resources:
+  - clusters
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - cluster.x-k8s.io
   resources:
@@ -98,6 +106,18 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - cluster.x-k8s.io
+  resources:
+  - machinedeployments
+  - machinedeployments/finalizers
+  verbs:
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - cluster.x-k8s.io
   resources:
@@ -162,6 +182,18 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - cluster.x-k8s.io
+  resources:
+  - machinesets
+  - machinesets/finalizers
+  verbs:
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - cluster.x-k8s.io
   resources:

controllers/external/util.go

@@ -50,6 +50,19 @@ func Get(ctx context.Context, c client.Client, ref *corev1.ObjectReference, name
 	return obj, nil
 }
 
+// Delete uses the client and reference to delete an external, unstructured object.
+func Delete(ctx context.Context, c client.Client, ref *corev1.ObjectReference) error {
+	obj := new(unstructured.Unstructured)
+	obj.SetAPIVersion(ref.APIVersion)
+	obj.SetKind(ref.Kind)
+	obj.SetName(ref.Name)
+	obj.SetNamespace(ref.Namespace)
+	if err := c.Delete(ctx, obj); err != nil {
+		return errors.Wrapf(err, "failed to delete %s external object %q/%q", obj.GetKind(), obj.GetNamespace(), obj.GetName())
+	}
+	return nil
+}
+
 // CloneTemplateInput is the input to CloneTemplate.
 type CloneTemplateInput struct {
 	// Client is the controller runtime client.

controllers/topology/desired_state.go

@@ -156,11 +156,16 @@ func computeControlPlane(_ context.Context, s *scope.Scope, infrastructureMachin
 
 		// Compute the labels and annotations to be applied to ControlPlane machines.
 		// We merge the labels and annotations from topology and ClusterClass.
+		// We also add the cluster-name and the topology owned labels.
 		topologyMetadata := s.Blueprint.Topology.ControlPlane.Metadata
 		clusterClassMetadata := s.Blueprint.ClusterClass.Spec.ControlPlane.Metadata
+
+		machineLabels := mergeMap(topologyMetadata.Labels, clusterClassMetadata.Labels)
+		machineLabels[clusterv1.ClusterLabelName] = cluster.Name
+		machineLabels[clusterv1.ClusterTopologyOwnedLabel] = ""
 		if err := contract.ControlPlane().MachineTemplate().Metadata().Set(controlPlane,
 			&clusterv1.ObjectMeta{
-				Labels:      mergeMap(topologyMetadata.Labels, clusterClassMetadata.Labels),
+				Labels:      machineLabels,
 				Annotations: mergeMap(topologyMetadata.Annotations, clusterClassMetadata.Annotations),
 			}); err != nil {
 			return nil, errors.Wrap(err, "failed to spec.machineTemplate.metadata in the ControlPlane object")
@@ -294,6 +299,13 @@ func computeMachineDeployment(_ context.Context, s *scope.Scope, machineDeployme
 	labels[clusterv1.ClusterTopologyMachineDeploymentLabelName] = machineDeploymentTopology.Name
 	desiredMachineDeploymentObj.SetLabels(labels)
 
+	// Also set the labels in .spec.template.labels so that they are propagated to
+	// MachineSet.labels and MachineSet.spec.template.labels and thus to Machine.labels.
+	// Note: the labels in MachineSet are used to properly cleanup templates when the MachineSet is deleted.
+	desiredMachineDeploymentObj.Spec.Template.Labels[clusterv1.ClusterLabelName] = s.Current.Cluster.Name
+	desiredMachineDeploymentObj.Spec.Template.Labels[clusterv1.ClusterTopologyOwnedLabel] = ""
+	desiredMachineDeploymentObj.Spec.Template.Labels[clusterv1.ClusterTopologyMachineDeploymentLabelName] = machineDeploymentTopology.Name
+
 	// Set the desired replicas.
 	desiredMachineDeploymentObj.Spec.Replicas = machineDeploymentTopology.Replicas
 

controllers/topology/desired_state_test.go

@@ -345,8 +345,12 @@ func TestComputeControlPlane(t *testing.T) {
 		})
 		gotMetadata, err := contract.ControlPlane().MachineTemplate().Metadata().Get(obj)
 		g.Expect(err).ToNot(HaveOccurred())
+
+		expectedLabels := mergeMap(scope.Current.Cluster.Spec.Topology.ControlPlane.Metadata.Labels, blueprint.ClusterClass.Spec.ControlPlane.Metadata.Labels)
+		expectedLabels[clusterv1.ClusterLabelName] = cluster.Name
+		expectedLabels[clusterv1.ClusterTopologyOwnedLabel] = ""
 		g.Expect(gotMetadata).To(Equal(&clusterv1.ObjectMeta{
-			Labels:      mergeMap(scope.Current.Cluster.Spec.Topology.ControlPlane.Metadata.Labels, blueprint.ClusterClass.Spec.ControlPlane.Metadata.Labels),
+			Labels:      expectedLabels,
 			Annotations: mergeMap(scope.Current.Cluster.Spec.Topology.ControlPlane.Metadata.Annotations, blueprint.ClusterClass.Spec.ControlPlane.Metadata.Annotations),
 		}))
 
@@ -504,9 +508,11 @@ func TestComputeMachineDeployment(t *testing.T) {
 		g.Expect(actualMd.Name).To(ContainSubstring("big-pool-of-machines"))
 
 		g.Expect(actualMd.Labels).To(HaveKeyWithValue(clusterv1.ClusterTopologyMachineDeploymentLabelName, "big-pool-of-machines"))
+		g.Expect(actualMd.Labels).To(HaveKey(clusterv1.ClusterTopologyOwnedLabel))
 
 		g.Expect(actualMd.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue("foo", "baz"))
 		g.Expect(actualMd.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue("fizz", "buzz"))
+		g.Expect(actualMd.Spec.Template.ObjectMeta.Labels).To(HaveKey(clusterv1.ClusterTopologyOwnedLabel))
 		g.Expect(actualMd.Spec.Template.Spec.InfrastructureRef.Name).ToNot(Equal("linux-worker-inframachinetemplate"))
 		g.Expect(actualMd.Spec.Template.Spec.Bootstrap.ConfigRef.Name).ToNot(Equal("linux-worker-bootstraptemplate"))
 	})
@@ -549,9 +555,11 @@ func TestComputeMachineDeployment(t *testing.T) {
 		g.Expect(actualMd.Name).To(Equal("existing-deployment-1"))
 
 		g.Expect(actualMd.Labels).To(HaveKeyWithValue(clusterv1.ClusterTopologyMachineDeploymentLabelName, "big-pool-of-machines"))
+		g.Expect(actualMd.Labels).To(HaveKey(clusterv1.ClusterTopologyOwnedLabel))
 
 		g.Expect(actualMd.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue("foo", "baz"))
 		g.Expect(actualMd.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue("fizz", "buzz"))
+		g.Expect(actualMd.Spec.Template.ObjectMeta.Labels).To(HaveKey(clusterv1.ClusterTopologyOwnedLabel))
 		g.Expect(actualMd.Spec.Template.Spec.InfrastructureRef.Name).To(Equal("linux-worker-inframachinetemplate"))
 		g.Expect(actualMd.Spec.Template.Spec.Bootstrap.ConfigRef.Name).To(Equal("linux-worker-bootstraptemplate"))
 	})