From 55cdb6dcd0f709301573ddb9f3348f9288572b91 Mon Sep 17 00:00:00 2001 From: Paulo Ribeiro Date: Thu, 14 Apr 2016 10:43:15 +0100 Subject: [PATCH] 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 Signed-off-by: Sebastiaan van Stijn --- api/client/formatter/formatter.go | 22 +++++++--------------- api/client/formatter/formatter_test.go | 21 ++++++++++++++++++++- api/client/ps.go | 20 ++++++++++++++++++++ integration-cli/docker_cli_ps_test.go | 17 +++++++++++++++++ 4 files changed, 64 insertions(+), 16 deletions(-) diff --git a/api/client/formatter/formatter.go b/api/client/formatter/formatter.go index bc3d50c948..a52ec8eb10 100644 --- a/api/client/formatter/formatter.go +++ b/api/client/formatter/formatter.go @@ -114,35 +114,27 @@ type ImageContext struct { func (ctx ContainerContext) Write() { switch ctx.Format { case tableFormatKey: - ctx.Format = defaultContainerTableFormat if ctx.Quiet { ctx.Format = defaultQuietFormat + } else { + ctx.Format = defaultContainerTableFormat + if ctx.Size { + ctx.Format += `\t{{.Size}}` + } } case rawFormatKey: if ctx.Quiet { ctx.Format = `container_id: {{.ID}}` } else { - ctx.Format = `container_id: {{.ID}} -image: {{.Image}} -command: {{.Command}} -created_at: {{.CreatedAt}} -status: {{.Status}} -names: {{.Names}} -labels: {{.Labels}} -ports: {{.Ports}} -` + ctx.Format = `container_id: {{.ID}}\nimage: {{.Image}}\ncommand: {{.Command}}\ncreated_at: {{.CreatedAt}}\nstatus: {{.Status}}\nnames: {{.Names}}\nlabels: {{.Labels}}\nports: {{.Ports}}\n` if ctx.Size { - ctx.Format += `size: {{.Size}} -` + ctx.Format += `size: {{.Size}}\n` } } } ctx.buffer = bytes.NewBufferString("") ctx.preformat() - if ctx.table && ctx.Size { - ctx.finalFormat += "\t{{.Size}}" - } tmpl, err := ctx.parseFormat() if err != nil { diff --git a/api/client/formatter/formatter_test.go b/api/client/formatter/formatter_test.go index 223cab970a..7dd5a68fcd 100644 --- a/api/client/formatter/formatter_test.go +++ b/api/client/formatter/formatter_test.go @@ -63,7 +63,7 @@ containerID2 ubuntu "" 24 hours ago }, Size: true, }, - "IMAGE SIZE\nubuntu 0 B\nubuntu 0 B\n", + "IMAGE\nubuntu\nubuntu\n", }, { ContainerContext{ @@ -230,6 +230,25 @@ func TestContainerContextWriteWithNoContainers(t *testing.T) { }, Size: true, }, + "IMAGE\n", + }, + { + ContainerContext{ + Context: Context{ + Format: "table {{.Image}}\t{{.Size}}", + Output: out, + }, + }, + "IMAGE SIZE\n", + }, + { + ContainerContext{ + Context: Context{ + Format: "table {{.Image}}\t{{.Size}}", + Output: out, + }, + Size: true, + }, "IMAGE SIZE\n", }, } diff --git a/api/client/ps.go b/api/client/ps.go index f44d80635f..0cab9b519f 100644 --- a/api/client/ps.go +++ b/api/client/ps.go @@ -2,15 +2,27 @@ package client import ( "golang.org/x/net/context" + "io/ioutil" "github.com/docker/docker/api/client/formatter" Cli "github.com/docker/docker/cli" "github.com/docker/docker/opts" flag "github.com/docker/docker/pkg/mflag" + "github.com/docker/docker/utils/templates" "github.com/docker/engine-api/types" "github.com/docker/engine-api/types/filters" ) +type preProcessor struct { + opts *types.ContainerListOptions +} + +// Size sets the size option when called by a template execution. +func (p *preProcessor) Size() bool { + p.opts.Size = true + return true +} + // CmdPs outputs a list of Docker containers. // // Usage: docker ps [OPTIONS] @@ -54,6 +66,14 @@ func (cli *DockerCli) CmdPs(args ...string) error { Filter: psFilterArgs, } + pre := &preProcessor{opts: &options} + tmpl, err := templates.Parse(*format) + if err != nil { + return err + } + + _ = tmpl.Execute(ioutil.Discard, pre) + containers, err := cli.client.ContainerList(context.Background(), options) if err != nil { return err diff --git a/integration-cli/docker_cli_ps_test.go b/integration-cli/docker_cli_ps_test.go index 3fe8ecf96e..6e0ce2b392 100644 --- a/integration-cli/docker_cli_ps_test.go +++ b/integration-cli/docker_cli_ps_test.go @@ -787,3 +787,20 @@ func (s *DockerSuite) TestPsShowMounts(c *check.C) { out, _ = dockerCmd(c, "ps", "--format", "{{.Names}} {{.Mounts}}", "--filter", "volume="+prefix+slash+"this-path-was-never-mounted") c.Assert(strings.TrimSpace(string(out)), checker.HasLen, 0) } + +func (s *DockerSuite) TestPsFormatSize(c *check.C) { + testRequires(c, DaemonIsLinux) + runSleepingContainer(c) + + out, _ := dockerCmd(c, "ps", "--format", "table {{.Size}}") + lines := strings.Split(out, "\n") + c.Assert(lines[1], checker.Not(checker.Equals), "0 B", check.Commentf("Should not display a size of 0 B")) + + out, _ = dockerCmd(c, "ps", "--size", "--format", "table {{.Size}}") + lines = strings.Split(out, "\n") + c.Assert(lines[0], checker.Equals, "SIZE", check.Commentf("Should only have one size column")) + + out, _ = dockerCmd(c, "ps", "--size", "--format", "raw") + lines = strings.Split(out, "\n") + c.Assert(lines[8], checker.HasPrefix, "size:", check.Commentf("Size should be appended on a newline")) +}