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> Upstream-commit: 0a3a3ba2b62a5720d949e93fd580d3cf797cfa9e Component: engine
2017-02-05 08:55:30 -08:00
parent ba62bc02bb
commit 190522d7df
3 changed files with 71 additions and 36 deletions
@@ -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,
+				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)
@@ -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()
+	}
+}