1. docker
  2. Commit 55cdb6dcd0f709301573ddb9f3348f9288572b91
Browse code

Add a check for size field in custom format string

This fix addresses an issue where including the `{{.Size}}` format
field in conjunction with `docker ps --format`, without the `--size`
flag, would not correctly display the size of containers.

This is done by doing a check on the custom format string, and setting
the size flag on the options struct if the field is found. This struct
gets passed to the engine API which then generates the correct query.

An integration test is included which runs `docker ps --format "table
{{.Size}}"` without `--size`, and checks that the returned output is
not `0 B`.

Fixes #21991

As suggested by @cpuguy83, a parser is implemented to process the format
string as a template, and then traverses the template tree to determine
if `.Size` was called.

This was then reworked by making use of template execution with a
pre-processor struct that will set the `--size` option if the template
calls for the field.

The pre-processor now also sets a boolean in the context passed to the
writer. There is an integration test for this that calls `docker ps
--size --format "{{.Size}}"` and then checks that `size: {{.Size}}` is
not appended, as it would with previous behavior.

Finally, a change was made to the formatter to not automatically
add a `{{.Size}}` if a custom format is provided.

Signed-off-by: Paulo Ribeiro <paigr.io@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Paulo Ribeiro authored on 2016/04/14 18:43:15
Showing 4 changed files
api/client/formatter/formatter.go
History View file @ 55cdb6d
... ... 
@@ -114,35 +114,27 @@ type ImageContext struct {
114 114 
 func (ctx ContainerContext) Write() {
115 115 
 	switch ctx.Format {
116 116 
 	case tableFormatKey:
117 
-		ctx.Format = defaultContainerTableFormat
118 117 
 		if ctx.Quiet {
119 118 
 			ctx.Format = defaultQuietFormat
119 
+		} else {
120 
+			ctx.Format = defaultContainerTableFormat
121 
+			if ctx.Size {
122 
+				ctx.Format += `\t{{.Size}}`
123 
+			}
120 124 
 		}
121 125 
 	case rawFormatKey:
122 126 
 		if ctx.Quiet {
123 127 
 			ctx.Format = `container_id: {{.ID}}`
124 128 
 		} else {
125 
-			ctx.Format = `container_id: {{.ID}}
126 
-image: {{.Image}}
127 
-command: {{.Command}}
128 
-created_at: {{.CreatedAt}}
129 
-status: {{.Status}}
130 
-names: {{.Names}}
131 
-labels: {{.Labels}}
132 
-ports: {{.Ports}}
133 
-`
129 
+			ctx.Format = `container_id: {{.ID}}\nimage: {{.Image}}\ncommand: {{.Command}}\ncreated_at: {{.CreatedAt}}\nstatus: {{.Status}}\nnames: {{.Names}}\nlabels: {{.Labels}}\nports: {{.Ports}}\n`
134 130 
 			if ctx.Size {
135 
-				ctx.Format += `size: {{.Size}}
136 
-`
131 
+				ctx.Format += `size: {{.Size}}\n`
137 132 
 			}
138 133 
 		}
139 134 
 	}
140 135
141 136 
 	ctx.buffer = bytes.NewBufferString("")
142 137 
 	ctx.preformat()
143 
-	if ctx.table && ctx.Size {
144 
-		ctx.finalFormat += "\t{{.Size}}"
145 
-	}
146 138
147 139 
 	tmpl, err := ctx.parseFormat()
148 140 
 	if err != nil {
api/client/formatter/formatter_test.go
History View file @ 55cdb6d
... ... 
@@ -63,7 +63,7 @@ containerID2        ubuntu              ""                  24 hours ago
63 63 
 				},
64 64 
 				Size: true,
65 65 
 			},
66 
-			"IMAGE               SIZE\nubuntu              0 B\nubuntu              0 B\n",
66 
+			"IMAGE\nubuntu\nubuntu\n",
67 67 
 		},
