mirror of https://github.com/docker/cli.git
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 <yong.tang.github@outlook.com>
This commit is contained in:
Yong Tang
parent
bba31c3f04
commit
af80020ef2
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
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
|
||||
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)
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue