Skip to content

Commit 25421ac

Browse files
thaJeztahthaJeztah
authored
Merge pull request #6471 from thaJeztah/28.x_backport_fix_stats_bounds
[28.x backport] cli/command/container: prevent panic during stats on empty event Actor.ID
2 parents ede7019 + 1cf78c4 commit 25421ac

File tree

3 files changed

+171
-29
lines changed

3 files changed

+171
-29
lines changed

‎cli/command/container/formatter_stats.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ func (c *statsContext) Container() string {
167167
}
168168

169169
func (c *statsContext) Name() string {
170+
// TODO(thaJeztah): make this explicitly trim the "/" prefix, not just any char.
170171
if len(c.s.Name) > 1 {
171172
return c.s.Name[1:]
172173
}

‎cli/command/container/formatter_stats_test.go‎

Lines changed: 160 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,45 +5,181 @@ import (
55
"testing"
66

77
"github.com/docker/cli/cli/command/formatter"
8-
"github.com/docker/cli/internal/test"
98
"gotest.tools/v3/assert"
109
is "gotest.tools/v3/assert/cmp"
1110
)
1211

1312
func TestContainerStatsContext(t *testing.T) {
14-
containerID := test.RandomID()
13+
const actorID = "c74518277ddc15a6afeaaeb06ee5f7433dcb27188224777c1efa7df1e8766d65"
1514

1615
var ctx statsContext
17-
tt := []struct {
16+
tests := []struct {
17+
name string
1818
stats StatsEntry
1919
osType string
2020
expValue string
2121
expHeader string
2222
call func() string
2323
}{
24-
{StatsEntry{Container: containerID}, "", containerID, containerHeader, ctx.Container},
25-
{StatsEntry{CPUPercentage: 5.5}, "", "5.50%", cpuPercHeader, ctx.CPUPerc},
26-
{StatsEntry{CPUPercentage: 5.5, IsInvalid: true}, "", "--", cpuPercHeader, ctx.CPUPerc},
27-
{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3}, "", "0.31B / 12.3B", netIOHeader, ctx.NetIO},
28-
{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true}, "", "--", netIOHeader, ctx.NetIO},
29-
{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3}, "", "0.1B / 2.3B", blockIOHeader, ctx.BlockIO},
30-
{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3, IsInvalid: true}, "", "--", blockIOHeader, ctx.BlockIO},
31-
{StatsEntry{MemoryPercentage: 10.2}, "", "10.20%", memPercHeader, ctx.MemPerc},
32-
{StatsEntry{MemoryPercentage: 10.2, IsInvalid: true}, "", "--", memPercHeader, ctx.MemPerc},
33-
{StatsEntry{MemoryPercentage: 10.2}, "windows", "--", memPercHeader, ctx.MemPerc},
34-
{StatsEntry{Memory: 24, MemoryLimit: 30}, "", "24B / 30B", memUseHeader, ctx.MemUsage},
35-
{StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true}, "", "-- / --", memUseHeader, ctx.MemUsage},
36-
{StatsEntry{Memory: 24, MemoryLimit: 30}, "windows", "24B", winMemUseHeader, ctx.MemUsage},
37-
{StatsEntry{PidsCurrent: 10}, "", "10", pidsHeader, ctx.PIDs},
38-
{StatsEntry{PidsCurrent: 10, IsInvalid: true}, "", "--", pidsHeader, ctx.PIDs},
39-
{StatsEntry{PidsCurrent: 10}, "windows", "--", pidsHeader, ctx.PIDs},
24+
{
25+
name: "Container id",
26+
stats: StatsEntry{ID: actorID, Container: actorID},
27+
expValue: actorID,
28+
expHeader: containerHeader,
29+
call: ctx.Container,
30+
},
31+
{
32+
name: "Container name",
33+
stats: StatsEntry{ID: actorID, Container: "a-long-container-name"},
34+
expValue: "a-long-container-name",
35+
expHeader: containerHeader,
36+
call: ctx.Container,
37+
},
38+
{
39+
name: "ID",
40+
stats: StatsEntry{ID: actorID},
41+
expValue: actorID,
42+
expHeader: formatter.ContainerIDHeader,
43+
call: ctx.ID,
44+
},
45+
{
46+
name: "Name",
47+
stats: StatsEntry{Name: "/container-name"},
48+
expValue: "container-name",
49+
expHeader: formatter.ContainerIDHeader,
50+
call: ctx.Name,
51+
},
52+
{
53+
name: "Name empty",
54+
stats: StatsEntry{Name: ""},
55+
expValue: "--",
56+
expHeader: formatter.ContainerIDHeader,
57+
call: ctx.Name,
58+
},
59+
{
60+
name: "Name prefix only",
61+
stats: StatsEntry{Name: "/"},
62+
expValue: "--",
63+
expHeader: formatter.ContainerIDHeader,
64+
call: ctx.Name,
65+
},
66+
{
67+
name: "CPUPerc",
68+
stats: StatsEntry{CPUPercentage: 5.5},
69+
expValue: "5.50%",
70+
expHeader: cpuPercHeader,
71+
call: ctx.CPUPerc,
72+
},
73+
{
74+
name: "CPUPerc invalid",
75+
stats: StatsEntry{CPUPercentage: 5.5, IsInvalid: true},
76+
expValue: "--",
77+
expHeader: cpuPercHeader,
78+
call: ctx.CPUPerc,
79+
},
80+
{
81+
name: "NetIO",
82+
stats: StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3},
83+
expValue: "0.31B / 12.3B",
84+
expHeader: netIOHeader,
85+
call: ctx.NetIO,
86+
},
87+
{
88+
name: "NetIO invalid",
89+
stats: StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true},
90+
expValue: "--",
91+
expHeader: netIOHeader,
92+
call: ctx.NetIO,
93+
},
94+
{
95+
name: "BlockIO",
96+
stats: StatsEntry{BlockRead: 0.1, BlockWrite: 2.3},
97+
expValue: "0.1B / 2.3B",
98+
expHeader: blockIOHeader,
99+
call: ctx.BlockIO,
100+
},
101+
{
102+
name: "BlockIO invalid",
103+
stats: StatsEntry{BlockRead: 0.1, BlockWrite: 2.3, IsInvalid: true},
104+
expValue: "--",
105+
expHeader: blockIOHeader,
106+
call: ctx.BlockIO,
107+
},
108+
{
109+
name: "MemPerc",
110+
stats: StatsEntry{MemoryPercentage: 10.2},
111+
expValue: "10.20%",
112+
expHeader: memPercHeader,
113+
call: ctx.MemPerc,
114+
},
115+
{
116+
name: "MemPerc invalid",
117+
stats: StatsEntry{MemoryPercentage: 10.2, IsInvalid: true},
118+
expValue: "--",
119+
expHeader: memPercHeader,
120+
call: ctx.MemPerc,
121+
},
122+
{
123+
name: "MemPerc windows",
124+
stats: StatsEntry{MemoryPercentage: 10.2},
125+
osType: "windows",
126+
expValue: "--",
127+
expHeader: memPercHeader,
128+
call: ctx.MemPerc,
129+
},
130+
{
131+
name: "MemUsage",
132+
stats: StatsEntry{Memory: 24, MemoryLimit: 30},
133+
expValue: "24B / 30B",
134+
expHeader: memUseHeader,
135+
call: ctx.MemUsage,
136+
},
137+
{
138+
name: "MemUsage invalid",
139+
stats: StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true},
140+
expValue: "-- / --",
141+
expHeader: memUseHeader,
142+
call: ctx.MemUsage,
143+
},
144+
{
145+
name: "MemUsage windows",
146+
stats: StatsEntry{Memory: 24, MemoryLimit: 30},
147+
osType: "windows",
148+
expValue: "24B",
149+
expHeader: winMemUseHeader,
150+
call: ctx.MemUsage,
151+
},
152+
{
153+
name: "PIDs",
154+
stats: StatsEntry{PidsCurrent: 10},
155+
expValue: "10",
156+
expHeader: pidsHeader,
157+
call: ctx.PIDs,
158+
},
159+
{
160+
name: "PIDs invalid",
161+
stats: StatsEntry{PidsCurrent: 10, IsInvalid: true},
162+
expValue: "--",
163+
expHeader: pidsHeader,
164+
call: ctx.PIDs,
165+
},
166+
{
167+
name: "PIDs windows",
168+
stats: StatsEntry{PidsCurrent: 10},
169+
osType: "windows",
170+
expValue: "--",
171+
expHeader: pidsHeader,
172+
call: ctx.PIDs,
173+
},
40174
}
41175

