Merge pull request #23311 from WeiZhang555/cobra-rm

Migrate rm command to cobra

api/client/commands.go

@@ -16,7 +16,6 @@ func (cli *DockerCli) Command(name string) func(...string) error {
 		"ps":      cli.CmdPs,
 		"pull":    cli.CmdPull,
 		"push":    cli.CmdPush,
-		"rm":      cli.CmdRm,
 		"save":    cli.CmdSave,
 		"stats":   cli.CmdStats,
 		"update":  cli.CmdUpdate,

api/client/container/rm.go

@@ -0,0 +1,76 @@
+package container
+
+import (
+	"fmt"
+	"strings"
+
+	"golang.org/x/net/context"
+
+	"github.com/docker/docker/api/client"
+	"github.com/docker/docker/cli"
+	"github.com/docker/engine-api/types"
+	"github.com/spf13/cobra"
+)
+
+type rmOptions struct {
+	rmVolumes bool
+	rmLink    bool
+	force     bool
+
+	containers []string
+}
+
+// NewRmCommand creats a new cobra.Command for `docker rm`
+func NewRmCommand(dockerCli *client.DockerCli) *cobra.Command {
+	var opts rmOptions
+
+	cmd := &cobra.Command{
+		Use:   "rm [OPTIONS] CONTAINER [CONTAINER...]",
+		Short: "Remove one or more containers",
+		Args:  cli.RequiresMinArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			opts.containers = args
+			return runRm(dockerCli, &opts)
+		},
+	}
+
+	flags := cmd.Flags()
+	flags.BoolVarP(&opts.rmVolumes, "volumes", "v", false, "Remove the volumes associated with the container")
+	flags.BoolVarP(&opts.rmLink, "link", "l", false, "Remove the specified link")
+	flags.BoolVarP(&opts.force, "force", "f", false, "Force the removal of a running container (uses SIGKILL)")
+	return cmd
+}
+
+func runRm(dockerCli *client.DockerCli, opts *rmOptions) error {
+	ctx := context.Background()
+
+	var errs []string
+	for _, name := range opts.containers {
+		if name == "" {
+			return fmt.Errorf("Container name cannot be empty")
+		}
+		name = strings.Trim(name, "/")
+
+		if err := removeContainer(dockerCli, ctx, name, opts.rmVolumes, opts.rmLink, opts.force); err != nil {
+			errs = append(errs, err.Error())
+		} else {
+			fmt.Fprintf(dockerCli.Out(), "%s\n", name)
+		}
+	}
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
+	}
+	return nil
+}
+
+func removeContainer(dockerCli *client.DockerCli, ctx context.Context, container string, removeVolumes, removeLinks, force bool) error {
+	options := types.ContainerRemoveOptions{
+		RemoveVolumes: removeVolumes,
+		RemoveLinks:   removeLinks,
+		Force:         force,
+	}
+	if err := dockerCli.Client().ContainerRemove(ctx, container, options); err != nil {
+		return err
+	}
+	return nil
+}

api/client/container/run.go

@@ -234,7 +234,7 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
 		defer func() {
 			// Explicitly not sharing the context as it could be "Done" (by calling cancelFun)
 			// and thus the container would not be removed.
-			if err := dockerCli.RemoveContainer(context.Background(), createResponse.ID, true, false, true); err != nil {
+			if err := removeContainer(dockerCli, context.Background(), createResponse.ID, true, false, true); err != nil {
 				fmt.Fprintf(stderr, "%v\n", err)
 			}
 		}()

api/client/rm.go

@@ -1,60 +0,0 @@
-package client
-
-import (
-	"fmt"
-	"strings"
-
-	"golang.org/x/net/context"
-
-	Cli "github.com/docker/docker/cli"
-	flag "github.com/docker/docker/pkg/mflag"
-	"github.com/docker/engine-api/types"
-)
-
-// CmdRm removes one or more containers.
-//
-// Usage: docker rm [OPTIONS] CONTAINER [CONTAINER...]
-func (cli *DockerCli) CmdRm(args ...string) error {
-	cmd := Cli.Subcmd("rm", []string{"CONTAINER [CONTAINER...]"}, Cli.DockerCommands["rm"].Description, true)
-	v := cmd.Bool([]string{"v", "-volumes"}, false, "Remove the volumes associated with the container")
-	link := cmd.Bool([]string{"l", "-link"}, false, "Remove the specified link")
-	force := cmd.Bool([]string{"f", "-force"}, false, "Force the removal of a running container (uses SIGKILL)")
-	cmd.Require(flag.Min, 1)
-
-	cmd.ParseFlags(args, true)
-
-	ctx := context.Background()
-
-	var errs []string
-	for _, name := range cmd.Args() {
-		if name == "" {
-			return fmt.Errorf("Container name cannot be empty")
-		}
-		name = strings.Trim(name, "/")
-
-		if err := cli.RemoveContainer(ctx, name, *v, *link, *force); err != nil {
-			errs = append(errs, err.Error())
-		} else {
-			fmt.Fprintf(cli.out, "%s\n", name)
-		}
-	}
-	if len(errs) > 0 {
-		return fmt.Errorf("%s", strings.Join(errs, "\n"))
-	}
-	return nil
-}
-
-// RemoveContainer removes a container
-// TODO: this can be unexported again once all container commands are under
-// api/client/container
-func (cli *DockerCli) RemoveContainer(ctx context.Context, container string, removeVolumes, removeLinks, force bool) error {
-	options := types.ContainerRemoveOptions{
-		RemoveVolumes: removeVolumes,
-		RemoveLinks:   removeLinks,
-		Force:         force,
-	}
-	if err := cli.client.ContainerRemove(ctx, container, options); err != nil {
-		return err
-	}
-	return nil
-}

cli/cobraadaptor/adaptor.go

@@ -44,6 +44,7 @@ func NewCobraAdaptor(clientFlags *cliflags.ClientFlags) CobraAdaptor {
 		container.NewPortCommand(dockerCli),
 		container.NewRenameCommand(dockerCli),
 		container.NewRestartCommand(dockerCli),
+		container.NewRmCommand(dockerCli),
 		container.NewRunCommand(dockerCli),
 		container.NewStartCommand(dockerCli),
 		container.NewStopCommand(dockerCli),

cli/usage.go

@@ -21,7 +21,6 @@ var DockerCommandUsage = []Command{
 	{"ps", "List containers"},
 	{"pull", "Pull an image or a repository from a registry"},
 	{"push", "Push an image or a repository to a registry"},
-	{"rm", "Remove one or more containers"},
 	{"save", "Save one or more images to a tar archive"},
 	{"stats", "Display a live stream of container(s) resource usage statistics"},
 	{"update", "Update configuration of one or more containers"},

integration-cli/docker_cli_help_test.go

@@ -184,7 +184,7 @@ func (s *DockerSuite) TestHelpExitCodesHelpOutput(c *check.C) {
 	c.Assert(stdout, checker.Equals, "")
 	// Should not contain full help text but should contain info about
 	// # of args and Usage line
-	c.Assert(stderr, checker.Contains, "requires a minimum", check.Commentf("Missing # of args text from 'docker rm'\n"))
+	c.Assert(stderr, checker.Contains, "requires at least 1 argument", check.Commentf("Missing # of args text from 'docker rm'\n"))
 
 	// docker rm NoSuchContainer: stdout=empty, stderr=all, rc=0
 	// testing to make sure no blank line on error