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
This commit is contained in:
Sebastiaan van Stijn
2025-09-24 13:50:25 +02:00
committed by GitHub
3 changed files with 171 additions and 29 deletions
+1
View File
@@ -167,6 +167,7 @@ func (c *statsContext) Container() string {
}
func (c *statsContext) Name() string {
// TODO(thaJeztah): make this explicitly trim the "/" prefix, not just any char.
if len(c.s.Name) > 1 {
return c.s.Name[1:]
}
+160 -24
View File
@@ -5,45 +5,181 @@ import (
"testing"
"github.com/docker/cli/cli/command/formatter"
"github.com/docker/cli/internal/test"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
func TestContainerStatsContext(t *testing.T) {
containerID := test.RandomID()
const actorID = "c74518277ddc15a6afeaaeb06ee5f7433dcb27188224777c1efa7df1e8766d65"
var ctx statsContext
tt := []struct {
tests := []struct {
name string
stats StatsEntry
osType string
expValue string
expHeader string
call func() string
}{
{StatsEntry{Container: containerID}, "", containerID, containerHeader, ctx.Container},
{StatsEntry{CPUPercentage: 5.5}, "", "5.50%", cpuPercHeader, ctx.CPUPerc},
{StatsEntry{CPUPercentage: 5.5, IsInvalid: true}, "", "--", cpuPercHeader, ctx.CPUPerc},
{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3}, "", "0.31B / 12.3B", netIOHeader, ctx.NetIO},
{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true}, "", "--", netIOHeader, ctx.NetIO},
{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3}, "", "0.1B / 2.3B", blockIOHeader, ctx.BlockIO},
{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3, IsInvalid: true}, "", "--", blockIOHeader, ctx.BlockIO},
{StatsEntry{MemoryPercentage: 10.2}, "", "10.20%", memPercHeader, ctx.MemPerc},
{StatsEntry{MemoryPercentage: 10.2, IsInvalid: true}, "", "--", memPercHeader, ctx.MemPerc},
{StatsEntry{MemoryPercentage: 10.2}, "windows", "--", memPercHeader, ctx.MemPerc},
{StatsEntry{Memory: 24, MemoryLimit: 30}, "", "24B / 30B", memUseHeader, ctx.MemUsage},
{StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true}, "", "-- / --", memUseHeader, ctx.MemUsage},
{StatsEntry{Memory: 24, MemoryLimit: 30}, "windows", "24B", winMemUseHeader, ctx.MemUsage},
{StatsEntry{PidsCurrent: 10}, "", "10", pidsHeader, ctx.PIDs},
{StatsEntry{PidsCurrent: 10, IsInvalid: true}, "", "--", pidsHeader, ctx.PIDs},
{StatsEntry{PidsCurrent: 10}, "windows", "--", pidsHeader, ctx.PIDs},
{
name: "Container id",
stats: StatsEntry{ID: actorID, Container: actorID},
expValue: actorID,
expHeader: containerHeader,
call: ctx.Container,
},
{
name: "Container name",
stats: StatsEntry{ID: actorID, Container: "a-long-container-name"},
expValue: "a-long-container-name",
expHeader: containerHeader,
call: ctx.Container,
},
{
name: "ID",
stats: StatsEntry{ID: actorID},
expValue: actorID,
expHeader: formatter.ContainerIDHeader,
call: ctx.ID,
},
{
name: "Name",
stats: StatsEntry{Name: "/container-name"},
expValue: "container-name",
expHeader: formatter.ContainerIDHeader,
call: ctx.Name,
},
{
name: "Name empty",
stats: StatsEntry{Name: ""},
expValue: "--",
expHeader: formatter.ContainerIDHeader,
call: ctx.Name,
},
{
name: "Name prefix only",
stats: StatsEntry{Name: "/"},
expValue: "--",
expHeader: formatter.ContainerIDHeader,
call: ctx.Name,
},
{
name: "CPUPerc",
stats: StatsEntry{CPUPercentage: 5.5},
expValue: "5.50%",
expHeader: cpuPercHeader,
call: ctx.CPUPerc,
},
{
name: "CPUPerc invalid",
stats: StatsEntry{CPUPercentage: 5.5, IsInvalid: true},
expValue: "--",
expHeader: cpuPercHeader,
call: ctx.CPUPerc,
},
{
name: "NetIO",
stats: StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3},
expValue: "0.31B / 12.3B",
expHeader: netIOHeader,
call: ctx.NetIO,
},
{
name: "NetIO invalid",
stats: StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true},
expValue: "--",
expHeader: netIOHeader,
call: ctx.NetIO,
},
{
name: "BlockIO",
stats: StatsEntry{BlockRead: 0.1, BlockWrite: 2.3},
expValue: "0.1B / 2.3B",
expHeader: blockIOHeader,
call: ctx.BlockIO,
},
{
name: "BlockIO invalid",
stats: StatsEntry{BlockRead: 0.1, BlockWrite: 2.3, IsInvalid: true},
expValue: "--",
expHeader: blockIOHeader,
call: ctx.BlockIO,
},
{
name: "MemPerc",
stats: StatsEntry{MemoryPercentage: 10.2},
expValue: "10.20%",
expHeader: memPercHeader,
call: ctx.MemPerc,
},
{
name: "MemPerc invalid",
stats: StatsEntry{MemoryPercentage: 10.2, IsInvalid: true},
expValue: "--",
expHeader: memPercHeader,
call: ctx.MemPerc,
},
{
name: "MemPerc windows",
stats: StatsEntry{MemoryPercentage: 10.2},
osType: "windows",
expValue: "--",
expHeader: memPercHeader,
call: ctx.MemPerc,
},
{
name: "MemUsage",
stats: StatsEntry{Memory: 24, MemoryLimit: 30},
expValue: "24B / 30B",
expHeader: memUseHeader,
call: ctx.MemUsage,
},
{
name: "MemUsage invalid",
stats: StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true},
expValue: "-- / --",
expHeader: memUseHeader,
call: ctx.MemUsage,
},
{
name: "MemUsage windows",
stats: StatsEntry{Memory: 24, MemoryLimit: 30},
osType: "windows",
expValue: "24B",
expHeader: winMemUseHeader,
call: ctx.MemUsage,
},
{
name: "PIDs",
stats: StatsEntry{PidsCurrent: 10},
expValue: "10",
expHeader: pidsHeader,
call: ctx.PIDs,
},
{
name: "PIDs invalid",
stats: StatsEntry{PidsCurrent: 10, IsInvalid: true},
expValue: "--",
expHeader: pidsHeader,
call: ctx.PIDs,
},
{
name: "PIDs windows",
stats: StatsEntry{PidsCurrent: 10},
osType: "windows",
expValue: "--",
expHeader: pidsHeader,
call: ctx.PIDs,
},
}
for _, te := range tt {
ctx = statsContext{s: te.stats, os: te.osType}
if v := te.call(); v != te.expValue {
t.Fatalf("Expected %q, got %q", te.expValue, v)
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
ctx = statsContext{s: tc.stats, os: tc.osType}
if v := tc.call(); v != tc.expValue {
t.Fatalf("Expected %q, got %q", tc.expValue, v)
}
})
}
}
+10 -5
View File
@@ -139,7 +139,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
eh := newEventHandler()
if options.All {
eh.setHandler(events.ActionCreate, func(e events.Message) {
s := NewStats(e.Actor.ID[:12])
s := NewStats(e.Actor.ID)
if cStats.add(s) {
waitFirst.Add(1)
go collect(ctx, s, apiClient, !options.NoStream, waitFirst)
@@ -148,7 +148,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
}
eh.setHandler(events.ActionStart, func(e events.Message) {
s := NewStats(e.Actor.ID[:12])
s := NewStats(e.Actor.ID)
if cStats.add(s) {
waitFirst.Add(1)
go collect(ctx, s, apiClient, !options.NoStream, waitFirst)
@@ -157,7 +157,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
if !options.All {
eh.setHandler(events.ActionDie, func(e events.Message) {
cStats.remove(e.Actor.ID[:12])
cStats.remove(e.Actor.ID)
})
}
@@ -210,7 +210,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
return err
}
for _, ctr := range cs {
s := NewStats(ctr.ID[:12])
s := NewStats(ctr.ID)
if cStats.add(s) {
waitFirst.Add(1)
go collect(ctx, s, apiClient, !options.NoStream, waitFirst)
@@ -363,7 +363,12 @@ func (eh *eventHandler) watch(c <-chan events.Message) {
if !exists {
continue
}
logrus.Debugf("event handler: received event: %v", e)
if e.Actor.ID == "" {
logrus.WithField("event", e).Errorf("event handler: received %s event with empty ID", e.Action)
continue
}
logrus.WithField("event", e).Debugf("event handler: received %s event for: %s", e.Action, e.Actor.ID)
go h(e)
}
}