From 9d9dff3d0d9e92adf7c2e59f94c63766659d1d47 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Mon, 20 Jun 2016 13:27:56 +0000 Subject: [PATCH] Migrate exec command to cobra Signed-off-by: Akihiro Suda --- api/client/cli.go | 7 +- api/client/commands.go | 1 - api/client/container/exec.go | 175 ++++++++++++++++++++++++ api/client/{ => container}/exec_test.go | 51 ++++--- api/client/exec.go | 160 ---------------------- api/client/utils.go | 4 +- cli/cobraadaptor/adaptor.go | 1 + cli/usage.go | 1 - integration-cli/docker_cli_exec_test.go | 2 +- 9 files changed, 208 insertions(+), 194 deletions(-) create mode 100644 api/client/container/exec.go rename api/client/{ => container}/exec_test.go (61%) delete mode 100644 api/client/exec.go diff --git a/api/client/cli.go b/api/client/cli.go index 1732c518c0..4f02d0bae2 100644 --- a/api/client/cli.go +++ b/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 } diff --git a/api/client/commands.go b/api/client/commands.go index 8e0bc3e819..d746466316 100644 --- a/api/client/commands.go +++ b/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] } diff --git a/api/client/container/exec.go b/api/client/container/exec.go new file mode 100644 index 0000000000..1fba5ced98 --- /dev/null +++ b/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: [:])") + 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 +} diff --git a/api/client/exec_test.go b/api/client/container/exec_test.go similarity index 61% rename from api/client/exec_test.go rename to api/client/container/exec_test.go index 8b1a3674e9..00d4e503c8 100644 --- a/api/client/exec_test.go +++ b/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) } diff --git a/api/client/exec.go b/api/client/exec.go deleted file mode 100644 index a61c16f376..0000000000 --- a/api/client/exec.go +++ /dev/null @@ -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: [:])") - 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 -} diff --git a/api/client/utils.go b/api/client/utils.go index 99b0f86189..a3dd3dc2e3 100644 --- a/api/client/utils.go +++ b/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. diff --git a/cli/cobraadaptor/adaptor.go b/cli/cobraadaptor/adaptor.go index f2c64c9f47..6614f1fa69 100644 --- a/cli/cobraadaptor/adaptor.go +++ b/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), diff --git a/cli/usage.go b/cli/usage.go index 56e406e3a3..0ad053d734 100644 --- a/cli/usage.go +++ b/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"}, } diff --git a/integration-cli/docker_cli_exec_test.go b/integration-cli/docker_cli_exec_test.go index a2b7331e38..77472d47b4 100644 --- a/integration-cli/docker_cli_exec_test.go +++ b/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) {