68 68 
 		{
69 69 
 			ContainerContext{
... ... 
@@ -230,6 +230,25 @@ func TestContainerContextWriteWithNoContainers(t *testing.T) {
230 230 
 				},
231 231 
 				Size: true,
232 232 
 			},
233 
+			"IMAGE\n",
234 
+		},
235 
+		{
236 
+			ContainerContext{
237 
+				Context: Context{
238 
+					Format: "table {{.Image}}\t{{.Size}}",
239 
+					Output: out,
240 
+				},
241 
+			},
242 
+			"IMAGE               SIZE\n",
243 
+		},
244 
+		{
245 
+			ContainerContext{
246 
+				Context: Context{
247 
+					Format: "table {{.Image}}\t{{.Size}}",
248 
+					Output: out,
249 
+				},
250 
+				Size: true,
251 
+			},
233 252 
 			"IMAGE               SIZE\n",
234 253 
 		},
235 254 
 	}
api/client/ps.go
History View file @ 55cdb6d
... ... 
@@ -2,15 +2,27 @@ package client
2 2
3 3 
 import (
4 4 
 	"golang.org/x/net/context"
5 
+	"io/ioutil"
5 6
6 7 
 	"github.com/docker/docker/api/client/formatter"
7 8 
 	Cli "github.com/docker/docker/cli"
8 9 
 	"github.com/docker/docker/opts"
9 10 
 	flag "github.com/docker/docker/pkg/mflag"
11 
+	"github.com/docker/docker/utils/templates"
10 12 
 	"github.com/docker/engine-api/types"
11 13 
 	"github.com/docker/engine-api/types/filters"
12 14 
 )
13 15
16 
+type preProcessor struct {
17 
+	opts *types.ContainerListOptions
18 
+}
19 
+
20 
+// Size sets the size option when called by a template execution.
21 
+func (p *preProcessor) Size() bool {
22 
+	p.opts.Size = true
23 
+	return true
24 
+}
25 
+
14 26 
 // CmdPs outputs a list of Docker containers.
15 27 
 //
16 28 
 // Usage: docker ps [OPTIONS]
... ... 
@@ -54,6 +66,14 @@ func (cli *DockerCli) CmdPs(args ...string) error {
54 54 
 		Filter: psFilterArgs,
55 55 
 	}
56 56
57 
+	pre := &preProcessor{opts: &options}
58 
+	tmpl, err := templates.Parse(*format)
59 
+	if err != nil {
60 
+		return err
61 
+	}
62 
+
63 
+	_ = tmpl.Execute(ioutil.Discard, pre)
64 
+
57 65 
 	containers, err := cli.client.ContainerList(context.Background(), options)
58 66 
 	if err != nil {
59 67 
 		return err
integration-cli/docker_cli_ps_test.go
History View file @ 55cdb6d
... ... 
@@ -787,3 +787,20 @@ func (s *DockerSuite) TestPsShowMounts(c *check.C) {
787 787 
 	out, _ = dockerCmd(c, "ps", "--format", "{{.Names}} {{.Mounts}}", "--filter", "volume="+prefix+slash+"this-path-was-never-mounted")
788 788 
 	c.Assert(strings.TrimSpace(string(out)), checker.HasLen, 0)
789 789 
 }
790 
+
791 
+func (s *DockerSuite) TestPsFormatSize(c *check.C) {
792 
+	testRequires(c, DaemonIsLinux)
793 
+	runSleepingContainer(c)
794 
+
795 
+	out, _ := dockerCmd(c, "ps", "--format", "table {{.Size}}")
796 
+	lines := strings.Split(out, "\n")
797 
+	c.Assert(lines[1], checker.Not(checker.Equals), "0 B", check.Commentf("Should not display a size of 0 B"))
798 
+
799 
+	out, _ = dockerCmd(c, "ps", "--size", "--format", "table {{.Size}}")
800 
+	lines = strings.Split(out, "\n")
801 
+	c.Assert(lines[0], checker.Equals, "SIZE", check.Commentf("Should only have one size column"))
802 
+
803 
+	out, _ = dockerCmd(c, "ps", "--size", "--format", "raw")
804 
+	lines = strings.Split(out, "\n")
805 
+	c.Assert(lines[8], checker.HasPrefix, "size:", check.Commentf("Size should be appended on a newline"))
806 
+}