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

…concileCP-errors-1.5 [release-1.5] 🐛 Improve handling of topology orphaned objects
kubernetes-sigs · Apr 3, 2024 · 7527fa1 · 7527fa1
2 parents 64e275a + d78b823
commit 7527fa1
Show file tree

Hide file tree

Showing 4 changed files with 562 additions and 88 deletions.
diff --git a/internal/controllers/topology/cluster/desired_state.go b/internal/controllers/topology/cluster/desired_state.go
@@ -86,7 +86,7 @@ func (r *Reconciler) computeDesiredState(ctx context.Context, s *scope.Scope) (*
 		desiredState.ControlPlane.MachineHealthCheck = computeMachineHealthCheck(
 			desiredState.ControlPlane.Object,
 			selectorForControlPlaneMHC(),
-			s.Current.Cluster.Name,
+			s.Current.Cluster,
 			s.Blueprint.ControlPlaneMachineHealthCheckClass())
 	}
 
@@ -764,7 +764,7 @@ func computeMachineDeployment(_ context.Context, s *scope.Scope, machineDeployme
 		desiredMachineDeployment.MachineHealthCheck = computeMachineHealthCheck(
 			desiredMachineDeploymentObj,
 			selectorForMachineDeploymentMHC(desiredMachineDeploymentObj),
-			s.Current.Cluster.Name,
+			s.Current.Cluster,
 			s.Blueprint.MachineDeploymentMachineHealthCheckClass(&machineDeploymentTopology))
 	}
 	return desiredMachineDeployment, nil
@@ -1020,7 +1020,7 @@ func ownerReferenceTo(obj client.Object) *metav1.OwnerReference {
 	}
 }
 
-func computeMachineHealthCheck(healthCheckTarget client.Object, selector *metav1.LabelSelector, clusterName string, check *clusterv1.MachineHealthCheckClass) *clusterv1.MachineHealthCheck {
+func computeMachineHealthCheck(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{
@@ -1033,9 +1033,14 @@ func computeMachineHealthCheck(healthCheckTarget client.Object, selector *metav1
 			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),
+			},
 		},
 		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
@@ -2239,7 +2239,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{
 			Kind:       clusterv1.GroupVersion.WithKind("MachineHealthCheck").Kind,
@@ -2253,9 +2253,12 @@ func Test_computeMachineHealthCheck(t *testing.T) {
 				"cluster.x-k8s.io/cluster-name":     "cluster1",
 				clusterv1.ClusterTopologyOwnedLabel: "",
 			},
+			OwnerReferences: []metav1.OwnerReference{
+				*ownerReferenceTo(cluster),
+			},
 		},
 		Spec: clusterv1.MachineHealthCheckSpec{
-			ClusterName: "cluster1",
+			ClusterName: cluster.Name,
 			Selector: metav1.LabelSelector{MatchLabels: map[string]string{
 				"foo": "bar",
 			}},
@@ -2281,7 +2284,7 @@ func Test_computeMachineHealthCheck(t *testing.T) {
 	t.Run("set all fields correctly", func(t *testing.T) {
 		g := NewWithT(t)
 
-		got := computeMachineHealthCheck(healthCheckTarget, selector, clusterName, mhcSpec)
+		got := computeMachineHealthCheck(healthCheckTarget, selector, cluster, mhcSpec)
 
 		g.Expect(got).To(Equal(want), cmp.Diff(got, want))
 	})