From 456e0396596f6184ebd374336f0d4ad3046371ba Mon Sep 17 00:00:00 2001 From: Josh Horwitz Date: Fri, 15 Jul 2016 18:37:05 -0400 Subject: [PATCH] Fixes #24696 - fixes size showing 0 in format output Signed-off-by: Josh Horwitz --- api/client/container/ps.go | 79 ++++++++++++++++++++------------- api/client/container/ps_test.go | 74 ++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 31 deletions(-) create mode 100644 api/client/container/ps_test.go diff --git a/api/client/container/ps.go b/api/client/container/ps.go index cc4add938b..08c2ab6c29 100644 --- a/api/client/container/ps.go +++ b/api/client/container/ps.go @@ -9,9 +9,10 @@ import ( "github.com/docker/engine-api/types" "github.com/docker/engine-api/types/filters" + "io/ioutil" + "github.com/docker/docker/utils/templates" "github.com/spf13/cobra" - "io/ioutil" ) type psOptions struct { @@ -25,16 +26,6 @@ type psOptions struct { filter []string } -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 -} - // NewPsCommand creates a new cobra.Command for `docker ps` func NewPsCommand(dockerCli *client.DockerCli) *cobra.Command { var opts psOptions @@ -62,39 +53,65 @@ func NewPsCommand(dockerCli *client.DockerCli) *cobra.Command { return cmd } -func runPs(dockerCli *client.DockerCli, opts *psOptions) error { - ctx := context.Background() +type preProcessor struct { + types.Container + opts *types.ContainerListOptions +} - if opts.nLatest && opts.last == -1 { - opts.last = 1 - } +// Size sets the size option when called by a template execution. +func (p *preProcessor) Size() bool { + p.opts.Size = true + return true +} - containerFilterArgs := filters.NewArgs() - for _, f := range opts.filter { - var err error - containerFilterArgs, err = filters.ParseFlag(f, containerFilterArgs) - if err != nil { - return err - } - } +func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, error) { - options := types.ContainerListOptions{ + options := &types.ContainerListOptions{ All: opts.all, Limit: opts.last, Size: opts.size, - Filter: containerFilterArgs, + Filter: filters.NewArgs(), } - pre := &preProcessor{opts: &options} + if opts.nLatest && opts.last == -1 { + options.Limit = 1 + } + + for _, f := range opts.filter { + var err error + options.Filter, err = filters.ParseFlag(f, options.Filter) + if err != nil { + return nil, err + } + } + + // Currently only used with Size, so we can determine if the user + // put {{.Size}} in their format. + pre := &preProcessor{opts: options} tmpl, err := templates.Parse(opts.format) + if err != nil { + return nil, err + } + + // This shouldn't error out but swallowing the error makes it harder + // to track down if preProcessor issues come up. Ref #24696 + if err := tmpl.Execute(ioutil.Discard, pre); err != nil { + return nil, err + } + + return options, nil +} + +func runPs(dockerCli *client.DockerCli, opts *psOptions) error { + ctx := context.Background() + + listOptions, err := buildContainerListOptions(opts) if err != nil { return err } - _ = tmpl.Execute(ioutil.Discard, pre) - - containers, err := dockerCli.Client().ContainerList(ctx, options) + containers, err := dockerCli.Client().ContainerList(ctx, *listOptions) if err != nil { return err } @@ -115,7 +132,7 @@ func runPs(dockerCli *client.DockerCli, opts *psOptions) error { Quiet: opts.quiet, Trunc: !opts.noTrunc, }, - Size: opts.size, + Size: listOptions.Size, Containers: containers, } diff --git a/api/client/container/ps_test.go b/api/client/container/ps_test.go new file mode 100644 index 0000000000..2af183cce1 --- /dev/null +++ b/api/client/container/ps_test.go @@ -0,0 +1,74 @@ +package container + +import "testing" + +func TestBuildContainerListOptions(t *testing.T) { + + contexts := []struct { + psOpts *psOptions + expectedAll bool + expectedSize bool + expectedLimit int + expectedFilters map[string]string + }{ + { + psOpts: &psOptions{ + all: true, + size: true, + last: 5, + filter: []string{"foo=bar", "baz=foo"}, + }, + expectedAll: true, + expectedSize: true, + expectedLimit: 5, + expectedFilters: map[string]string{ + "foo": "bar", + "baz": "foo", + }, + }, + { + psOpts: &psOptions{ + all: true, + size: true, + last: -1, + nLatest: true, + }, + expectedAll: true, + expectedSize: true, + expectedLimit: 1, + expectedFilters: make(map[string]string), + }, + } + + for _, c := range contexts { + options, err := buildContainerListOptions(c.psOpts) + if err != nil { + t.Fatal(err) + } + + if c.expectedAll != options.All { + t.Fatalf("Expected All to be %t but got %t", c.expectedAll, options.All) + } + + if c.expectedSize != options.Size { + t.Fatalf("Expected Size to be %t but got %t", c.expectedSize, options.Size) + } + + if c.expectedLimit != options.Limit { + t.Fatalf("Expected Limit to be %d but got %d", c.expectedLimit, options.Limit) + } + + f := options.Filter + + if f.Len() != len(c.expectedFilters) { + t.Fatalf("Expected %d filters but got %d", len(c.expectedFilters), f.Len()) + } + + for k, v := range c.expectedFilters { + f := options.Filter + if !f.ExactMatch(k, v) { + t.Fatalf("Expected filter with key %s to be %s but got %s", k, v, f.Get(k)) + } + } + } +}