Merge pull request #24703 from jhorwit2/jah/#24696

Fixes #24696 - Fixes Size showing 0 in formatted ps output

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
+}
+
+// Size sets the size option when called by a template execution.
+func (p *preProcessor) Size() bool {
+	p.opts.Size = true
+	return true
+}
+
+func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, error) {
+
+	options := &types.ContainerListOptions{
+		All:    opts.all,
+		Limit:  opts.last,
+		Size:   opts.size,
+		Filter: filters.NewArgs(),
+	}
 
 	if opts.nLatest && opts.last == -1 {
-		opts.last = 1
+		options.Limit = 1
 	}
 
-	containerFilterArgs := filters.NewArgs()
 	for _, f := range opts.filter {
 		var err error
-		containerFilterArgs, err = filters.ParseFlag(f, containerFilterArgs)
+		options.Filter, err = filters.ParseFlag(f, options.Filter)
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
 
-	options := types.ContainerListOptions{
-		All:    opts.all,
-		Limit:  opts.last,
-		Size:   opts.size,
-		Filter: containerFilterArgs,
+	// 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
 	}
 
-	pre := &preProcessor{opts: &options}
-	tmpl, err := templates.Parse(opts.format)
+	// 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,
 	}
 

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))
+			}
+		}
+	}
+}