From 0ea587b0d73c47d4e6a9a22ac4f1b811e1fcf5fa Mon Sep 17 00:00:00 2001 From: Conner Crosby Date: Fri, 8 Apr 2022 22:43:28 -0400 Subject: [PATCH] Add --force option to network rm subcommand The code is similar to that used by the volume rm subcommand, however, one difference I noticed was VolumeRemove takes the force flag/option was a parameter. This isn't the case for NetworkRemove. To get NetworkRemove to take a similar parameter, this would require modifying the Docker daemon. For now this isn't a route I wish to take when the code can be arrange to mimic the same behavior. Co-authored-by: Sebastiaan van Stijn Signed-off-by: Conner Crosby --- cli/command/network/client_test.go | 12 +++ cli/command/network/remove.go | 20 ++++- cli/command/network/remove_test.go | 97 ++++++++++++++++++++++++ contrib/completion/bash/docker | 2 +- docs/reference/commandline/network_rm.md | 2 +- 5 files changed, 128 insertions(+), 5 deletions(-) create mode 100644 cli/command/network/remove_test.go diff --git a/cli/command/network/client_test.go b/cli/command/network/client_test.go index 33cec6e5c5..38d942e23d 100644 --- a/cli/command/network/client_test.go +++ b/cli/command/network/client_test.go @@ -13,6 +13,7 @@ type fakeClient struct { networkCreateFunc func(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) networkConnectFunc func(ctx context.Context, networkID, container string, config *network.EndpointSettings) error networkDisconnectFunc func(ctx context.Context, networkID, container string, force bool) error + networkRemoveFunc func(ctx context.Context, networkID string) error networkListFunc func(ctx context.Context, options types.NetworkListOptions) ([]types.NetworkResource, error) } @@ -43,3 +44,14 @@ func (c *fakeClient) NetworkList(ctx context.Context, options types.NetworkListO } return []types.NetworkResource{}, nil } + +func (c *fakeClient) NetworkRemove(ctx context.Context, networkID string) error { + if c.networkRemoveFunc != nil { + return c.networkRemoveFunc(ctx, networkID) + } + return nil +} + +func (c *fakeClient) NetworkInspectWithRaw(ctx context.Context, network string, options types.NetworkInspectOptions) (types.NetworkResource, []byte, error) { + return types.NetworkResource{}, nil, nil +} diff --git a/cli/command/network/remove.go b/cli/command/network/remove.go index 66f48197f2..6ee28b053b 100644 --- a/cli/command/network/remove.go +++ b/cli/command/network/remove.go @@ -7,19 +7,30 @@ 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/spf13/cobra" ) +type removeOptions struct { + force bool +} + func newRemoveCommand(dockerCli command.Cli) *cobra.Command { - return &cobra.Command{ + var opts removeOptions + + cmd := &cobra.Command{ Use: "rm NETWORK [NETWORK...]", Aliases: []string{"remove"}, Short: "Remove one or more networks", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runRemove(dockerCli, args) + return runRemove(dockerCli, args, &opts) }, } + + flags := cmd.Flags() + flags.BoolVarP(&opts.force, "force", "f", false, "Do not error if the network does not exist") + return cmd } const ingressWarning = "WARNING! Before removing the routing-mesh network, " + @@ -27,7 +38,7 @@ const ingressWarning = "WARNING! Before removing the routing-mesh network, " + "Otherwise, removal may not be effective and functionality of newly create " + "ingress networks will be impaired.\nAre you sure you want to continue?" -func runRemove(dockerCli command.Cli, networks []string) error { +func runRemove(dockerCli command.Cli, networks []string, opts *removeOptions) error { client := dockerCli.Client() ctx := context.Background() status := 0 @@ -39,6 +50,9 @@ func runRemove(dockerCli command.Cli, networks []string) error { continue } if err := client.NetworkRemove(ctx, name); err != nil { + if opts.force && errdefs.IsNotFound(err) { + continue + } fmt.Fprintf(dockerCli.Err(), "%s\n", err) status = 1 continue diff --git a/cli/command/network/remove_test.go b/cli/command/network/remove_test.go new file mode 100644 index 0000000000..ed85e54699 --- /dev/null +++ b/cli/command/network/remove_test.go @@ -0,0 +1,97 @@ +package network + +import ( + "context" + "io" + "testing" + + "github.com/docker/cli/internal/test" + "github.com/docker/docker/errdefs" + "github.com/pkg/errors" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestNetworkRemoveForce(t *testing.T) { + tests := []struct { + doc string + args []string + expectedErr string + }{ + { + doc: "existing network", + args: []string{"existing-network"}, + }, + { + doc: "existing network (forced)", + args: []string{"--force", "existing-network"}, + }, + { + doc: "non-existing network", + args: []string{"no-such-network"}, + expectedErr: "no such network: no-such-network", + }, + { + doc: "non-existing network (forced)", + args: []string{"--force", "no-such-network"}, + }, + { + doc: "in-use network", + args: []string{"in-use-network"}, + expectedErr: "network is in use", + }, + { + doc: "in-use network (forced)", + args: []string{"--force", "in-use-network"}, + expectedErr: "network is in use", + }, + { + doc: "multiple networks", + args: []string{"existing-network", "no-such-network"}, + expectedErr: "no such network: no-such-network", + }, + { + doc: "multiple networks (forced)", + args: []string{"--force", "existing-network", "no-such-network"}, + }, + { + doc: "multiple networks 2 (forced)", + args: []string{"--force", "existing-network", "no-such-network", "in-use-network"}, + expectedErr: "network is in use", + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + fakeCli := test.NewFakeCli(&fakeClient{ + networkRemoveFunc: func(ctx context.Context, networkID string) error { + switch networkID { + case "no-such-network": + return errdefs.NotFound(errors.New("no such network: no-such-network")) + case "in-use-network": + return errdefs.Forbidden(errors.New("network is in use")) + case "existing-network": + return nil + default: + return nil + } + }, + }) + + cmd := newRemoveCommand(fakeCli) + cmd.SetOut(io.Discard) + cmd.SetErr(fakeCli.ErrBuffer()) + cmd.SetArgs(tc.args) + + err := cmd.Execute() + if tc.expectedErr == "" { + assert.NilError(t, err) + } else { + assert.Check(t, is.Contains(fakeCli.ErrBuffer().String(), tc.expectedErr)) + assert.ErrorContains(t, err, "Code: 1") + + } + }) + } +} diff --git a/contrib/completion/bash/docker b/contrib/completion/bash/docker index 5a1ec89f5a..bc954e7782 100644 --- a/contrib/completion/bash/docker +++ b/contrib/completion/bash/docker @@ -3495,7 +3495,7 @@ _docker_network_prune() { _docker_network_rm() { case "$cur" in -*) - COMPREPLY=( $( compgen -W "--help" -- "$cur" ) ) + COMPREPLY=( $( compgen -W "--force -f --help" -- "$cur" ) ) ;; *) __docker_complete_networks --filter type=custom diff --git a/docs/reference/commandline/network_rm.md b/docs/reference/commandline/network_rm.md index ecbcdd16f2..79869db5d7 100644 --- a/docs/reference/commandline/network_rm.md +++ b/docs/reference/commandline/network_rm.md @@ -15,7 +15,7 @@ Aliases: rm, remove Options: - --help Print usage + -f, --force Do not error if the network does not exist ``` ## Description