Migrate exec command to cobra

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>

api/client/cli.go

@@ -87,7 +87,12 @@ func (cli *DockerCli) ConfigFile() *configfile.ConfigFile {
 	return cli.configFile
 }
 
-// IsTerminalOut returns true if the clients stdin is a TTY
+// IsTerminalIn returns true if the clients stdin is a TTY
+func (cli *DockerCli) IsTerminalIn() bool {
+	return cli.isTerminalIn
+}
+
+// IsTerminalOut returns true if the clients stdout is a TTY
 func (cli *DockerCli) IsTerminalOut() bool {
 	return cli.isTerminalOut
 }

api/client/commands.go

@@ -3,7 +3,6 @@ package client
 // Command returns a cli command handler if one exists
 func (cli *DockerCli) Command(name string) func(...string) error {
 	return map[string]func(...string) error{
-		"exec":    cli.CmdExec,
 		"inspect": cli.CmdInspect,
 	}[name]
 }

api/client/container/exec.go

@@ -0,0 +1,175 @@
+package container
+
+import (
+	"fmt"
+	"io"
+
+	"golang.org/x/net/context"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/api/client"
+	"github.com/docker/docker/cli"
+	"github.com/docker/docker/pkg/promise"
+	"github.com/docker/engine-api/types"
+	"github.com/spf13/cobra"
+)
+
+type execOptions struct {
+	detachKeys  string
+	interactive bool
+	tty         bool
+	detach      bool
+	user        string
+	privileged  bool
+}
+
+// NewExecCommand creats a new cobra.Command for `docker exec`
+func NewExecCommand(dockerCli *client.DockerCli) *cobra.Command {
+	var opts execOptions
+
+	cmd := &cobra.Command{
+		Use:   "exec CONTAINER COMMAND [ARG...]",
+		Short: "Run a command in a running container",
+		Args:  cli.RequiresMinArgs(2),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			container := args[0]
+			execCmd := args[1:]
+			return runExec(dockerCli, &opts, container, execCmd)
+		},
+	}
+
+	flags := cmd.Flags()
+	flags.SetInterspersed(false)
+
+	flags.StringVarP(&opts.detachKeys, "detach-keys", "", "", "Override the key sequence for detaching a container")
+	flags.BoolVarP(&opts.interactive, "interactive", "i", false, "Keep STDIN open even if not attached")
+	flags.BoolVarP(&opts.tty, "tty", "t", false, "Allocate a pseudo-TTY")
+	flags.BoolVarP(&opts.detach, "detach", "d", false, "Detached mode: run command in the background")
+	flags.StringVarP(&opts.user, "user", "u", "", "Username or UID (format: <name|uid>[:<group|gid>])")
+	flags.BoolVarP(&opts.privileged, "privileged", "", false, "Give extended privileges to the command")
+
+	return cmd
+}
+
+func runExec(dockerCli *client.DockerCli, opts *execOptions, container string, execCmd []string) error {
+	execConfig, err := parseExec(opts, container, execCmd)
+	// just in case the ParseExec does not exit
+	if container == "" || err != nil {
+		return cli.StatusError{StatusCode: 1}
+	}
+
+	if opts.detachKeys != "" {
+		dockerCli.ConfigFile().DetachKeys = opts.detachKeys
+	}
+
+	// Send client escape keys
+	execConfig.DetachKeys = dockerCli.ConfigFile().DetachKeys
+
+	ctx := context.Background()
+
+	response, err := dockerCli.Client().ContainerExecCreate(ctx, container, *execConfig)
+	if err != nil {
+		return err
+	}
+
+	execID := response.ID
+	if execID == "" {
+		fmt.Fprintf(dockerCli.Out(), "exec ID empty")
+		return nil
+	}
+
+	//Temp struct for execStart so that we don't need to transfer all the execConfig
+	if !execConfig.Detach {
+		if err := dockerCli.CheckTtyInput(execConfig.AttachStdin, execConfig.Tty); err != nil {
+			return err
+		}
+	} else {
+		execStartCheck := types.ExecStartCheck{
+			Detach: execConfig.Detach,
+			Tty:    execConfig.Tty,
+		}
+
+		if err := dockerCli.Client().ContainerExecStart(ctx, execID, execStartCheck); err != nil {
+			return err
+		}
+		// For now don't print this - wait for when we support exec wait()
+		// fmt.Fprintf(dockerCli.Out(), "%s\n", execID)
+		return nil
+	}
+
+	// Interactive exec requested.
+	var (
+		out, stderr io.Writer
+		in          io.ReadCloser
+		errCh       chan error
+	)
+
+	if execConfig.AttachStdin {
+		in = dockerCli.In()
+	}
+	if execConfig.AttachStdout {
+		out = dockerCli.Out()
+	}
+	if execConfig.AttachStderr {
+		if execConfig.Tty {
+			stderr = dockerCli.Out()
+		} else {
+			stderr = dockerCli.Err()
+		}
+	}
+
+	resp, err := dockerCli.Client().ContainerExecAttach(ctx, execID, *execConfig)
+	if err != nil {
+		return err
+	}
+	defer resp.Close()
+	errCh = promise.Go(func() error {
+		return dockerCli.HoldHijackedConnection(ctx, execConfig.Tty, in, out, stderr, resp)
+	})
+
+	if execConfig.Tty && dockerCli.IsTerminalIn() {
+		if err := dockerCli.MonitorTtySize(ctx, execID, true); err != nil {
+			fmt.Fprintf(dockerCli.Err(), "Error monitoring TTY size: %s\n", err)
+		}
+	}
+
+	if err := <-errCh; err != nil {
+		logrus.Debugf("Error hijack: %s", err)
+		return err
+	}
+
+	var status int
+	if _, status, err = dockerCli.GetExecExitCode(ctx, execID); err != nil {
+		return err
+	}
+
+	if status != 0 {
+		return cli.StatusError{StatusCode: status}
+	}
+
+	return nil
+}
+
+// parseExec parses the specified args for the specified command and generates
+// an ExecConfig from it.
+func parseExec(opts *execOptions, container string, execCmd []string) (*types.ExecConfig, error) {
+	execConfig := &types.ExecConfig{
+		User:       opts.user,
+		Privileged: opts.privileged,
+		Tty:        opts.tty,
+		Cmd:        execCmd,
+		Detach:     opts.detach,
+		// container is not used here
+	}
+
+	// If -d is not set, attach to everything by default
+	if !opts.detach {
+		execConfig.AttachStdout = true
+		execConfig.AttachStderr = true
+		if opts.interactive {
+			execConfig.AttachStdin = true
+		}
+	}
+
+	return execConfig, nil
+}

