Merge pull request #2232 from thaJeztah/fixup_docker_ps_formatting

Fix some issues with docker ps --format

cli/command/container/list.go

@@ -10,6 +10,7 @@ import (
 	"github.com/docker/cli/opts"
 	"github.com/docker/cli/templates"
 	"github.com/docker/docker/api/types"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -58,27 +59,6 @@ func newListCommand(dockerCli command.Cli) *cobra.Command {
 	return &cmd
 }
 
-// listOptionsProcessor is used to set any container list options which may only
-// be embedded in the format template.
-// This is passed directly into tmpl.Execute in order to allow the preprocessor
-// to set any list options that were not provided by flags (e.g. `.Size`).
-// It is using a `map[string]bool` so that unknown fields passed into the
-// template format do not cause errors. These errors will get picked up when
-// running through the actual template processor.
-type listOptionsProcessor map[string]bool
-
-// Size sets the size of the map when called by a template execution.
-func (o listOptionsProcessor) Size() bool {
-	o["size"] = true
-	return true
-}
-
-// Label is needed here as it allows the correct pre-processing
-// because Label() is a method with arguments
-func (o listOptionsProcessor) Label(name string) string {
-	return ""
-}
-
 func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, error) {
 	options := &types.ContainerListOptions{
 		All:     opts.all,
@@ -91,20 +71,32 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
 		options.Limit = 1
 	}
 
-	tmpl, err := templates.Parse(opts.format)
+	options.Size = opts.size
+	if !options.Size && len(opts.format) > 0 {
+		// The --size option isn't set, but .Size may be used in the template.
+		// Parse and execute the given template to detect if the .Size field is
+		// used. If it is, then automatically enable the --size option. See #24696
+		//
+		// Only requesting container size information when needed is an optimization,
+		// because calculating the size is a costly operation.
+		tmpl, err := templates.NewParse("", opts.format)
+
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to parse template")
+		}
+
+		optionsProcessor := formatter.NewContainerContext()
 
-	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.
+		if err := tmpl.Execute(ioutil.Discard, optionsProcessor); err != nil {
+			return nil, errors.Wrap(err, "failed to execute template")
+		}
 
-	optionsProcessor := listOptionsProcessor{}
-	// 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, optionsProcessor); err != nil {
-		return nil, err
+		if _, ok := optionsProcessor.FieldsUsed["Size"]; ok {
+			options.Size = true
+		}
 	}
-	// At the moment all we need is to capture .Size for preprocessor
-	options.Size = opts.size || optionsProcessor["size"]
 
 	return options, nil
 }

cli/command/container/list_test.go

@@ -78,7 +78,7 @@ func TestContainerListBuildContainerListOptions(t *testing.T) {
 				last:   5,
 				filter: filters,
 				// With .Size, size should be true
-				format: "{{.Size}} {{.CreatedAt}} {{.Networks}}",
+				format: "{{.Size}} {{.CreatedAt}} {{upper .Networks}}",
 			},
 			expectedAll:   true,
 			expectedSize:  true,

cli/command/formatter/container.go

@@ -62,24 +62,32 @@ ports: {{- pad .Ports 1 0}}
 func ContainerWrite(ctx Context, containers []types.Container) error {
 	render := func(format func(subContext SubContext) error) error {
 		for _, container := range containers {
-			err := format(&containerContext{trunc: ctx.Trunc, c: container})
+			err := format(&ContainerContext{trunc: ctx.Trunc, c: container})
 			if err != nil {
 				return err
 			}
 		}
 		return nil
 	}
-	return ctx.Write(newContainerContext(), render)
+	return ctx.Write(NewContainerContext(), render)
 }
 
