Skip to content

Commit 01e9612

Browse files
derailedgandro
authored andcommitted
Bug: Fix module health status output
Add fall safe to ensure module health tree rendering output does not panic when producing the output. Note: This was a reported failure during ci testing where the cilium status command failed with `panic: strings: negative Repeat count` Signed-off-by: Fernand Galiana <[email protected]>
1 parent ba6c661 commit 01e9612

File tree

3 files changed

+93
-17
lines changed

3 files changed

+93
-17
lines changed

pkg/health/client/modules_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ agent
4040
├── a
4141
│ └── b
4242
│ └── c
43-
│ ├── fred [OK] yo (20s, x1)
44-
│ │ └── blee [OK] doh (20s, x1)
45-
│ └── dork [DEGRADED] bozo -- BOOM! (20s, x1)
46-
└── m1 [OK] status nominal (2s, x0)`,
43+
│ ├── fred [OK] yo (20s, x1)
44+
│ │ └── blee [OK] doh (20s, x1)
45+
│ └── dork [DEGRADED] bozo -- BOOM! (20s, x1)
46+
└── m1 [OK] status nominal (2s, x0)`,
4747
v: true,
4848
},
4949
}

pkg/health/client/tree.go

+31-11
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,11 @@ func (n *node) find(val string) *node {
7878
}
7979

8080
func (n *node) asBytes() []byte {
81-
w := new(bytes.Buffer)
82-
level := 0
83-
var levelsEnded []int
81+
var (
82+
w = new(bytes.Buffer)
83+
levelsEnded []int
84+
max = computeMaxLevel(0, n)
85+
)
8486
if n.parent == nil {
8587
w.WriteString(n.val)
8688
if n.meta != "" {
@@ -91,12 +93,12 @@ func (n *node) asBytes() []byte {
9193
edge := mid
9294
if len(n.nodes) == 0 {
9395
edge = end
94-
levelsEnded = append(levelsEnded, level)
96+
levelsEnded = append(levelsEnded, 0)
9597
}
96-
dumpVals(w, 0, levelsEnded, edge, n)
98+
dumpVals(w, 0, max, levelsEnded, edge, n)
9799
}
98100
if len(n.nodes) > 0 {
99-
dumpNodes(w, level, levelsEnded, n.nodes)
101+
dumpNodes(w, 0, max, levelsEnded, n.nodes)
100102
}
101103

102104
return w.Bytes()
@@ -115,21 +117,36 @@ func (n *node) lastNode() *node {
115117
return n.nodes[c-1]
116118
}
117119

118-
func dumpNodes(w io.Writer, level int, levelsEnded []int, nodes []*node) {
120+
func computeMaxLevel(level int, n *node) int {
121+
if n == nil || len(n.nodes) == 0 {
122+
return level
123+
}
124+
var max int
125+
for _, n := range n.nodes {
126+
m := computeMaxLevel(level+1, n)
127+
if m > max {
128+
max = m
129+
}
130+
}
131+
132+
return max
133+
}
134+
135+
func dumpNodes(w io.Writer, level, maxLevel int, levelsEnded []int, nodes []*node) {
119136
for i, node := range nodes {
120137
edge := mid
121138
if i == len(nodes)-1 {
122139
levelsEnded = append(levelsEnded, level)
123140
edge = end
124141
}
125-
dumpVals(w, level, levelsEnded, edge, node)
142+
dumpVals(w, level, maxLevel, levelsEnded, edge, node)
126143
if len(node.nodes) > 0 {
127-
dumpNodes(w, level+1, levelsEnded, node.nodes)
144+
dumpNodes(w, level+1, maxLevel, levelsEnded, node.nodes)
128145
}
129146
}
130147
}
131148

132-
func dumpVals(w io.Writer, level int, levelsEnded []int, edge decoration, node *node) {
149+
func dumpVals(w io.Writer, level, maxLevel int, levelsEnded []int, edge decoration, node *node) {
133150
for i := 0; i < level; i++ {
134151
if isEnded(levelsEnded, i) {
135152
fmt.Fprint(w, strings.Repeat(" ", indentSize+1))
@@ -140,7 +157,10 @@ func dumpVals(w io.Writer, level int, levelsEnded []int, edge decoration, node *
140157

141158
val := dumpVal(level, node)
142159
if node.meta != "" {
143-
c := 4 - level
160+
c := maxLevel - level
161+
if c < 0 {
162+
c = 0
163+
}
144164
fmt.Fprintf(w, "%s %-"+strconv.Itoa(leafMaxWidth+c*2)+"s%s%s\n", edge, val, strings.Repeat(" ", c), node.meta)
145165
return
146166
}

pkg/health/client/tree_test.go

+58-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,49 @@ import (
99
"github.com/stretchr/testify/assert"
1010
)
1111

12+
func TestComputeMaxLevel(t *testing.T) {
13+
uu := map[string]struct {
14+
n *node
15+
max int
16+
}{
17+
"empty": {},
18+
"single": {
19+
n: &node{val: "n1", meta: "m1"},
20+
},
21+
"flat": {
22+
n: &node{val: "n1", meta: "m1", nodes: []*node{
23+
{val: "n2", meta: "m2"},
24+
{val: "n3", meta: "m3"},
25+
}},
26+
max: 1,
27+
},
28+
"multi": {
29+
n: &node{val: "n1", meta: "m1", nodes: []*node{
30+
{val: "n1_1", meta: "m1_1", nodes: []*node{
31+
{val: "n1_1_1", meta: "m1_1_1", nodes: []*node{
32+
{val: "n1_1_1_1", meta: "m1_1_1_1"},
33+
{val: "n1_1_1_2", meta: "m1_1_1_2"},
34+
}},
35+
}},
36+
{val: "n2", meta: "m2", nodes: []*node{
37+
{val: "n2_1", meta: "m2_1", nodes: []*node{
38+
{val: "n2_1_1", meta: "m2_1_1"},
39+
}},
40+
}},
41+
{val: "n3", meta: "m3"},
42+
}},
43+
max: 3,
44+
},
45+
}
46+
47+
for k := range uu {
48+
u := uu[k]
49+
t.Run(k, func(t *testing.T) {
50+
assert.Equal(t, u.max, computeMaxLevel(0, u.n))
51+
})
52+
}
53+
}
54+
1255
func TestTreeAddNode(t *testing.T) {
1356
r := newRoot("fred")
1457
r.addNode("a")
@@ -17,7 +60,13 @@ func TestTreeAddNode(t *testing.T) {
1760
r.addNode("c")
1861
r.addNodeWithMeta("d", "blee")
1962

20-
assert.Equal(t, "fred\n├── a\n├── b\n│ └── b1\n├── c\n└── d blee\n", r.String())
63+
assert.Equal(t, `fred
64+
├── a
65+
├── b
66+
│ └── b1
67+
├── c
68+
└── d blee
69+
`, r.String())
2170
}
2271

2372
func TestTreeAddBranch(t *testing.T) {
@@ -29,7 +78,14 @@ func TestTreeAddBranch(t *testing.T) {
2978
r.addBranch("c")
3079
r.addBranchWithMeta("d", "blee")
3180

32-
assert.Equal(t, "fred\n├── a\n├── b\n│ └── b1\n│ └── b1_1\n├── c\n└── d blee\n", r.String())
81+
assert.Equal(t, `fred
82+
├── a
83+
├── b
84+
│ └── b1
85+
│ └── b1_1
86+
├── c
87+
└── d blee
88+
`, r.String())
3389
}
3490

3591
func TestTreeFind(t *testing.T) {

0 commit comments

Comments
 (0)