api/client/container/exec_test.go

@@ -1,41 +1,40 @@
-package client
+package container
 
 import (
-	"fmt"
-	"io/ioutil"
 	"testing"
 
-	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/engine-api/types"
 )
 
 type arguments struct {
-	args []string
+	options   execOptions
+	container string
+	execCmd   []string
 }
 
 func TestParseExec(t *testing.T) {
-	invalids := map[*arguments]error{
-		&arguments{[]string{"-unknown"}}: fmt.Errorf("flag provided but not defined: -unknown"),
-		&arguments{[]string{"-u"}}:       fmt.Errorf("flag needs an argument: -u"),
-		&arguments{[]string{"--user"}}:   fmt.Errorf("flag needs an argument: --user"),
-	}
 	valids := map[*arguments]*types.ExecConfig{
 		&arguments{
-			[]string{"container", "command"},
+			execCmd: []string{"command"},
 		}: {
 			Cmd:          []string{"command"},
 			AttachStdout: true,
 			AttachStderr: true,
 		},
 		&arguments{
-			[]string{"container", "command1", "command2"},
+			execCmd: []string{"command1", "command2"},
 		}: {
 			Cmd:          []string{"command1", "command2"},
 			AttachStdout: true,
 			AttachStderr: true,
 		},
 		&arguments{
-			[]string{"-i", "-t", "-u", "uid", "container", "command"},
+			options: execOptions{
+				interactive: true,
+				tty:         true,
+				user:        "uid",
+			},
+			execCmd: []string{"command"},
 		}: {
 			User:         "uid",
 			AttachStdin:  true,
@@ -45,7 +44,10 @@ func TestParseExec(t *testing.T) {
 			Cmd:          []string{"command"},
 		},
 		&arguments{
-			[]string{"-d", "container", "command"},
+			options: execOptions{
+				detach: true,
+			},
+			execCmd: []string{"command"},
 		}: {
 			AttachStdin:  false,
 			AttachStdout: false,
@@ -54,7 +56,12 @@ func TestParseExec(t *testing.T) {
 			Cmd:          []string{"command"},
 		},
 		&arguments{
-			[]string{"-t", "-i", "-d", "container", "command"},
+			options: execOptions{
+				tty:         true,
+				interactive: true,
+				detach:      true,
+			},
+			execCmd: []string{"command"},
 		}: {
 			AttachStdin:  false,
 			AttachStdout: false,
@@ -64,21 +71,9 @@ func TestParseExec(t *testing.T) {
 			Cmd:          []string{"command"},
 		},
 	}
-	for invalid, expectedError := range invalids {
-		cmd := flag.NewFlagSet("exec", flag.ContinueOnError)
-		cmd.ShortUsage = func() {}
-		cmd.SetOutput(ioutil.Discard)
-		_, err := ParseExec(cmd, invalid.args)
-		if err == nil || err.Error() != expectedError.Error() {
-			t.Fatalf("Expected an error [%v] for %v, got %v", expectedError, invalid, err)
-		}
 
-	}
 	for valid, expectedExecConfig := range valids {
-		cmd := flag.NewFlagSet("exec", flag.ContinueOnError)
-		cmd.ShortUsage = func() {}
-		cmd.SetOutput(ioutil.Discard)
-		execConfig, err := ParseExec(cmd, valid.args)
+		execConfig, err := parseExec(&valid.options, valid.container, valid.execCmd)
 		if err != nil {
 			t.Fatal(err)
 		}

api/client/exec.go

@@ -1,160 +0,0 @@
-package client
-
-import (
-	"fmt"
-	"io"
-
-	"golang.org/x/net/context"
-
-	"github.com/Sirupsen/logrus"
-	Cli "github.com/docker/docker/cli"
-	flag "github.com/docker/docker/pkg/mflag"
-	"github.com/docker/docker/pkg/promise"
-	"github.com/docker/engine-api/types"
-)
-
-// CmdExec runs a command in a running container.
-//
-// Usage: docker exec [OPTIONS] CONTAINER COMMAND [ARG...]
-func (cli *DockerCli) CmdExec(args ...string) error {
-	cmd := Cli.Subcmd("exec", []string{"[OPTIONS] CONTAINER COMMAND [ARG...]"}, Cli.DockerCommands["exec"].Description, true)
-	detachKeys := cmd.String([]string{"-detach-keys"}, "", "Override the key sequence for detaching a container")
-
-	execConfig, err := ParseExec(cmd, args)
-	container := cmd.Arg(0)
-	// just in case the ParseExec does not exit
-	if container == "" || err != nil {
-		return Cli.StatusError{StatusCode: 1}
-	}
-
-	if *detachKeys != "" {
-		cli.configFile.DetachKeys = *detachKeys
-	}
-
-	// Send client escape keys
-	execConfig.DetachKeys = cli.configFile.DetachKeys
-
-	ctx := context.Background()
-
-	response, err := cli.client.ContainerExecCreate(ctx, container, *execConfig)
-	if err != nil {
-		return err
-	}
-
-	execID := response.ID
-	if execID == "" {
-		fmt.Fprintf(cli.out, "exec ID empty")
-		return nil
-	}
-
-	//Temp struct for execStart so that we don't need to transfer all the execConfig
-	if !execConfig.Detach {
-		if err := cli.CheckTtyInput(execConfig.AttachStdin, execConfig.Tty); err != nil {
-			return err
-		}
-	} else {
-		execStartCheck := types.ExecStartCheck{
-			Detach: execConfig.Detach,
-			Tty:    execConfig.Tty,
-		}
-
-		if err := cli.client.ContainerExecStart(ctx, execID, execStartCheck); err != nil {
-			return err
-		}
-		// For now don't print this - wait for when we support exec wait()
-		// fmt.Fprintf(cli.out, "%s\n", execID)
-		return nil
-	}
-
-	// Interactive exec requested.
-	var (
-		out, stderr io.Writer
-		in          io.ReadCloser
-		errCh       chan error
-	)
-
-	if execConfig.AttachStdin {
-		in = cli.in
-	}
-	if execConfig.AttachStdout {
-		out = cli.out
-	}
-	if execConfig.AttachStderr {
-		if execConfig.Tty {
-			stderr = cli.out
-		} else {
-			stderr = cli.err
-		}
-	}
-
-	resp, err := cli.client.ContainerExecAttach(ctx, execID, *execConfig)
-	if err != nil {
-		return err
-	}
-	defer resp.Close()
-	errCh = promise.Go(func() error {
-		return cli.HoldHijackedConnection(ctx, execConfig.Tty, in, out, stderr, resp)
-	})
-
-	if execConfig.Tty && cli.isTerminalIn {
-		if err := cli.MonitorTtySize(ctx, execID, true); err != nil {
-			fmt.Fprintf(cli.err, "Error monitoring TTY size: %s\n", err)
-		}
-	}
-
-	if err := <-errCh; err != nil {
-		logrus.Debugf("Error hijack: %s", err)
-		return err
-	}
-
-	var status int
-	if _, status, err = cli.getExecExitCode(ctx, execID); err != nil {
-		return err
-	}
-
-	if status != 0 {
-		return Cli.StatusError{StatusCode: status}
-	}
-
-	return nil
-}
-
-// ParseExec parses the specified args for the specified command and generates
-// an ExecConfig from it.
-// If the minimal number of specified args is not right or if specified args are
-// not valid, it will return an error.
-func ParseExec(cmd *flag.FlagSet, args []string) (*types.ExecConfig, error) {
-	var (
-		flStdin      = cmd.Bool([]string{"i", "-interactive"}, false, "Keep STDIN open even if not attached")
-		flTty        = cmd.Bool([]string{"t", "-tty"}, false, "Allocate a pseudo-TTY")
-		flDetach     = cmd.Bool([]string{"d", "-detach"}, false, "Detached mode: run command in the background")
-		flUser       = cmd.String([]string{"u", "-user"}, "", "Username or UID (format: <name|uid>[:<group|gid>])")
-		flPrivileged = cmd.Bool([]string{"-privileged"}, false, "Give extended privileges to the command")
-		execCmd      []string
-	)
-	cmd.Require(flag.Min, 2)
-	if err := cmd.ParseFlags(args, true); err != nil {
-		return nil, err
-	}
-	parsedArgs := cmd.Args()
-	execCmd = parsedArgs[1:]
-
-	execConfig := &types.ExecConfig{
-		User:       *flUser,
-		Privileged: *flPrivileged,
-		Tty:        *flTty,
-		Cmd:        execCmd,
-		Detach:     *flDetach,
-	}
-
-	// If -d is not set, attach to everything by default
-	if !*flDetach {
-		execConfig.AttachStdout = true
-		execConfig.AttachStderr = true
-		if *flStdin {
-			execConfig.AttachStdin = true
-		}
-	}
-
-	return execConfig, nil
-}

api/client/utils.go

@@ -49,9 +49,9 @@ func (cli *DockerCli) ResizeTtyTo(ctx context.Context, id string, height, width
 	}
 }
 
-// getExecExitCode perform an inspect on the exec command. It returns
+// GetExecExitCode perform an inspect on the exec command. It returns
 // the running state and the exit code.
-func (cli *DockerCli) getExecExitCode(ctx context.Context, execID string) (bool, int, error) {
+func (cli *DockerCli) GetExecExitCode(ctx context.Context, execID string) (bool, int, error) {
 	resp, err := cli.client.ContainerExecInspect(ctx, execID)
 	if err != nil {
 		// If we can't connect, then the daemon probably died.

cli/cobraadaptor/adaptor.go

@@ -52,6 +52,7 @@ func NewCobraAdaptor(clientFlags *cliflags.ClientFlags) CobraAdaptor {
 		container.NewCopyCommand(dockerCli),
 		container.NewCreateCommand(dockerCli),
 		container.NewDiffCommand(dockerCli),
+		container.NewExecCommand(dockerCli),
 		container.NewExportCommand(dockerCli),
 		container.NewKillCommand(dockerCli),
 		container.NewLogsCommand(dockerCli),

cli/usage.go

@@ -8,7 +8,6 @@ type Command struct {
 
 // DockerCommandUsage lists the top level docker commands and their short usage
 var DockerCommandUsage = []Command{
-	{"exec", "Run a command in a running container"},
 	{"inspect", "Return low-level information on a container, image or task"},
 }
 

integration-cli/docker_cli_exec_test.go

@@ -214,7 +214,7 @@ func (s *DockerSuite) TestExecParseError(c *check.C) {
 	cmd := exec.Command(dockerBinary, "exec", "top")
 	_, stderr, _, err := runCommandWithStdoutStderr(cmd)
 	c.Assert(err, checker.NotNil)
-	c.Assert(stderr, checker.Contains, "See '"+dockerBinary+" exec --help'")
+	c.Assert(stderr, checker.Contains, "See 'docker exec --help'")
 }
 
 func (s *DockerSuite) TestExecStopNotHanging(c *check.C) {