Merge pull request #10277 from fabriziopandini/improve-handling-of-re…

…concileCP-errors 🐛 Improve handling of topology orphaned objects
kubernetes-sigs · Mar 27, 2024 · 3a16912 · 3a16912
2 parents 128914b + 5f141e8
commit 3a16912
Show file tree

Hide file tree

Showing 4 changed files with 665 additions and 94 deletions.
diff --git a/internal/controllers/topology/cluster/desired_state.go b/internal/controllers/topology/cluster/desired_state.go
@@ -107,7 +107,7 @@ func (r *Reconciler) computeDesiredState(ctx context.Context, s *scope.Scope) (*
 			ctx,
 			desiredState.ControlPlane.Object,
 			selectorForControlPlaneMHC(),
-			s.Current.Cluster.Name,
+			s.Current.Cluster,
 			s.Blueprint.ControlPlaneMachineHealthCheckClass())
 	}
 
@@ -796,7 +796,7 @@ func computeMachineDeployment(ctx context.Context, s *scope.Scope, machineDeploy
 			ctx,
 			desiredMachineDeploymentObj,
 			selectorForMachineDeploymentMHC(desiredMachineDeploymentObj),
-			s.Current.Cluster.Name,
+			s.Current.Cluster,
 			s.Blueprint.MachineDeploymentMachineHealthCheckClass(&machineDeploymentTopology))
 	}
 	return desiredMachineDeployment, nil
@@ -1357,7 +1357,7 @@ func ownerReferenceTo(obj client.Object, gvk schema.GroupVersionKind) *metav1.Ow
 	}
 }
 
-func computeMachineHealthCheck(ctx context.Context, healthCheckTarget client.Object, selector *metav1.LabelSelector, clusterName string, check *clusterv1.MachineHealthCheckClass) *clusterv1.MachineHealthCheck {
+func computeMachineHealthCheck(ctx context.Context, healthCheckTarget client.Object, selector *metav1.LabelSelector, cluster *clusterv1.Cluster, check *clusterv1.MachineHealthCheckClass) *clusterv1.MachineHealthCheck {
 	// Create a MachineHealthCheck with the spec given in the ClusterClass.
 	mhc := &clusterv1.MachineHealthCheck{
 		TypeMeta: metav1.TypeMeta{
@@ -1370,9 +1370,14 @@ func computeMachineHealthCheck(ctx context.Context, healthCheckTarget client.Obj
 			Labels: map[string]string{
 				clusterv1.ClusterTopologyOwnedLabel: "",
 			},
+			// Note: we are adding an ownerRef to Cluster so the MHC will be automatically garbage collected
+			// in case deletion is triggered before an object reconcile happens.
+			OwnerReferences: []metav1.OwnerReference{
+				*ownerReferenceTo(cluster, clusterv1.GroupVersion.WithKind("Cluster")),
+			},
 		},
 		Spec: clusterv1.MachineHealthCheckSpec{
-			ClusterName:         clusterName,
+			ClusterName:         cluster.Name,
 			Selector:            *selector,
 			UnhealthyConditions: check.UnhealthyConditions,
 			MaxUnhealthy:        check.MaxUnhealthy,

diff --git a/internal/controllers/topology/cluster/desired_state_test.go b/internal/controllers/topology/cluster/desired_state_test.go
@@ -2874,7 +2874,7 @@ func Test_computeMachineHealthCheck(t *testing.T) {
 		"foo": "bar",
 	}}
 	healthCheckTarget := builder.MachineDeployment("ns1", "md1").Build()
-	clusterName := "cluster1"
+	cluster := builder.Cluster("ns1", "cluster1").Build()
 	want := &clusterv1.MachineHealthCheck{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: clusterv1.GroupVersion.String(),
@@ -2888,9 +2888,12 @@ func Test_computeMachineHealthCheck(t *testing.T) {
 				"cluster.x-k8s.io/cluster-name":     "cluster1",
 				clusterv1.ClusterTopologyOwnedLabel: "",
 			},
+			OwnerReferences: []metav1.OwnerReference{
+				*ownerReferenceTo(cluster, clusterv1.GroupVersion.WithKind("Cluster")),
+			},
 		},
 		Spec: clusterv1.MachineHealthCheckSpec{
-			ClusterName: "cluster1",
+			ClusterName: cluster.Name,
 			Selector: metav1.LabelSelector{MatchLabels: map[string]string{
 				"foo": "bar",
 			}},
@@ -2916,7 +2919,7 @@ func Test_computeMachineHealthCheck(t *testing.T) {
 	t.Run("set all fields correctly", func(t *testing.T) {
 		g := NewWithT(t)
 
-		got := computeMachineHealthCheck(ctx, healthCheckTarget, selector, clusterName, mhcSpec)
+		got := computeMachineHealthCheck(ctx, healthCheckTarget, selector, cluster, mhcSpec)
 
 		g.Expect(got).To(BeComparableTo(want), cmp.Diff(got, want))
 	})