From 9a071a993a7d15cc490937104b3d37fd6a339bc2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Aug 2020 16:04:32 +0200 Subject: [PATCH] Return zero exit-code when force-removing non-existing containers When using `docker rm` / `docker container rm` with the `-f` / `--force` option, attempts to remove non-existing containers should print a warning, but should return a zero exit code ("successful"). Currently, a non-zero exit code is returned, marking the removal as "failed"; $ docker rm -fv 798c9471b695 Error: No such container: 798c9471b695 $ echo $? 1 The command should match the behavior of `rm` / `rm -f`, with the exception that a warning is printed (instead of silently ignored): Running `rm` with `-f` silences output and returns a zero exit code: touch some-file && rm -f no-such-file some-file; echo exit code: $?; ls -la # exit code: 0 # total 0 # drwxr-xr-x 2 sebastiaan staff 64 Aug 14 12:17 . # drwxr-xr-x 199 sebastiaan staff 6368 Aug 14 12:13 .. mkdir some-directory && rm -rf no-such-directory some-directory; echo exit code: $?; ls -la # exit code: 0 # total 0 # drwxr-xr-x 2 sebastiaan staff 64 Aug 14 12:17 . # drwxr-xr-x 199 sebastiaan staff 6368 Aug 14 12:13 .. Note that other reasons for a delete to fail should still result in a non-zero exit code, matching the behavior of `rm`. For instance, in the example below, the `rm` failed because directories can only be removed if the `-r` option is used; touch some-file && mkdir some-directory && rm -f some-directory no-such-file some-file; echo exit code: $?; ls -la # rm: some-directory: is a directory # exit code: 1 # total 0 # drwxr-xr-x 3 sebastiaan staff 96 Aug 14 14:15 . # drwxr-xr-x 199 sebastiaan staff 6368 Aug 14 12:13 .. # drwxr-xr-x 2 sebastiaan staff 64 Aug 14 14:15 some-directory This patch updates the `docker rm` / `docker container rm` command to not produce an error when attempting to remove a missing containers, and instead only print the error, but return a zero (0) exit code. With this patch applied: docker create --name mycontainer busybox \ && docker rm nosuchcontainer mycontainer; \ echo exit code: $?; \ docker ps -a --filter name=mycontainer # df23cc8573f00e97d6e948b48d9ea7d75ce3b4faaab4fe1d3458d3bfa451f39d # mycontainer # Error: No such container: nosuchcontainer # exit code: 0 # CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES Signed-off-by: Sebastiaan van Stijn --- cli/command/container/client_test.go | 8 +++++ cli/command/container/rm.go | 5 +++ cli/command/container/rm_test.go | 46 ++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 cli/command/container/rm_test.go diff --git a/cli/command/container/client_test.go b/cli/command/container/client_test.go index 67ae04904f..3643550b92 100644 --- a/cli/command/container/client_test.go +++ b/cli/command/container/client_test.go @@ -31,6 +31,7 @@ type fakeClient struct { containerListFunc func(types.ContainerListOptions) ([]types.Container, error) containerExportFunc func(string) (io.ReadCloser, error) containerExecResizeFunc func(id string, options types.ResizeOptions) error + containerRemoveFunc func(ctx context.Context, container string, options types.ContainerRemoveOptions) error Version string } @@ -80,6 +81,13 @@ func (f *fakeClient) ContainerCreate( return container.ContainerCreateCreatedBody{}, nil } +func (f *fakeClient) ContainerRemove(ctx context.Context, container string, options types.ContainerRemoveOptions) error { + if f.containerRemoveFunc != nil { + return f.containerRemoveFunc(ctx, container, options) + } + return nil +} + func (f *fakeClient) ImageCreate(ctx context.Context, parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { if f.imageCreateFunc != nil { return f.imageCreateFunc(parentReference, options) diff --git a/cli/command/container/rm.go b/cli/command/container/rm.go index 54ce2ad1a8..2a2528c6f4 100644 --- a/cli/command/container/rm.go +++ b/cli/command/container/rm.go @@ -8,6 +8,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/docker/api/types" + "github.com/docker/docker/errdefs" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -61,6 +62,10 @@ func runRm(dockerCli command.Cli, opts *rmOptions) error { for _, name := range opts.containers { if err := <-errChan; err != nil { + if opts.force && errdefs.IsNotFound(err) { + fmt.Fprintln(dockerCli.Err(), err) + continue + } errs = append(errs, err.Error()) continue } diff --git a/cli/command/container/rm_test.go b/cli/command/container/rm_test.go new file mode 100644 index 0000000000..07c3f490f0 --- /dev/null +++ b/cli/command/container/rm_test.go @@ -0,0 +1,46 @@ +package container + +import ( + "context" + "fmt" + "io/ioutil" + "sort" + "testing" + + "github.com/docker/cli/internal/test" + "github.com/docker/docker/api/types" + "github.com/docker/docker/errdefs" + "gotest.tools/v3/assert" +) + +func TestRemoveForce(t *testing.T) { + var removed []string + + cli := test.NewFakeCli(&fakeClient{ + containerRemoveFunc: func(ctx context.Context, container string, options types.ContainerRemoveOptions) error { + removed = append(removed, container) + if container == "nosuchcontainer" { + return errdefs.NotFound(fmt.Errorf("Error: No such container: " + container)) + } + return nil + }, + Version: "1.36", + }) + cmd := NewRmCommand(cli) + cmd.SetOut(ioutil.Discard) + + t.Run("without force", func(t *testing.T) { + cmd.SetArgs([]string{"nosuchcontainer", "mycontainer"}) + removed = []string{} + assert.ErrorContains(t, cmd.Execute(), "No such container") + sort.Strings(removed) + assert.DeepEqual(t, removed, []string{"mycontainer", "nosuchcontainer"}) + }) + t.Run("with force", func(t *testing.T) { + cmd.SetArgs([]string{"--force", "nosuchcontainer", "mycontainer"}) + removed = []string{} + assert.NilError(t, cmd.Execute()) + sort.Strings(removed) + assert.DeepEqual(t, removed, []string{"mycontainer", "nosuchcontainer"}) + }) +}