-type containerContext struct {
+// ContainerContext is a struct used for rendering a list of containers in a Go template.
+type ContainerContext struct {
 	HeaderContext
 	trunc bool
 	c     types.Container
+
+	// FieldsUsed is used in the pre-processing step to detect which fields are
+	// used in the template. It's currently only used to detect use of the .Size
+	// field which (if used) automatically sets the '--size' option when making
+	// the API call.
+	FieldsUsed map[string]interface{}
 }
 
-func newContainerContext() *containerContext {
-	containerCtx := containerContext{}
+// NewContainerContext creates a new context for rendering containers
+func NewContainerContext() *ContainerContext {
+	containerCtx := ContainerContext{}
 	containerCtx.Header = SubHeaderContext{
 		"ID":           ContainerIDHeader,
 		"Names":        namesHeader,
@@ -99,18 +107,24 @@ func newContainerContext() *containerContext {
 	return &containerCtx
 }
 
-func (c *containerContext) MarshalJSON() ([]byte, error) {
+// MarshalJSON makes ContainerContext implement json.Marshaler
+func (c *ContainerContext) MarshalJSON() ([]byte, error) {
 	return MarshalJSON(c)
 }
 
-func (c *containerContext) ID() string {
+// ID returns the container's ID as a string. Depending on the `--no-trunc`
+// option being set, the full or truncated ID is returned.
+func (c *ContainerContext) ID() string {
 	if c.trunc {
 		return stringid.TruncateID(c.c.ID)
 	}
 	return c.c.ID
 }
 
-func (c *containerContext) Names() string {
+// Names returns a comma-separated string of the container's names, with their
+// slash (/) prefix stripped. Additional names for the container (related to the
+// legacy `--link` feature) are omitted.
+func (c *ContainerContext) Names() string {
 	names := stripNamePrefix(c.c.Names)
 	if c.trunc {
 		for _, name := range names {
@@ -123,7 +137,9 @@ func (c *containerContext) Names() string {
 	return strings.Join(names, ",")
 }
 
-func (c *containerContext) Image() string {
+// Image returns the container's image reference. If the trunc option is set,
+// the image's registry digest can be included.
+func (c *ContainerContext) Image() string {
 	if c.c.Image == "" {
 		return "<no image>"
 	}
@@ -150,36 +166,56 @@ func (c *containerContext) Image() string {
 	return c.c.Image
 }
 
-func (c *containerContext) Command() string {
+// Command returns's the container's command. If the trunc option is set, the
+// returned command is truncated (ellipsized).
+func (c *ContainerContext) Command() string {
 	command := c.c.Command
 	if c.trunc {
 		command = Ellipsis(command, 20)
 	}
 	return strconv.Quote(command)
 }
 
-func (c *containerContext) CreatedAt() string {
+// CreatedAt returns the "Created" date/time of the container as a unix timestamp.
+func (c *ContainerContext) CreatedAt() string {
 	return time.Unix(c.c.Created, 0).String()
 }
 
-func (c *containerContext) RunningFor() string {
+// RunningFor returns a human-readable representation of the duration for which
+// the container has been running.
+//
+// Note that this duration is calculated on the client, and as such is influenced
+// by clock skew between the client and the daemon.
+func (c *ContainerContext) RunningFor() string {
 	createdAt := time.Unix(c.c.Created, 0)
 	return units.HumanDuration(time.Now().UTC().Sub(createdAt)) + " ago"
 }
 
-func (c *containerContext) Ports() string {
+// Ports returns a comma-separated string representing open ports of the container
+// e.g. "0.0.0.0:80->9090/tcp, 9988/tcp"
+// it's used by command 'docker ps'
+// Both published and exposed ports are included.
+func (c *ContainerContext) Ports() string {
 	return DisplayablePorts(c.c.Ports)
 }
 
-func (c *containerContext) State() string {
+// State returns the container's current state (e.g. "running" or "paused")
+func (c *ContainerContext) State() string {
 	return c.c.State
 }
 
-func (c *containerContext) Status() string {
+// Status returns the container's status in a human readable form (for example,
+// "Up 24 hours" or "Exited (0) 8 days ago")
+func (c *ContainerContext) Status() string {
 	return c.c.Status
 }
 
-func (c *containerContext) Size() string {
+// Size returns the container's size and virtual size (e.g. "2B (virtual 21.5MB)")
+func (c *ContainerContext) Size() string {
+	if c.FieldsUsed == nil {
+		c.FieldsUsed = map[string]interface{}{}
+	}
+	c.FieldsUsed["Size"] = struct{}{}
 	srw := units.HumanSizeWithPrecision(float64(c.c.SizeRw), 3)
 	sv := units.HumanSizeWithPrecision(float64(c.c.SizeRootFs), 3)
 
@@ -190,7 +226,8 @@ func (c *containerContext) Size() string {
 	return sf
 }
 
-func (c *containerContext) Labels() string {
+// Labels returns a comma-separated string of labels present on the container.
+func (c *ContainerContext) Labels() string {
 	if c.c.Labels == nil {
 		return ""
 	}
@@ -202,14 +239,18 @@ func (c *containerContext) Labels() string {
 	return strings.Join(joinLabels, ",")
 }
 
-func (c *containerContext) Label(name string) string {
+// Label returns the value of the label with the given name or an empty string
+// if the given label does not exist.
+func (c *ContainerContext) Label(name string) string {
 	if c.c.Labels == nil {
 		return ""
 	}
 	return c.c.Labels[name]
 }
 
-func (c *containerContext) Mounts() string {
+// Mounts returns a comma-separated string of mount names present on the container.
+// If the trunc option is set, names can be truncated (ellipsized).
+func (c *ContainerContext) Mounts() string {
 	var name string
 	var mounts []string
 	for _, m := range c.c.Mounts {
@@ -226,7 +267,8 @@ func (c *containerContext) Mounts() string {
 	return strings.Join(mounts, ",")
 }
 
-func (c *containerContext) LocalVolumes() string {
+// LocalVolumes returns the number of volumes using the "local" volume driver.
+func (c *ContainerContext) LocalVolumes() string {
 	count := 0
 	for _, m := range c.c.Mounts {
 		if m.Driver == "local" {
@@ -237,7 +279,9 @@ func (c *containerContext) LocalVolumes() string {
 	return fmt.Sprintf("%d", count)
 }
 
-func (c *containerContext) Networks() string {
+// Networks returns a comma-separated string of networks that the container is
+// attached to.
+func (c *ContainerContext) Networks() string {
 	if c.c.NetworkSettings == nil {
 		return ""
 	}

cli/command/formatter/container_test.go

@@ -19,7 +19,7 @@ func TestContainerPsContext(t *testing.T) {
 	containerID := stringid.GenerateRandomID()
 	unix := time.Now().Add(-65 * time.Second).Unix()
 
-	var ctx containerContext
+	var ctx ContainerContext
 	cases := []struct {
 		container types.Container
 		trunc     bool
@@ -87,7 +87,7 @@ func TestContainerPsContext(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		ctx = containerContext{c: c.container, trunc: c.trunc}
+		ctx = ContainerContext{c: c.container, trunc: c.trunc}
 		v := c.call()
 		if strings.Contains(v, ",") {
 			compareMultipleValues(t, v, c.expValue)
@@ -97,7 +97,7 @@ func TestContainerPsContext(t *testing.T) {
 	}
 
 	c1 := types.Container{Labels: map[string]string{"com.docker.swarm.swarm-id": "33", "com.docker.swarm.node_name": "ubuntu"}}
-	ctx = containerContext{c: c1, trunc: true}
+	ctx = ContainerContext{c: c1, trunc: true}
 
 	sid := ctx.Label("com.docker.swarm.swarm-id")
 	node := ctx.Label("com.docker.swarm.node_name")
@@ -110,7 +110,7 @@ func TestContainerPsContext(t *testing.T) {
 	}
 
 	c2 := types.Container{}
-	ctx = containerContext{c: c2, trunc: true}
+	ctx = ContainerContext{c: c2, trunc: true}
 
 	label := ctx.Label("anything.really")
 	if label != "" {
@@ -241,6 +241,10 @@ size: 0B
 			Context{Format: NewContainerFormat(`table {{truncate .ID 5}}\t{{json .Image}} {{.RunningFor}}/{{title .Status}}/{{pad .Ports 2 2}}.{{upper .Names}} {{lower .Status}}`, false, true)},
 			string(golden.Get(t, "container-context-write-special-headers.golden")),
 		},
+		{
+			Context{Format: NewContainerFormat(`table {{split .Image ":"}}`, false, false)},
+			"IMAGE\n[ubuntu]\n[ubuntu]\n",
+		},
 	}
 
 	for _, testcase := range cases {

cli/command/formatter/disk_usage.go

@@ -136,15 +136,15 @@ func (ctx *DiskUsageContext) Write() (err error) {
 
 type diskUsageContext struct {
 	Images     []*imageContext
-	Containers []*containerContext
+	Containers []*ContainerContext
 	Volumes    []*volumeContext
 	BuildCache []*buildCacheContext
 }
 
 func (ctx *DiskUsageContext) verboseWrite() error {
 	duc := &diskUsageContext{
 		Images:     make([]*imageContext, 0, len(ctx.Images)),
-		Containers: make([]*containerContext, 0, len(ctx.Containers)),
+		Containers: make([]*ContainerContext, 0, len(ctx.Containers)),
 		Volumes:    make([]*volumeContext, 0, len(ctx.Volumes)),
 		BuildCache: make([]*buildCacheContext, 0, len(ctx.BuildCache)),
 	}
@@ -178,7 +178,7 @@ func (ctx *DiskUsageContext) verboseWrite() error {
 	for _, c := range ctx.Containers {
 		// Don't display the virtual size
 		c.SizeRootFs = 0
-		duc.Containers = append(duc.Containers, &containerContext{trunc: trunc, c: *c})
+		duc.Containers = append(duc.Containers, &ContainerContext{trunc: trunc, c: *c})
 	}
 
 	// And volumes
@@ -227,7 +227,7 @@ func (ctx *DiskUsageContext) verboseWriteTable(duc *diskUsageContext) error {
 			return err
 		}
 	}
-	ctx.postFormat(tmpl, newContainerContext())
+	ctx.postFormat(tmpl, NewContainerContext())
 
 	tmpl, err = ctx.startSubsection(defaultDiskUsageVolumeTableFormat)
 	if err != nil {

templates/templates.go

@@ -30,11 +30,23 @@ var basicFunctions = template.FuncMap{
 // HeaderFunctions are used to created headers of a table.
 // This is a replacement of basicFunctions for header generation
 // because we want the header to remain intact.
-// Some functions like `split` are irrelevant so not added.
+// Some functions like `pad` are not overridden (to preserve alignment
+// with the columns).
 var HeaderFunctions = template.FuncMap{
 	"json": func(v string) string {
 		return v
 	},
+	"split": func(v string, _ string) string {
+		// we want the table header to show the name of the column, and not
+		// split the table header itself. Using a different signature
+		// here, and return a string instead of []string
+		return v
+	},
+	"join": func(v string, _ string) string {
+		// table headers are always a string, so use a different signature
+		// for the "join" function (string instead of []string)
+		return v
+	},
 	"title": func(v string) string {
 		return v
 	},