From 0a3a3ba2b62a5720d949e93fd580d3cf797cfa9e Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sun, 5 Feb 2017 08:55:30 -0800 Subject: [PATCH] Fix Windows `docker stats` showing Linux headers This fix is an attempt to fix issue raised in #28005 where `docker stats` on Windows shows Linux headers if there is no containers in stats. The reason for the issue is that, in case there is no container, a header is faked in: https://github.com/docker/docker/blob/v1.13.0/cli/command/formatter/formatter.go#L74-L78 which does not know OS type information (as OS was stored with container stat entries) This fix tries to fix the issue by moving OS type information to stats context (instead of individual container stats entry). Additional unit tests have been added. This fix fixes #28005. Signed-off-by: Yong Tang --- cli/command/container/stats.go | 2 +- cli/command/formatter/stats.go | 22 ++++---- cli/command/formatter/stats_test.go | 83 ++++++++++++++++++++--------- 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/cli/command/container/stats.go b/cli/command/container/stats.go index 593db27b2a..940a039143 100644 --- a/cli/command/container/stats.go +++ b/cli/command/container/stats.go @@ -213,7 +213,7 @@ func runStats(dockerCli *command.DockerCli, opts *statsOptions) error { ccstats = append(ccstats, c.GetStatistics()) } cStats.mu.Unlock() - if err = formatter.ContainerStatsWrite(statsCtx, ccstats); err != nil { + if err = formatter.ContainerStatsWrite(statsCtx, ccstats, daemonOSType); err != nil { break } if len(cStats.cs) == 0 && !showAll { diff --git a/cli/command/formatter/stats.go b/cli/command/formatter/stats.go index a37e9d7923..7302bca010 100644 --- a/cli/command/formatter/stats.go +++ b/cli/command/formatter/stats.go @@ -37,7 +37,6 @@ type StatsEntry struct { BlockWrite float64 PidsCurrent uint64 // Not used on Windows IsInvalid bool - OSType string } // ContainerStats represents an entity to store containers statistics synchronously @@ -88,7 +87,6 @@ func (cs *ContainerStats) SetStatistics(s StatsEntry) { cs.mutex.Lock() defer cs.mutex.Unlock() s.Container = cs.Container - s.OSType = cs.OSType cs.StatsEntry = s } @@ -113,16 +111,17 @@ func NewStatsFormat(source, osType string) Format { // NewContainerStats returns a new ContainerStats entity and sets in it the given name func NewContainerStats(container, osType string) *ContainerStats { return &ContainerStats{ - StatsEntry: StatsEntry{Container: container, OSType: osType}, + StatsEntry: StatsEntry{Container: container}, } } // ContainerStatsWrite renders the context for a list of containers statistics -func ContainerStatsWrite(ctx Context, containerStats []StatsEntry) error { +func ContainerStatsWrite(ctx Context, containerStats []StatsEntry, osType string) error { render := func(format func(subContext subContext) error) error { for _, cstats := range containerStats { containerStatsCtx := &containerStatsContext{ - s: cstats, + s: cstats, + os: osType, } if err := format(containerStatsCtx); err != nil { return err @@ -130,12 +129,13 @@ func ContainerStatsWrite(ctx Context, containerStats []StatsEntry) error { } return nil } - return ctx.Write(&containerStatsContext{}, render) + return ctx.Write(&containerStatsContext{os: osType}, render) } type containerStatsContext struct { HeaderContext - s StatsEntry + s StatsEntry + os string } func (c *containerStatsContext) MarshalJSON() ([]byte, error) { @@ -168,14 +168,14 @@ func (c *containerStatsContext) CPUPerc() string { func (c *containerStatsContext) MemUsage() string { header := memUseHeader - if c.s.OSType == winOSType { + if c.os == winOSType { header = winMemUseHeader } c.AddHeader(header) if c.s.IsInvalid { return fmt.Sprintf("-- / --") } - if c.s.OSType == winOSType { + if c.os == winOSType { return fmt.Sprintf("%s", units.BytesSize(c.s.Memory)) } return fmt.Sprintf("%s / %s", units.BytesSize(c.s.Memory), units.BytesSize(c.s.MemoryLimit)) @@ -184,7 +184,7 @@ func (c *containerStatsContext) MemUsage() string { func (c *containerStatsContext) MemPerc() string { header := memPercHeader c.AddHeader(header) - if c.s.IsInvalid || c.s.OSType == winOSType { + if c.s.IsInvalid || c.os == winOSType { return fmt.Sprintf("--") } return fmt.Sprintf("%.2f%%", c.s.MemoryPercentage) @@ -208,7 +208,7 @@ func (c *containerStatsContext) BlockIO() string { func (c *containerStatsContext) PIDs() string { c.AddHeader(pidsHeader) - if c.s.IsInvalid || c.s.OSType == winOSType { + if c.s.IsInvalid || c.os == winOSType { return fmt.Sprintf("--") } return fmt.Sprintf("%d", c.s.PidsCurrent) diff --git a/cli/command/formatter/stats_test.go b/cli/command/formatter/stats_test.go index d5a17cc70e..f9ecda33ec 100644 --- a/cli/command/formatter/stats_test.go +++ b/cli/command/formatter/stats_test.go @@ -14,30 +14,31 @@ func TestContainerStatsContext(t *testing.T) { var ctx containerStatsContext tt := []struct { 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.31 B / 12.3 B", netIOHeader, ctx.NetIO}, - {StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true}, "--", netIOHeader, ctx.NetIO}, - {StatsEntry{BlockRead: 0.1, BlockWrite: 2.3}, "0.1 B / 2.3 B", 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, OSType: "windows"}, "--", memPercHeader, ctx.MemPerc}, - {StatsEntry{Memory: 24, MemoryLimit: 30}, "24 B / 30 B", memUseHeader, ctx.MemUsage}, - {StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true}, "-- / --", memUseHeader, ctx.MemUsage}, - {StatsEntry{Memory: 24, MemoryLimit: 30, OSType: "windows"}, "24 B", winMemUseHeader, ctx.MemUsage}, - {StatsEntry{PidsCurrent: 10}, "10", pidsHeader, ctx.PIDs}, - {StatsEntry{PidsCurrent: 10, IsInvalid: true}, "--", pidsHeader, ctx.PIDs}, - {StatsEntry{PidsCurrent: 10, OSType: "windows"}, "--", pidsHeader, ctx.PIDs}, + {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.31 B / 12.3 B", netIOHeader, ctx.NetIO}, + {StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true}, "", "--", netIOHeader, ctx.NetIO}, + {StatsEntry{BlockRead: 0.1, BlockWrite: 2.3}, "", "0.1 B / 2.3 B", 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}, "", "24 B / 30 B", memUseHeader, ctx.MemUsage}, + {StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true}, "", "-- / --", memUseHeader, ctx.MemUsage}, + {StatsEntry{Memory: 24, MemoryLimit: 30}, "windows", "24 B", 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}, } for _, te := range tt { - ctx = containerStatsContext{s: te.stats} + ctx = containerStatsContext{s: te.stats, os: te.osType} if v := te.call(); v != te.expValue { t.Fatalf("Expected %q, got %q", te.expValue, v) } @@ -93,7 +94,6 @@ container2 -- BlockWrite: 20, PidsCurrent: 2, IsInvalid: false, - OSType: "linux", }, { Container: "container2", @@ -107,12 +107,11 @@ container2 -- BlockWrite: 30, PidsCurrent: 3, IsInvalid: true, - OSType: "linux", }, } var out bytes.Buffer te.context.Output = &out - err := ContainerStatsWrite(te.context, stats) + err := ContainerStatsWrite(te.context, stats, "linux") if err != nil { assert.Error(t, err, te.expected) } else { @@ -161,7 +160,6 @@ container2 -- -- BlockWrite: 20, PidsCurrent: 2, IsInvalid: false, - OSType: "windows", }, { Container: "container2", @@ -175,12 +173,11 @@ container2 -- -- BlockWrite: 30, PidsCurrent: 3, IsInvalid: true, - OSType: "windows", }, } var out bytes.Buffer te.context.Output = &out - err := ContainerStatsWrite(te.context, stats) + err := ContainerStatsWrite(te.context, stats, "windows") if err != nil { assert.Error(t, err, te.expected) } else { @@ -220,9 +217,47 @@ func TestContainerStatsContextWriteWithNoStats(t *testing.T) { } for _, context := range contexts { - ContainerStatsWrite(context.context, []StatsEntry{}) + ContainerStatsWrite(context.context, []StatsEntry{}, "linux") assert.Equal(t, context.expected, out.String()) // Clean buffer out.Reset() } } + +func TestContainerStatsContextWriteWithNoStatsWindows(t *testing.T) { + var out bytes.Buffer + + contexts := []struct { + context Context + expected string + }{ + { + Context{ + Format: "{{.Container}}", + Output: &out, + }, + "", + }, + { + Context{ + Format: "table {{.Container}}\t{{.MemUsage}}", + Output: &out, + }, + "CONTAINER PRIV WORKING SET\n", + }, + { + Context{ + Format: "table {{.Container}}\t{{.CPUPerc}}\t{{.MemUsage}}", + Output: &out, + }, + "CONTAINER CPU % PRIV WORKING SET\n", + }, + } + + for _, context := range contexts { + ContainerStatsWrite(context.context, []StatsEntry{}, "windows") + assert.Equal(t, out.String(), context.expected) + // Clean buffer + out.Reset() + } +}