From 9a493b1bf7ca8933ee9a25570ea63ddc354567ac Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 29 Sep 2022 02:01:09 +0200 Subject: [PATCH] docker context rm: allow --force to ignore non-existing contexts Before this change, running `docker context rm --force` would fail if the context did not exist. This behavior was different from other commands, which allowed ignoring non-existing objects. For example; when trying to remove a non-existing volume, the command would fail without "force": ```bash docker volume rm nosuchvolume Error: No such volume: nosuchvolume echo $? 1 ``` But using the `-f` / `--force` option would make the command complete successfully (the error itself is still printed for informational purposes); ```bash docker volume rm -f nosuchvolume nosuchvolume echo $? 0 ``` With this patch, `docker context rm` behaves the same: ```bash docker context rm nosuchcontext context "nosuchcontext" does not exist echo $? 1 ``` ```bash docker context rm -f nosuchcontext nosuchcontext echo $? 0 ``` This patch also simplifies how we check if the context exists; previously we would try to read the context's metadata; this could fail if a context was corrupted, or if an empty directory was present. This patch now only checks if the directory exists, without first validating the context's data. Signed-off-by: Sebastiaan van Stijn --- cli/command/context/remove.go | 25 +++++++++++++++++++++---- cli/command/context/remove_test.go | 3 +++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/cli/command/context/remove.go b/cli/command/context/remove.go index 59126e5034..434e53e8fa 100644 --- a/cli/command/context/remove.go +++ b/cli/command/context/remove.go @@ -1,12 +1,14 @@ package context import ( - "errors" "fmt" + "os" "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" + "github.com/docker/docker/errdefs" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -50,9 +52,6 @@ func RunRemove(dockerCli command.Cli, opts RemoveOptions, names []string) error } func doRemove(dockerCli command.Cli, name string, isCurrent, force bool) error { - if _, err := dockerCli.ContextStore().GetMetadata(name); err != nil { - return err - } if isCurrent { if !force { return errors.New("context is in use, set -f flag to force remove") @@ -64,5 +63,23 @@ func doRemove(dockerCli command.Cli, name string, isCurrent, force bool) error { return err } } + + if !force { + if err := checkContextExists(dockerCli, name); err != nil { + return err + } + } return dockerCli.ContextStore().Remove(name) } + +// checkContextExists returns an error if the context directory does not exist. +func checkContextExists(dockerCli command.Cli, name string) error { + contextDir := dockerCli.ContextStore().GetStorageInfo(name).MetadataPath + _, err := os.Stat(contextDir) + if os.IsNotExist(err) { + return errdefs.NotFound(errors.Errorf("context %q does not exist", name)) + } + // Ignore other errors; if relevant, they will produce an error when + // performing the actual delete. + return nil +} diff --git a/cli/command/context/remove_test.go b/cli/command/context/remove_test.go index ba3d9d1a56..300d3d2fdc 100644 --- a/cli/command/context/remove_test.go +++ b/cli/command/context/remove_test.go @@ -27,6 +27,9 @@ func TestRemoveNotAContext(t *testing.T) { createTestContext(t, cli, "other") err := RunRemove(cli, RemoveOptions{}, []string{"not-a-context"}) assert.ErrorContains(t, err, `context "not-a-context" does not exist`) + + err = RunRemove(cli, RemoveOptions{Force: true}, []string{"not-a-context"}) + assert.NilError(t, err) } func TestRemoveCurrent(t *testing.T) {