Browse code

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>

Yong Tang authored on 2017/02/06 01:55:30
Showing 3 changed files
... ...
@@ -213,7 +213,7 @@ func runStats(dockerCli *command.DockerCli, opts *statsOptions) error {
213 213
 			ccstats = append(ccstats, c.GetStatistics())
214 214
 		}
215 215
 		cStats.mu.Unlock()
216
-		if err = formatter.ContainerStatsWrite(statsCtx, ccstats); err != nil {
216
+		if err = formatter.ContainerStatsWrite(statsCtx, ccstats, daemonOSType); err != nil {
217 217
 			break
218 218
 		}
219 219
 		if len(cStats.cs) == 0 && !showAll {
... ...
@@ -37,7 +37,6 @@ type StatsEntry struct {
37 37
 	BlockWrite       float64
38 38
 	PidsCurrent      uint64 // Not used on Windows
39 39
 	IsInvalid        bool
40
-	OSType           string
41 40
 }
42 41
 
43 42
 // ContainerStats represents an entity to store containers statistics synchronously
... ...
@@ -88,7 +87,6 @@ func (cs *ContainerStats) SetStatistics(s StatsEntry) {
88 88
 	cs.mutex.Lock()
89 89
 	defer cs.mutex.Unlock()
90 90
 	s.Container = cs.Container
91
-	s.OSType = cs.OSType
92 91
 	cs.StatsEntry = s
93 92
 }
94 93
 
... ...
@@ -113,16 +111,17 @@ func NewStatsFormat(source, osType string) Format {
113 113
 // NewContainerStats returns a new ContainerStats entity and sets in it the given name
114 114
 func NewContainerStats(container, osType string) *ContainerStats {
115 115
 	return &ContainerStats{
116
-		StatsEntry: StatsEntry{Container: container, OSType: osType},
116
+		StatsEntry: StatsEntry{Container: container},
117 117
 	}
118 118
 }
119 119
 
120 120
 // ContainerStatsWrite renders the context for a list of containers statistics
121
-func ContainerStatsWrite(ctx Context, containerStats []StatsEntry) error {
121
+func ContainerStatsWrite(ctx Context, containerStats []StatsEntry, osType string) error {
122 122
 	render := func(format func(subContext subContext) error) error {
123 123
 		for _, cstats := range containerStats {
124 124
 			containerStatsCtx := &containerStatsContext{
125
-				s: cstats,
125
+				s:  cstats,
126
+				os: osType,
126 127
 			}
127 128
 			if err := format(containerStatsCtx); err != nil {
128 129
 				return err
... ...
@@ -130,12 +129,13 @@ func ContainerStatsWrite(ctx Context, containerStats []StatsEntry) error {
130 130
 		}
131 131
 		return nil
132 132
 	}
133
-	return ctx.Write(&containerStatsContext{}, render)
133
+	return ctx.Write(&containerStatsContext{os: osType}, render)
134 134
 }
135 135
 
136 136
 type containerStatsContext struct {
137 137
 	HeaderContext
138
-	s StatsEntry
138
+	s  StatsEntry
139
+	os string
139 140
 }
140 141
 
141 142
 func (c *containerStatsContext) MarshalJSON() ([]byte, error) {
... ...
@@ -168,14 +168,14 @@ func (c *containerStatsContext) CPUPerc() string {
168 168
 
169 169
 func (c *containerStatsContext) MemUsage() string {
170 170
 	header := memUseHeader
171
-	if c.s.OSType == winOSType {
171
+	if c.os == winOSType {
172 172
 		header = winMemUseHeader
173 173
 	}
174 174
 	c.AddHeader(header)
175 175
 	if c.s.IsInvalid {
176 176
 		return fmt.Sprintf("-- / --")
177 177
 	}
178
-	if c.s.OSType == winOSType {
178
+	if c.os == winOSType {
179 179
 		return fmt.Sprintf("%s", units.BytesSize(c.s.Memory))
180 180
 	}
181 181
 	return fmt.Sprintf("%s / %s", units.BytesSize(c.s.Memory), units.BytesSize(c.s.MemoryLimit))
... ...
@@ -184,7 +184,7 @@ func (c *containerStatsContext) MemUsage() string {
184 184
 func (c *containerStatsContext) MemPerc() string {
185 185
 	header := memPercHeader
186 186
 	c.AddHeader(header)
187
-	if c.s.IsInvalid || c.s.OSType == winOSType {
187
+	if c.s.IsInvalid || c.os == winOSType {
188 188
 		return fmt.Sprintf("--")
189 189
 	}
190 190
 	return fmt.Sprintf("%.2f%%", c.s.MemoryPercentage)
... ...
@@ -208,7 +208,7 @@ func (c *containerStatsContext) BlockIO() string {
208 208
 
209 209
 func (c *containerStatsContext) PIDs() string {
210 210
 	c.AddHeader(pidsHeader)
211
-	if c.s.IsInvalid || c.s.OSType == winOSType {
211
+	if c.s.IsInvalid || c.os == winOSType {
212 212
 		return fmt.Sprintf("--")
213 213
 	}
214 214
 	return fmt.Sprintf("%d", c.s.PidsCurrent)
... ...
@@ -14,30 +14,31 @@ func TestContainerStatsContext(t *testing.T) {
14 14
 	var ctx containerStatsContext
15 15
 	tt := []struct {
16 16
 		stats     StatsEntry
17
+		osType    string
17 18
 		expValue  string
18 19
 		expHeader string
19 20
 		call      func() string
20 21
 	}{
21
-		{StatsEntry{Container: containerID}, containerID, containerHeader, ctx.Container},
22
-		{StatsEntry{CPUPercentage: 5.5}, "5.50%", cpuPercHeader, ctx.CPUPerc},
23
-		{StatsEntry{CPUPercentage: 5.5, IsInvalid: true}, "--", cpuPercHeader, ctx.CPUPerc},
24
-		{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3}, "0.31 B / 12.3 B", netIOHeader, ctx.NetIO},
25
-		{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true}, "--", netIOHeader, ctx.NetIO},
26
-		{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3}, "0.1 B / 2.3 B", blockIOHeader, ctx.BlockIO},
27
-		{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3, IsInvalid: true}, "--", blockIOHeader, ctx.BlockIO},
28
-		{StatsEntry{MemoryPercentage: 10.2}, "10.20%", memPercHeader, ctx.MemPerc},
29
-		{StatsEntry{MemoryPercentage: 10.2, IsInvalid: true}, "--", memPercHeader, ctx.MemPerc},
30
-		{StatsEntry{MemoryPercentage: 10.2, OSType: "windows"}, "--", memPercHeader, ctx.MemPerc},
31
-		{StatsEntry{Memory: 24, MemoryLimit: 30}, "24 B / 30 B", memUseHeader, ctx.MemUsage},
32
-		{StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true}, "-- / --", memUseHeader, ctx.MemUsage},
33
-		{StatsEntry{Memory: 24, MemoryLimit: 30, OSType: "windows"}, "24 B", winMemUseHeader, ctx.MemUsage},
34
-		{StatsEntry{PidsCurrent: 10}, "10", pidsHeader, ctx.PIDs},
35
-		{StatsEntry{PidsCurrent: 10, IsInvalid: true}, "--", pidsHeader, ctx.PIDs},
36
-		{StatsEntry{PidsCurrent: 10, OSType: "windows"}, "--", pidsHeader, ctx.PIDs},
22
+		{StatsEntry{Container: containerID}, "", containerID, containerHeader, ctx.Container},
23
+		{StatsEntry{CPUPercentage: 5.5}, "", "5.50%", cpuPercHeader, ctx.CPUPerc},
24
+		{StatsEntry{CPUPercentage: 5.5, IsInvalid: true}, "", "--", cpuPercHeader, ctx.CPUPerc},
25
+		{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3}, "", "0.31 B / 12.3 B", netIOHeader, ctx.NetIO},
26
+		{StatsEntry{NetworkRx: 0.31, NetworkTx: 12.3, IsInvalid: true}, "", "--", netIOHeader, ctx.NetIO},
27
+		{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3}, "", "0.1 B / 2.3 B", blockIOHeader, ctx.BlockIO},
28
+		{StatsEntry{BlockRead: 0.1, BlockWrite: 2.3, IsInvalid: true}, "", "--", blockIOHeader, ctx.BlockIO},
29
+		{StatsEntry{MemoryPercentage: 10.2}, "", "10.20%", memPercHeader, ctx.MemPerc},
30
+		{StatsEntry{MemoryPercentage: 10.2, IsInvalid: true}, "", "--", memPercHeader, ctx.MemPerc},
31
+		{StatsEntry{MemoryPercentage: 10.2}, "windows", "--", memPercHeader, ctx.MemPerc},
32
+		{StatsEntry{Memory: 24, MemoryLimit: 30}, "", "24 B / 30 B", memUseHeader, ctx.MemUsage},
33
+		{StatsEntry{Memory: 24, MemoryLimit: 30, IsInvalid: true}, "", "-- / --", memUseHeader, ctx.MemUsage},
34
+		{StatsEntry{Memory: 24, MemoryLimit: 30}, "windows", "24 B", winMemUseHeader, ctx.MemUsage},
35
+		{StatsEntry{PidsCurrent: 10}, "", "10", pidsHeader, ctx.PIDs},
36
+		{StatsEntry{PidsCurrent: 10, IsInvalid: true}, "", "--", pidsHeader, ctx.PIDs},
37
+		{StatsEntry{PidsCurrent: 10}, "windows", "--", pidsHeader, ctx.PIDs},
37 38
 	}
38 39
 
39 40
 	for _, te := range tt {
40
-		ctx = containerStatsContext{s: te.stats}
41
+		ctx = containerStatsContext{s: te.stats, os: te.osType}
41 42
 		if v := te.call(); v != te.expValue {
42 43
 			t.Fatalf("Expected %q, got %q", te.expValue, v)
43 44
 		}
... ...
@@ -93,7 +94,6 @@ container2  --
93 93
 				BlockWrite:       20,
94 94
 				PidsCurrent:      2,
95 95
 				IsInvalid:        false,
96
-				OSType:           "linux",
97 96
 			},
98 97
 			{
99 98
 				Container:        "container2",
... ...
@@ -107,12 +107,11 @@ container2  --
107 107
 				BlockWrite:       30,
108 108
 				PidsCurrent:      3,
109 109
 				IsInvalid:        true,
110
-				OSType:           "linux",
111 110
 			},
112 111
 		}
113 112
 		var out bytes.Buffer
114 113
 		te.context.Output = &out
115
-		err := ContainerStatsWrite(te.context, stats)
114
+		err := ContainerStatsWrite(te.context, stats, "linux")
116 115
 		if err != nil {
117 116
 			assert.Error(t, err, te.expected)
118 117
 		} else {
... ...
@@ -161,7 +160,6 @@ container2  --  --
161 161
 				BlockWrite:       20,
162 162
 				PidsCurrent:      2,
163 163
 				IsInvalid:        false,
164
-				OSType:           "windows",
165 164
 			},
166 165
 			{
167 166
 				Container:        "container2",
... ...
@@ -175,12 +173,11 @@ container2  --  --
175 175
 				BlockWrite:       30,
176 176
 				PidsCurrent:      3,
177 177
 				IsInvalid:        true,
178
-				OSType:           "windows",
179 178
 			},
180 179
 		}
181 180
 		var out bytes.Buffer
182 181
 		te.context.Output = &out
183
-		err := ContainerStatsWrite(te.context, stats)
182
+		err := ContainerStatsWrite(te.context, stats, "windows")
184 183
 		if err != nil {
185 184
 			assert.Error(t, err, te.expected)
186 185
 		} else {
... ...
@@ -220,9 +217,47 @@ func TestContainerStatsContextWriteWithNoStats(t *testing.T) {
220 220
 	}
221 221
 
222 222
 	for _, context := range contexts {
223
-		ContainerStatsWrite(context.context, []StatsEntry{})
223
+		ContainerStatsWrite(context.context, []StatsEntry{}, "linux")
224 224
 		assert.Equal(t, context.expected, out.String())
225 225
 		// Clean buffer
226 226
 		out.Reset()
227 227
 	}
228 228
 }
229
+
230
+func TestContainerStatsContextWriteWithNoStatsWindows(t *testing.T) {
231
+	var out bytes.Buffer
232
+
233
+	contexts := []struct {
234
+		context  Context
235
+		expected string
236
+	}{
237
+		{
238
+			Context{
239
+				Format: "{{.Container}}",
240
+				Output: &out,
241
+			},
242
+			"",
243
+		},
244
+		{
245
+			Context{
246
+				Format: "table {{.Container}}\t{{.MemUsage}}",
247
+				Output: &out,
248
+			},
249
+			"CONTAINER           PRIV WORKING SET\n",
250
+		},
251
+		{
252
+			Context{
253
+				Format: "table {{.Container}}\t{{.CPUPerc}}\t{{.MemUsage}}",
254
+				Output: &out,
255
+			},
256
+			"CONTAINER           CPU %               PRIV WORKING SET\n",
257
+		},
258
+	}
259
+
260
+	for _, context := range contexts {
261
+		ContainerStatsWrite(context.context, []StatsEntry{}, "windows")
262
+		assert.Equal(t, out.String(), context.expected)
263
+		// Clean buffer
264
+		out.Reset()
265
+	}
266
+}