🐛 Fix multiline Ready condition in clusterctl describe for v1beta2

cmd/clusterctl/cmd/describe_cluster.go

@@ -304,9 +304,10 @@ func addObjectRowV1Beta2(prefix string, tbl *tablewriter.Table, objectTree *tree
 		rowDescriptor.age,
 		msg0})
 
+	multilinePrefix := getRootMultiLineObjectPrefix(obj, objectTree, prefix)
 	for _, m := range msg[1:] {
 		tbl.Append([]string{
-			getMultilinePrefix(gray.Sprint(prefix)),
+			gray.Sprint(multilinePrefix),
 			"",
 			"",
 			"",
@@ -324,7 +325,14 @@ func addObjectRowV1Beta2(prefix string, tbl *tablewriter.Table, objectTree *tree
 
 	// Add a row for each object's children, taking care of updating the tree view prefix.
 	childrenObj := objectTree.GetObjectsByParent(obj.GetUID())
+	childrenObj = orderChildrenObjects(childrenObj)
 
+	for i, child := range childrenObj {
+		addObjectRowV1Beta2(getChildPrefix(prefix, i, len(childrenObj)), tbl, objectTree, child)
+	}
+}
+
+func orderChildrenObjects(childrenObj []ctrlclient.Object) []ctrlclient.Object {
 	// printBefore returns true if children[i] should be printed before children[j]. Objects are sorted by z-order and
 	// row name such that objects with higher z-order are printed first, and objects with the same z-order are
 	// printed in alphabetical order.
@@ -336,10 +344,7 @@ func addObjectRowV1Beta2(prefix string, tbl *tablewriter.Table, objectTree *tree
 		return tree.GetZOrder(childrenObj[i]) > tree.GetZOrder(childrenObj[j])
 	}
 	sort.Slice(childrenObj, printBefore)
-
-	for i, child := range childrenObj {
-		addObjectRowV1Beta2(getChildPrefix(prefix, i, len(childrenObj)), tbl, objectTree, child)
-	}
+	return childrenObj
 }
 
 // addObjectRow add a row for a given object, and recursively for all the object's children.
@@ -452,7 +457,7 @@ func addOtherConditionsV1Beta2(prefix string, tbl *tablewriter.Table, objectTree
 
 		for _, m := range msg[1:] {
 			tbl.Append([]string{
-				gray.Sprint(getMultilinePrefix(childPrefix)),
+				gray.Sprint(getMultilineConditionPrefix(childPrefix)),
 				"",
 				"",
 				"",
@@ -510,8 +515,8 @@ func getChildPrefix(currentPrefix string, childIndex, childCount int) string {
 	return nextPrefix + lastElemPrefix
 }
 
-// getMultilinePrefix return the tree view prefix for a multiline condition.
-func getMultilinePrefix(currentPrefix string) string {
+// getMultilineConditionPrefix return the tree view prefix for a multiline condition.
+func getMultilineConditionPrefix(currentPrefix string) string {
 	// All ├─ should be replaced by |, so all the existing hierarchic dependencies are carried on
 	if strings.HasSuffix(currentPrefix, firstElemPrefix) {
 		return strings.TrimSuffix(currentPrefix, firstElemPrefix) + pipe
@@ -524,6 +529,63 @@ func getMultilinePrefix(currentPrefix string) string {
 	return "?"
 }
 
+// getRootMultiLineObjectPrefix return the tree view prefix for an object with a message that span on more than one line.
+func getRootMultiLineObjectPrefix(obj ctrlclient.Object, objectTree *tree.ObjectTree, prefix string) string {
+	// If it is the last object we can return early with an empty string.
+	orderedObjects := getOrderedTreeObjects(objectTree)
+	if orderedObjects[len(orderedObjects)-1].GetUID() == obj.GetUID() {
+		return ""
+	}
+
+	// Get the multiline prefix for the objects condition.
+	multilinePrefix := getMultilineConditionPrefix(prefix)
+
+	// If it is the top-level root object, set the multiline prefix to a pipe.
+	if prefix == "" {
+		multilinePrefix = pipe
+	}
+
+	// If the object is not the root object and has children, we need to add a pipe to the multiline prefix.
+	childrenObj := objectTree.GetObjectsByParent(obj.GetUID())
+	if obj.GetUID() != objectTree.GetRoot().GetUID() && len(childrenObj) > 0 {
+		// If the multiline prefix is empty, indent the multiline prefix first
+		if multilinePrefix == "" {
+			multilinePrefix = indent
+		}
+		multilinePrefix += pipe
+	}
+
+	// If the multiline prefix is empty, we have to ensure that even multiline conditions on this root object are indented.
+	if strings.TrimSpace(multilinePrefix) == "" {
+		filler := strings.Repeat(" ", 10)
+		childrenPipe := indent
+		if objectTree.IsObjectWithChild(obj.GetUID()) {
+			childrenPipe = pipe
+		}
+		// We just want to indent the multiline message for the Ready condition.
+		// All ├─ should be replaced by |, so all the existing hierarchic dependencies are carried on
+		multilinePrefix = strings.ReplaceAll(prefix+childrenPipe+filler, firstElemPrefix, pipe)
+		// All └─ should be replaced by " " because we are under the last element of the tree (nothing to carry on)
+		multilinePrefix = strings.ReplaceAll(multilinePrefix, lastElemPrefix, strings.Repeat(" ", len([]rune(lastElemPrefix))))
+	}
+
+	return multilinePrefix
+}
+
+// getOrderedTreeObjects returns the objects in the tree in the order they should be printed.
+func getOrderedTreeObjects(objectTree *tree.ObjectTree) []ctrlclient.Object {
+	rootObjs := objectTree.GetObjectsByParent(objectTree.GetRoot().GetUID())
+	rootObjs = orderChildrenObjects(rootObjs)
+	objs := []ctrlclient.Object{objectTree.GetRoot()}
+	for _, obj := range rootObjs {
+		childrenObjs := objectTree.GetObjectsByParent(obj.GetUID())
+		childrenObjs = orderChildrenObjects(childrenObjs)
+		objs = append(objs, obj)
+		objs = append(objs, childrenObjs...)
+	}
+	return objs
+}
+
 // getRowName returns the object name in the tree view according to following rules:
 // - group objects are represented as #of objects kind, e.g. 3 Machines...
 // - other virtual objects are represented using the object name, e.g. Workers, or meta name if provided.

cmd/clusterctl/cmd/describe_cluster_test.go

@@ -294,6 +294,154 @@ func Test_TreePrefix(t *testing.T) {
 	}
 }
 
+func Test_V1Beta2TreePrefix(t *testing.T) {
+	tests := []struct {
+		name         string
+		objectTree   *tree.ObjectTree
+		expectPrefix []string
+	}{
+		{
+			name: "Conditions should get the right prefix with multiline message",
+			objectTree: func() *tree.ObjectTree {
+				root := fakeObject("root",
+					withV1Beta2Condition(metav1.Condition{
+						Type:    "Available",
+						Status:  metav1.ConditionFalse,
+						Reason:  "NotAvailable",
+						Message: "1\n2",
+					}),
+				)
+				obectjTree := tree.NewObjectTree(root, tree.ObjectTreeOptions{
+					V1Beta2: true,
+				})
+
+				o1 := fakeObject("child1",
+					withV1Beta2Condition(falseV1Beta2Condition("Available", "1\n2")),
+				)
+
+				o2 := fakeObject("child2",
+					withV1Beta2Condition(falseV1Beta2Condition("Available", "1\n2")),
+				)
+				obectjTree.Add(root, o1)
+				obectjTree.Add(root, o2)
+				return obectjTree
+			}(),
+			expectPrefix: []string{
+				"Object/root",
+				"│                                                              2",
+				"├─Object/child1",
+				"│                                                              2",
+				"└─Object/child2",
+			},
+		},
+		{
+			name: "Conditions should get the right prefix with multiline message and a child",
+			objectTree: func() *tree.ObjectTree {
+				root := fakeObject("root",
+					withV1Beta2Condition(metav1.Condition{
+						Type:   "Available",
+						Status: metav1.ConditionTrue,
+						Reason: "Available",
+					}),
+				)
+				obectjTree := tree.NewObjectTree(root, tree.ObjectTreeOptions{
+					V1Beta2: true,
+				})
+
+				o1 := fakeObject("child1",
+					withV1Beta2Condition(trueV1Beta2Condition()),
+				)
+
+				o2 := fakeObject("child2",
+					withV1Beta2Condition(falseV1Beta2Condition("Available", "1\n2")),
+				)
+				o2_1 := fakeObject("child2.1")
+				obectjTree.Add(root, o1)
+				obectjTree.Add(root, o2)
+				obectjTree.Add(o2, o2_1)
+				return obectjTree
+			}(),
+			expectPrefix: []string{
+				"Object/root",
+				"├─Object/child1",
+				"└─Object/child2",
+				"  │                                                                2",
+				"  └─Object/child2.1",
+			},
+		},
+		{
+			name: "Conditions should get the right prefix with childs",
+			objectTree: func() *tree.ObjectTree {
+				root := fakeObject("root",
+					withV1Beta2Condition(metav1.Condition{
+						Type:   "Available",
+						Status: metav1.ConditionTrue,
+						Reason: "Available",
+					}),
+				)
+				obectjTree := tree.NewObjectTree(root, tree.ObjectTreeOptions{
+					V1Beta2: true,
+				})
+
+				o1 := fakeObject("child1",
+					withV1Beta2Condition(trueV1Beta2Condition()),
+				)
+
+				o2 := fakeObject("child2",
+					withV1Beta2Condition(falseV1Beta2Condition("Available", "1\n2")),
+				)
+				o2_1 := fakeObject("child2.1",
+					withV1Beta2Condition(falseV1Beta2Condition("Available", "1\n2")),
+				)
+				o3 := fakeObject("child3",
+					withV1Beta2Condition(falseV1Beta2Condition("Available", "1\n2")),
+				)
+				o3_1 := fakeObject("child3.1",
+					withV1Beta2Condition(falseV1Beta2Condition("Available", "1\n2")),
+				)
+				obectjTree.Add(root, o1)
+				obectjTree.Add(root, o2)
+				obectjTree.Add(o2, o2_1)
+				obectjTree.Add(root, o3)
+				obectjTree.Add(o3, o3_1)
+				return obectjTree
+			}(),
+			expectPrefix: []string{
+				"Object/root",
+				"├─Object/child1",
+				"├─Object/child2",
+				"│ │                                                                2",
+				"│ └─Object/child2.1",
+				"│                                                                  2",
+				"└─Object/child3",
+				"  │                                                                2",
+				"  └─Object/child3.1",
+				"                                                                   2",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			var output bytes.Buffer
+
+			// Creates the output table
+			tbl := tablewriter.NewWriter(&output)
+
+			formatTableTreeV1Beta2(tbl)
+
+			// Add row for the root object, the cluster, and recursively for all the nodes representing the cluster status.
+			addObjectRowV1Beta2("", tbl, tt.objectTree, tt.objectTree.GetRoot())
+			tbl.Render()
+
+			// remove empty lines from the output. We need this because v1beta2 adds lines at the beginning and end.
+			outputString := strings.TrimSpace(output.String())
+
+			g.Expect(outputString).Should(MatchTable(tt.expectPrefix))
+		})
+	}
+}
+
 type objectOption func(object ctrlclient.Object)
 
 func fakeObject(name string, options ...objectOption) ctrlclient.Object {
@@ -331,6 +479,32 @@ func withCondition(c *clusterv1.Condition) func(ctrlclient.Object) {
 	}
 }
 
+func withV1Beta2Condition(c metav1.Condition) func(ctrlclient.Object) {
+	return func(m ctrlclient.Object) {
+		cluster := m.(*clusterv1.Cluster)
+		conds := cluster.GetV1Beta2Conditions()
+		conds = append(conds, c)
+		cluster.SetV1Beta2Conditions(conds)
+	}
+}
+
+func trueV1Beta2Condition() metav1.Condition {
+	return metav1.Condition{
+		Type:   "Available",
+		Status: metav1.ConditionTrue,
+		Reason: "Available",
+	}
+}
+
+func falseV1Beta2Condition(t, m string) metav1.Condition {
+	return metav1.Condition{
+		Type:    t,
+		Status:  metav1.ConditionFalse,
+		Reason:  "Not" + t,
+		Message: m,
+	}
+}
+
 func withDeletionTimestamp(object ctrlclient.Object) {
 	now := metav1.Now()
 	object.SetDeletionTimestamp(&now)