Skip to content

Commit

Permalink
Fix multiline Ready condition in clusterctl describe
Browse files Browse the repository at this point in the history 
The multiline feature in clusterctl has a bug in the multiline conditions.
If the Ready condition message has multipile lines the multiline prefix is not calculated properly.
To fix this we can use the same logic as for the other conditions and verify if the object is the last object in the tree.

Signed-off-by: Tobias Giese <[email protected]>
  • Loading branch information
@tobiasgiese
tobiasgiese committed Feb 2, 2025
1 parent 010af7f commit ac341c5
Show file tree
Hide file tree
Showing 2 changed files with 266 additions and 8 deletions.
78 changes: 70 additions & 8 deletions cmd/clusterctl/cmd/describe_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ func addObjectRowV1Beta2(prefix string, tbl *tablewriter.Table, objectTree *tree
rowDescriptor.message = ""
}

multilinePrefix := getMultilinePrefix(obj, objectTree, prefix)

// Add the row representing the object that includes
// - The row name with the tree view prefix.
// - Replica counters
Expand All @@ -306,7 +308,7 @@ func addObjectRowV1Beta2(prefix string, tbl *tablewriter.Table, objectTree *tree

for _, m := range msg[1:] {
tbl.Append([]string{
getMultilinePrefix(gray.Sprint(prefix)),
gray.Sprint(multilinePrefix),
"",
"",
"",
Expand All @@ -324,7 +326,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.
Expand All @@ -336,10 +345,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.
Expand Down Expand Up @@ -452,7 +458,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)),
"",
"",
"",
Expand Down Expand Up @@ -510,8 +516,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
Expand All @@ -524,6 +530,62 @@ func getMultilinePrefix(currentPrefix string) string {
return "?"
}

// getRootMultiLinePrefix return the tree view prefix for a multiline condition on the object.
func getMultilinePrefix(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 ""
}

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{}
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.
Expand Down
196 changes: 196 additions & 0 deletions cmd/clusterctl/cmd/describe_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,195 @@ 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(metav1.Condition{Type: "Available",
Status: metav1.ConditionFalse,
Reason: "NotAvailable",
Message: "1\n2",
}),
)

o2 := fakeObject("child2",
withV1Beta2Condition(metav1.Condition{Type: "Available",
Status: metav1.ConditionFalse,
Reason: "NotAvailable",
Message: "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(metav1.Condition{
Type: "Available",
Status: metav1.ConditionTrue,
Reason: "Available",
}),
)

o2 := fakeObject("child2",
withV1Beta2Condition(metav1.Condition{
Type: "Available",
Status: metav1.ConditionFalse,
Reason: "NotAvailable",
Message: "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 multiline message and a child with multiline message",
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(metav1.Condition{
Type: "Available",
Status: metav1.ConditionTrue,
Reason: "Available",
}),
)

o2 := fakeObject("child2",
withV1Beta2Condition(metav1.Condition{
Type: "Available",
Status: metav1.ConditionFalse,
Reason: "NotAvailable",
Message: "1\n2",
}),
)
o2_1 := fakeObject("child2.1",
withV1Beta2Condition(metav1.Condition{
Type: "Available",
Status: metav1.ConditionFalse,
Reason: "NotAvailable",
Message: "1\n2",
}),
)
o3 := fakeObject("child3",
withV1Beta2Condition(metav1.Condition{
Type: "Available",
Status: metav1.ConditionFalse,
Reason: "NotAvailable",
Message: "1\n2",
}),
)
o3_1 := fakeObject("child3.1",
withV1Beta2Condition(metav1.Condition{
Type: "Available",
Status: metav1.ConditionFalse,
Reason: "NotAvailable",
Message: "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 {
Expand Down Expand Up @@ -331,6 +520,13 @@ 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)
cluster.SetV1Beta2Conditions([]metav1.Condition{c})
}
}

func withDeletionTimestamp(object ctrlclient.Object) {
now := metav1.Now()
object.SetDeletionTimestamp(&now)
Expand Down

0 comments on commit ac341c5

Please sign in to comment.