Improve message for cluster delete

kubernetes-sigs · Nov 12, 2024 · 889583c · 889583c
1 parent be2406e
commit 889583c
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 14 deletions.
diff --git a/internal/controllers/cluster/cluster_controller.go b/internal/controllers/cluster/cluster_controller.go
@@ -390,10 +390,15 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) (reconcile.R
 
 	if descendantCount := s.descendants.objectsPendingDeleteCount(cluster); descendantCount > 0 {
 		indirect := descendantCount - len(children)
-		log.Info("Cluster still has descendants - need to requeue", "descendants", s.descendants.objectsPendingDeleteNames(cluster), "indirect descendants count", indirect)
+		names := s.descendants.objectsPendingDeleteNames(cluster)
+
+		log.Info("Cluster still has descendants - need to requeue", "descendants", strings.Join(names, "; "), "indirect descendants count", indirect)
 
 		s.deletingReason = clusterv1.ClusterDeletingWaitingForWorkersDeletionV1Beta2Reason
-		s.deletingMessage = s.descendants.objectsPendingDeleteNames(cluster)
+		for i := range names {
+			names[i] = "* " + names[i]
+		}
+		s.deletingMessage = strings.Join(names, "\n")
 
 		// Requeue so we can check the next time to see if there are still any descendants left.
 		return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil
@@ -511,7 +516,7 @@ func (c *clusterDescendants) objectsPendingDeleteCount(cluster *clusterv1.Cluste
 
 // objectsPendingDeleteNames return the names of descendants pending delete.
 // Note: infrastructure cluster, control plane object and its controlled machines are not included.
-func (c *clusterDescendants) objectsPendingDeleteNames(cluster *clusterv1.Cluster) string {
+func (c *clusterDescendants) objectsPendingDeleteNames(cluster *clusterv1.Cluster) []string {
 	descendants := make([]string, 0)
 	if cluster.Spec.ControlPlaneRef == nil {
 		controlPlaneMachineNames := make([]string, len(c.controlPlaneMachines))
@@ -557,7 +562,7 @@ func (c *clusterDescendants) objectsPendingDeleteNames(cluster *clusterv1.Cluste
 		sort.Strings(workerMachineNames)
 		descendants = append(descendants, "Worker Machines: "+clog.StringListToString(workerMachineNames))
 	}
-	return strings.Join(descendants, "; ")
+	return descendants
 }
 
 // getDescendants collects all MachineDeployments, MachineSets, MachinePools and Machines for the cluster.

diff --git a/internal/controllers/cluster/cluster_controller_status_test.go b/internal/controllers/cluster/cluster_controller_status_test.go
@@ -1648,15 +1648,21 @@ func TestDeletingCondition(t *testing.T) {
 			},
 		},
 		{
-			name:            "deletionTimestamp set (some reason/message reported)",
-			cluster:         fakeCluster("c", deleted(true)),
-			deletingReason:  clusterv1.ClusterDeletingWaitingForBeforeDeleteHookV1Beta2Reason,
-			deletingMessage: "Some message",
+			name:           "deletionTimestamp set (some reason/message reported)",
+			cluster:        fakeCluster("c", deleted(true)),
+			deletingReason: clusterv1.ClusterDeletingWaitingForBeforeDeleteHookV1Beta2Reason,
+			deletingMessage: "* Control plane Machines: cp1, cp2, cp3\n" +
+				"* MachineDeployments: md1, md2\n" +
+				"* MachineSets: ms1, ms2\n" +
+				"* Worker Machines: w1, w2, w3, w4, w5, ... (3 more)",
 			expectCondition: metav1.Condition{
-				Type:    clusterv1.ClusterDeletingV1Beta2Condition,
-				Status:  metav1.ConditionTrue,
-				Reason:  clusterv1.ClusterDeletingWaitingForBeforeDeleteHookV1Beta2Reason,
-				Message: "Some message",
+				Type:   clusterv1.ClusterDeletingV1Beta2Condition,
+				Status: metav1.ConditionTrue,
+				Reason: clusterv1.ClusterDeletingWaitingForBeforeDeleteHookV1Beta2Reason,
+				Message: "* Control plane Machines: cp1, cp2, cp3\n" +
+					"* MachineDeployments: md1, md2\n" +
+					"* MachineSets: ms1, ms2\n" +
+					"* Worker Machines: w1, w2, w3, w4, w5, ... (3 more)",
 			},
 		},
 	}
@@ -1759,6 +1765,63 @@ func TestSetAvailableCondition(t *testing.T) {
 				Message: "",
 			},
 		},
+		{
+			name: "Handles multiline conditions",
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "machine-test",
+					Namespace: metav1.NamespaceDefault,
+				},
+				Spec: clusterv1.ClusterSpec{
+					Topology: nil, // not using CC
+				},
+				Status: clusterv1.ClusterStatus{
+					V1Beta2: &clusterv1.ClusterV1Beta2Status{
+						Conditions: []metav1.Condition{
+							{
+								Type:   clusterv1.ClusterInfrastructureReadyV1Beta2Condition,
+								Status: metav1.ConditionTrue,
+								Reason: "Foo",
+							},
+							{
+								Type:   clusterv1.ClusterControlPlaneAvailableV1Beta2Condition,
+								Status: metav1.ConditionTrue,
+								Reason: "Foo",
+							},
+							{
+								Type:   clusterv1.ClusterWorkersAvailableV1Beta2Condition,
+								Status: metav1.ConditionTrue,
+								Reason: "Foo",
+							},
+							{
+								Type:   clusterv1.ClusterRemoteConnectionProbeV1Beta2Condition,
+								Status: metav1.ConditionTrue,
+								Reason: "Foo",
+							},
+							{
+								Type:   clusterv1.ClusterDeletingV1Beta2Condition,
+								Status: metav1.ConditionTrue,
+								Reason: clusterv1.ClusterDeletingWaitingForWorkersDeletionV1Beta2Reason,
+								Message: "* Control plane Machines: cp1, cp2, cp3\n" +
+									"* MachineDeployments: md1, md2\n" +
+									"* MachineSets: ms1, ms2\n" +
+									"* Worker Machines: w1, w2, w3, w4, w5, ... (3 more)",
+							},
+						},
+					},
+				},
+			},
+			expectCondition: metav1.Condition{
+				Type:   clusterv1.ClusterAvailableV1Beta2Condition,
+				Status: metav1.ConditionFalse,
+				Reason: clusterv1.ClusterDeletingWaitingForWorkersDeletionV1Beta2Reason,
+				Message: "* Deleting: \n" +
+					"  * Control plane Machines: cp1, cp2, cp3\n" +
+					"  * MachineDeployments: md1, md2\n" +
+					"  * MachineSets: ms1, ms2\n" +
+					"  * Worker Machines: w1, w2, w3, w4, w5, ... (3 more)",
+			},
+		},
 		{
 			name: "TopologyReconciled is required when the cluster is using CC",
 			cluster: &clusterv1.Cluster{

diff --git a/internal/controllers/cluster/cluster_controller_test.go b/internal/controllers/cluster/cluster_controller_test.go
@@ -886,15 +886,15 @@ func TestObjectsPendingDelete(t *testing.T) {
 
 		c := &clusterv1.Cluster{}
 		g.Expect(d.objectsPendingDeleteCount(c)).To(Equal(17))
-		g.Expect(d.objectsPendingDeleteNames(c)).To(Equal("Control plane Machines: cp1, cp2, cp3; MachineDeployments: md1, md2; MachineSets: ms1, ms2; MachinePools: mp1, mp2; Worker Machines: w1, w2, w3, w4, w5, ... (3 more)"))
+		g.Expect(d.objectsPendingDeleteNames(c)).To(Equal([]string{"Control plane Machines: cp1, cp2, cp3", "MachineDeployments: md1, md2", "MachineSets: ms1, ms2", "MachinePools: mp1, mp2", "Worker Machines: w1, w2, w3, w4, w5, ... (3 more)"}))
 	})
 
 	t.Run("With a control plane object", func(t *testing.T) {
 		g := NewWithT(t)
 
 		c := &clusterv1.Cluster{Spec: clusterv1.ClusterSpec{ControlPlaneRef: &corev1.ObjectReference{Kind: "SomeKind"}}}
 		g.Expect(d.objectsPendingDeleteCount(c)).To(Equal(14))
-		g.Expect(d.objectsPendingDeleteNames(c)).To(Equal("MachineDeployments: md1, md2; MachineSets: ms1, ms2; MachinePools: mp1, mp2; Worker Machines: w1, w2, w3, w4, w5, ... (3 more)"))
+		g.Expect(d.objectsPendingDeleteNames(c)).To(Equal([]string{"MachineDeployments: md1, md2", "MachineSets: ms1, ms2", "MachinePools: mp1, mp2", "Worker Machines: w1, w2, w3, w4, w5, ... (3 more)"}))
 	})
 }