42-
for _, te := range tt {
43-
ctx = statsContext{s: te.stats, os: te.osType}
44-
if v := te.call(); v != te.expValue {
45-
t.Fatalf("Expected %q, got %q", te.expValue, v)
46-
}
176+
for _, tc := range tests {
177+
t.Run(tc.name, func(t *testing.T) {
178+
ctx = statsContext{s: tc.stats, os: tc.osType}
179+
if v := tc.call(); v != tc.expValue {
180+
t.Fatalf("Expected %q, got %q", tc.expValue, v)
181+
}
182+
})
47183
}
48184
}
49185

‎cli/command/container/stats.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
139139
eh := newEventHandler()
140140
if options.All {
141141
eh.setHandler(events.ActionCreate, func(e events.Message) {
142-
s := NewStats(e.Actor.ID[:12])
142+
s := NewStats(e.Actor.ID)
143143
if cStats.add(s) {
144144
waitFirst.Add(1)
145145
go collect(ctx, s, apiClient, !options.NoStream, waitFirst)
@@ -148,7 +148,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
148148
}
149149

150150
eh.setHandler(events.ActionStart, func(e events.Message) {
151-
s := NewStats(e.Actor.ID[:12])
151+
s := NewStats(e.Actor.ID)
152152
if cStats.add(s) {
153153
waitFirst.Add(1)
154154
go collect(ctx, s, apiClient, !options.NoStream, waitFirst)
@@ -157,7 +157,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
157157

158158
if !options.All {
159159
eh.setHandler(events.ActionDie, func(e events.Message) {
160-
cStats.remove(e.Actor.ID[:12])
160+
cStats.remove(e.Actor.ID)
161161
})
162162
}
163163

@@ -210,7 +210,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
210210
return err
211211
}
212212
for _, ctr := range cs {
213-
s := NewStats(ctr.ID[:12])
213+
s := NewStats(ctr.ID)
214214
if cStats.add(s) {
215215
waitFirst.Add(1)
216216
go collect(ctx, s, apiClient, !options.NoStream, waitFirst)
@@ -363,7 +363,12 @@ func (eh *eventHandler) watch(c <-chan events.Message) {
363363
if !exists {
364364
continue
365365
}
366-
logrus.Debugf("event handler: received event: %v", e)
366+
if e.Actor.ID == "" {
367+
logrus.WithField("event", e).Errorf("event handler: received %s event with empty ID", e.Action)
368+
continue
369+
}
370+
371+
logrus.WithField("event", e).Debugf("event handler: received %s event for: %s", e.Action, e.Actor.ID)
367372
go h(e)
368373
}
369374
}

0 commit comments

Comments
 (0)