Don't automatically request size if `--size` was explicitly set to `false`

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
This commit is contained in:
Laura Brehm 2023-03-02 17:05:28 +01:00
parent 4a500f690f
commit 9733334487
No known key found for this signature in database
GPG Key ID: 526E3FC49260D47A
2 changed files with 74 additions and 25 deletions

View File

@ -19,6 +19,7 @@ import (
type psOptions struct { type psOptions struct {
quiet bool quiet bool
size bool size bool
sizeChanged bool
all bool all bool
noTrunc bool noTrunc bool
nLatest bool nLatest bool
@ -36,6 +37,7 @@ func NewPsCommand(dockerCli command.Cli) *cobra.Command {
Short: "List containers", Short: "List containers",
Args: cli.NoArgs, Args: cli.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
options.sizeChanged = cmd.Flags().Changed("size")
return runPs(dockerCli, &options) return runPs(dockerCli, &options)
}, },
Annotations: map[string]string{ Annotations: map[string]string{
@ -78,13 +80,8 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
options.Limit = 1 options.Limit = 1
} }
if !opts.quiet && !options.Size && len(opts.format) > 0 { // always validate template when `--format` is used, for consistency
// The --size option isn't set, but .Size may be used in the template. if len(opts.format) > 0 {
// 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) tmpl, err := templates.NewParse("", opts.format)
if err != nil { if err != nil {
return nil, errors.Wrap(err, "failed to parse template") return nil, errors.Wrap(err, "failed to parse template")
@ -98,10 +95,21 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
return nil, errors.Wrap(err, "failed to execute template") return nil, errors.Wrap(err, "failed to execute template")
} }
// if `size` was not explicitly set to false (with `--size=false`)
// and `--quiet` is not set, request size if the template requires it
if !opts.quiet && !options.Size && !opts.sizeChanged {
// 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.
if _, ok := optionsProcessor.FieldsUsed["Size"]; ok { if _, ok := optionsProcessor.FieldsUsed["Size"]; ok {
options.Size = true options.Size = true
} }
} }
}
return options, nil return options, nil
} }

View File

@ -231,15 +231,56 @@ func TestContainerListFormatTemplateWithArg(t *testing.T) {
} }
func TestContainerListFormatSizeSetsOption(t *testing.T) { func TestContainerListFormatSizeSetsOption(t *testing.T) {
tests := []struct {
doc, format, sizeFlag string
sizeExpected bool
}{
{
doc: "detect with all fields",
format: `{{json .}}`,
sizeExpected: true,
},
{
doc: "detect with explicit field",
format: `{{.Size}}`,
sizeExpected: true,
},
{
doc: "detect no size",
format: `{{.Names}}`,
sizeExpected: false,
},
{
doc: "override enable",
format: `{{.Names}}`,
sizeFlag: "true",
sizeExpected: true,
},
{
doc: "override disable",
format: `{{.Size}}`,
sizeFlag: "false",
sizeExpected: false,
},
}
for _, tc := range tests {
tc := tc
t.Run(tc.doc, func(t *testing.T) {
cli := test.NewFakeCli(&fakeClient{ cli := test.NewFakeCli(&fakeClient{
containerListFunc: func(options types.ContainerListOptions) ([]types.Container, error) { containerListFunc: func(options types.ContainerListOptions) ([]types.Container, error) {
assert.Check(t, options.Size) assert.Check(t, is.Equal(options.Size, tc.sizeExpected))
return []types.Container{}, nil return []types.Container{}, nil
}, },
}) })
cmd := newListCommand(cli) cmd := newListCommand(cli)
cmd.Flags().Set("format", `{{.Size}}`) cmd.Flags().Set("format", tc.format)
if tc.sizeFlag != "" {
cmd.Flags().Set("size", tc.sizeFlag)
}
assert.NilError(t, cmd.Execute()) assert.NilError(t, cmd.Execute())
})
}
} }
func TestContainerListWithConfigFormat(t *testing.T) { func TestContainerListWithConfigFormat(t *testing.T) {