From 4a0218bb116f245efa913960a09593ea5631c492 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 17 Nov 2018 16:00:37 +0100 Subject: [PATCH 1/2] Fix system prune warning missing filters from config-file The warning, printed before runing docker system prune was missing any filter that was set in the configuration file. In addition, the warning prefixes the filters with `label=`, which is no longer accurate, now that the prune command also supports "until" as a filter. Before this change, only the filters set on the command-line were shown, and any filter set in the configuration file was missing; ``` mkdir -p ./test-config echo '{"pruneFilters": ["label!=never=remove-me", "label=remove=me"]}' > test-config/config.json docker --config=./test-config system prune --filter until=24h --filter label=hello-world --filter label!=foo=bar --filter label=bar=baz WARNING! This will remove: - all stopped containers - all networks not used by at least one container - all dangling images - all dangling build cache - Elements to be pruned will be filtered with: - label={"label":{"bar=baz":true,"hello-world":true},"label!":{"foo=bar":true},"until":{"24h":true}} Are you sure you want to continue? [y/N] ``` With this patch applied, both options from the commandline and options set in the configuration file are shown; ``` mkdir -p ./test-config echo '{"pruneFilters": ["label!=never=remove-me", "label=remove=me"]}' > test-config/config.json docker --config=./test-config system prune --filter until=24h --filter label=hello-world --filter label!=foo=bar --filter label=bar=baz WARNING! This will remove: - all stopped containers - all networks not used by at least one container - all dangling images - all dangling build cache - Elements to be pruned will be filtered with: - filter={"label":{"bar=baz":true,"hello-world":true,"remove=me":true},"label!":{"foo=bar":true,"never=remove-me":true},"until":{"24h":true}} ``` Signed-off-by: Sebastiaan van Stijn --- cli/command/system/prune.go | 14 ++++++++++---- cli/command/system/prune_test.go | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/cli/command/system/prune.go b/cli/command/system/prune.go index 3f0015545e..8aee689d6d 100644 --- a/cli/command/system/prune.go +++ b/cli/command/system/prune.go @@ -13,6 +13,7 @@ import ( "github.com/docker/cli/cli/command/network" "github.com/docker/cli/cli/command/volume" "github.com/docker/cli/opts" + "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/versions" units "github.com/docker/go-units" "github.com/spf13/cobra" @@ -63,7 +64,7 @@ func runPrune(dockerCli command.Cli, options pruneOptions) error { if options.pruneVolumes && options.filter.Value().Contains("until") { return fmt.Errorf(`ERROR: The "until" filter is not supported with "--volumes"`) } - if !options.force && !command.PromptForConfirmation(dockerCli.In(), dockerCli.Out(), confirmationMessage(options)) { + if !options.force && !command.PromptForConfirmation(dockerCli.In(), dockerCli.Out(), confirmationMessage(dockerCli, options)) { return nil } pruneFuncs := []func(dockerCli command.Cli, all bool, filter opts.FilterOpt) (uint64, string, error){ @@ -96,7 +97,7 @@ func runPrune(dockerCli command.Cli, options pruneOptions) error { } // confirmationMessage constructs a confirmation message that depends on the cli options. -func confirmationMessage(options pruneOptions) string { +func confirmationMessage(dockerCli command.Cli, options pruneOptions) string { t := template.Must(template.New("confirmation message").Parse(confirmationTemplate)) warnings := []string{ @@ -118,9 +119,14 @@ func confirmationMessage(options pruneOptions) string { warnings = append(warnings, "all dangling build cache") } } - if len(options.filter.String()) > 0 { + pruneFilters := command.PruneFilters(dockerCli, options.filter.Value()) + if pruneFilters.Len() > 0 { + f, err := filters.ToJSON(pruneFilters) + if err != nil { + f = "invalid filters" + } warnings = append(warnings, "Elements to be pruned will be filtered with:") - warnings = append(warnings, "label="+options.filter.String()) + warnings = append(warnings, "filter="+f) } var buffer bytes.Buffer diff --git a/cli/command/system/prune_test.go b/cli/command/system/prune_test.go index c0b5cafd3d..6ee9d8fc11 100644 --- a/cli/command/system/prune_test.go +++ b/cli/command/system/prune_test.go @@ -3,6 +3,7 @@ package system import ( "testing" + "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/internal/test" "gotest.tools/assert" is "gotest.tools/assert/cmp" @@ -11,6 +12,7 @@ import ( func TestPrunePromptPre131DoesNotIncludeBuildCache(t *testing.T) { cli := test.NewFakeCli(&fakeClient{version: "1.30"}) cmd := newPruneCommand(cli) + cmd.SetArgs([]string{}) assert.NilError(t, cmd.Execute()) expected := `WARNING! This will remove: - all stopped containers @@ -18,5 +20,23 @@ func TestPrunePromptPre131DoesNotIncludeBuildCache(t *testing.T) { - all dangling images Are you sure you want to continue? [y/N] ` assert.Check(t, is.Equal(expected, cli.OutBuffer().String())) +} +func TestPrunePromptFilters(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{version: "1.30"}) + cli.SetConfigFile(&configfile.ConfigFile{ + PruneFilters: []string{"label!=never=remove-me", "label=remove=me"}, + }) + cmd := newPruneCommand(cli) + cmd.SetArgs([]string{"--filter", "until=24h", "--filter", "label=hello-world", "--filter", "label!=foo=bar", "--filter", "label=bar=baz"}) + + assert.NilError(t, cmd.Execute()) + expected := `WARNING! This will remove: + - all stopped containers + - all networks not used by at least one container + - all dangling images + - Elements to be pruned will be filtered with: + - filter={"label":{"bar=baz":true,"hello-world":true,"remove=me":true},"label!":{"foo=bar":true,"never=remove-me":true},"until":{"24h":true}} +Are you sure you want to continue? [y/N] ` + assert.Check(t, is.Equal(expected, cli.OutBuffer().String())) } From 26e004797be0eb933d7f8092574daff16b3ec687 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 17 Nov 2018 16:53:02 +0100 Subject: [PATCH 2/2] Make system prune warning filters human-readable The warning, printed before running `docker system prune` was printing the filters in JSON format. This patch attempts to make the output human readable; - updating the code, and template to print filters individually - reducing the indentation (which was quite deep) Before this patch was applied; ``` docker system prune --filter until=24h --filter label=hello-world --filter label!=foo=bar --filter label=bar=baz WARNING! This will remove: - all stopped containers - all networks not used by at least one container - all dangling images - all dangling build cache - Elements to be pruned will be filtered with: - label={"label":{"bar=baz":true,"hello-world":true},"label!":{"foo=bar":true},"until":{"24h":true}} Are you sure you want to continue? [y/N] ``` With this patch applied; ``` WARNING! This will remove: - all stopped containers - all networks not used by at least one container - all dangling images - all dangling build cache Items to be pruned will be filtered with: - label!=foo=bar - label!=never=remove-me - label=bar=baz - label=hello-world - label=remove=me - until=24h Are you sure you want to continue? [y/N] ``` Signed-off-by: Sebastiaan van Stijn --- cli/command/system/prune.go | 33 ++++++++++++++++++++++---------- cli/command/system/prune_test.go | 27 +++++++++++++++++--------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/cli/command/system/prune.go b/cli/command/system/prune.go index 8aee689d6d..0e8f115c5a 100644 --- a/cli/command/system/prune.go +++ b/cli/command/system/prune.go @@ -3,6 +3,7 @@ package system import ( "bytes" "fmt" + "sort" "text/template" "github.com/docker/cli/cli" @@ -13,10 +14,10 @@ import ( "github.com/docker/cli/cli/command/network" "github.com/docker/cli/cli/command/volume" "github.com/docker/cli/opts" - "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/versions" - units "github.com/docker/go-units" + "github.com/docker/go-units" "github.com/spf13/cobra" + "vbom.ml/util/sortorder" ) type pruneOptions struct { @@ -54,9 +55,15 @@ func newPruneCommand(dockerCli command.Cli) *cobra.Command { } const confirmationTemplate = `WARNING! This will remove: -{{- range $_, $warning := . }} - - {{ $warning }} +{{- range $_, $warning := .warnings }} + - {{ $warning }} {{- end }} +{{if .filters}} + Items to be pruned will be filtered with: +{{- range $_, $filters := .filters }} + - {{ $filters }} +{{- end }} +{{end}} Are you sure you want to continue?` func runPrune(dockerCli command.Cli, options pruneOptions) error { @@ -119,17 +126,23 @@ func confirmationMessage(dockerCli command.Cli, options pruneOptions) string { warnings = append(warnings, "all dangling build cache") } } + + var filters []string pruneFilters := command.PruneFilters(dockerCli, options.filter.Value()) if pruneFilters.Len() > 0 { - f, err := filters.ToJSON(pruneFilters) - if err != nil { - f = "invalid filters" + // TODO remove fixed list of filters, and print all filters instead, + // because the list of filters that is supported by the engine may evolve over time. + for _, name := range []string{"label", "label!", "until"} { + for _, v := range pruneFilters.Get(name) { + filters = append(filters, name+"="+v) + } } - warnings = append(warnings, "Elements to be pruned will be filtered with:") - warnings = append(warnings, "filter="+f) + sort.Slice(filters, func(i, j int) bool { + return sortorder.NaturalLess(filters[i], filters[j]) + }) } var buffer bytes.Buffer - t.Execute(&buffer, &warnings) + t.Execute(&buffer, map[string][]string{"warnings": warnings, "filters": filters}) return buffer.String() } diff --git a/cli/command/system/prune_test.go b/cli/command/system/prune_test.go index 6ee9d8fc11..761549fcc4 100644 --- a/cli/command/system/prune_test.go +++ b/cli/command/system/prune_test.go @@ -15,15 +15,16 @@ func TestPrunePromptPre131DoesNotIncludeBuildCache(t *testing.T) { cmd.SetArgs([]string{}) assert.NilError(t, cmd.Execute()) expected := `WARNING! This will remove: - - all stopped containers - - all networks not used by at least one container - - all dangling images + - all stopped containers + - all networks not used by at least one container + - all dangling images + Are you sure you want to continue? [y/N] ` assert.Check(t, is.Equal(expected, cli.OutBuffer().String())) } func TestPrunePromptFilters(t *testing.T) { - cli := test.NewFakeCli(&fakeClient{version: "1.30"}) + cli := test.NewFakeCli(&fakeClient{version: "1.31"}) cli.SetConfigFile(&configfile.ConfigFile{ PruneFilters: []string{"label!=never=remove-me", "label=remove=me"}, }) @@ -32,11 +33,19 @@ func TestPrunePromptFilters(t *testing.T) { assert.NilError(t, cmd.Execute()) expected := `WARNING! This will remove: - - all stopped containers - - all networks not used by at least one container - - all dangling images - - Elements to be pruned will be filtered with: - - filter={"label":{"bar=baz":true,"hello-world":true,"remove=me":true},"label!":{"foo=bar":true,"never=remove-me":true},"until":{"24h":true}} + - all stopped containers + - all networks not used by at least one container + - all dangling images + - all dangling build cache + + Items to be pruned will be filtered with: + - label!=foo=bar + - label!=never=remove-me + - label=bar=baz + - label=hello-world + - label=remove=me + - until=24h + Are you sure you want to continue? [y/N] ` assert.Check(t, is.Equal(expected, cli.OutBuffer().String())) }