From a4abe42cbdbb841342e9b26cd80abb7fe830842a Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Mon, 6 Nov 2023 10:00:13 +0100 Subject: [PATCH 1/2] make runAttach public and allow passing context Signed-off-by: Nicolas De Loof --- cli/command/container/attach.go | 49 +++++++++++++++--------------- cli/command/container/exec.go | 5 ++- cli/command/container/exec_test.go | 2 +- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go index b834197552f2..9f07fe604115 100644 --- a/cli/command/container/attach.go +++ b/cli/command/container/attach.go @@ -17,12 +17,13 @@ import ( "github.com/spf13/cobra" ) -type attachOptions struct { - noStdin bool - proxy bool - detachKeys string +// AttachOptions group options for `attach` command +type AttachOptions struct { + NoStdin bool + Proxy bool + DetachKeys string - container string + Container string } func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, args string) (*types.ContainerJSON, error) { @@ -45,15 +46,15 @@ func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, ar // NewAttachCommand creates a new cobra.Command for `docker attach` func NewAttachCommand(dockerCli command.Cli) *cobra.Command { - var opts attachOptions + var opts AttachOptions cmd := &cobra.Command{ Use: "attach [OPTIONS] CONTAINER", Short: "Attach local standard input, output, and error streams to a running container", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - opts.container = args[0] - return runAttach(dockerCli, &opts) + opts.Container = args[0] + return RunAttach(context.Background(), dockerCli, &opts) }, Annotations: map[string]string{ "aliases": "docker container attach, docker attach", @@ -64,36 +65,36 @@ func NewAttachCommand(dockerCli command.Cli) *cobra.Command { } flags := cmd.Flags() - flags.BoolVar(&opts.noStdin, "no-stdin", false, "Do not attach STDIN") - flags.BoolVar(&opts.proxy, "sig-proxy", true, "Proxy all received signals to the process") - flags.StringVar(&opts.detachKeys, "detach-keys", "", "Override the key sequence for detaching a container") + flags.BoolVar(&opts.NoStdin, "no-stdin", false, "Do not attach STDIN") + flags.BoolVar(&opts.Proxy, "sig-proxy", true, "Proxy all received signals to the process") + flags.StringVar(&opts.DetachKeys, "detach-keys", "", "Override the key sequence for detaching a container") return cmd } -func runAttach(dockerCli command.Cli, opts *attachOptions) error { - ctx := context.Background() +// RunAttach executes an `attach` command +func RunAttach(ctx context.Context, dockerCli command.Cli, opts *AttachOptions) error { apiClient := dockerCli.Client() // request channel to wait for client - resultC, errC := apiClient.ContainerWait(ctx, opts.container, "") + resultC, errC := apiClient.ContainerWait(ctx, opts.Container, "") - c, err := inspectContainerAndCheckState(ctx, apiClient, opts.container) + c, err := inspectContainerAndCheckState(ctx, apiClient, opts.Container) if err != nil { return err } - if err := dockerCli.In().CheckTty(!opts.noStdin, c.Config.Tty); err != nil { + if err := dockerCli.In().CheckTty(!opts.NoStdin, c.Config.Tty); err != nil { return err } detachKeys := dockerCli.ConfigFile().DetachKeys - if opts.detachKeys != "" { - detachKeys = opts.detachKeys + if opts.DetachKeys != "" { + detachKeys = opts.DetachKeys } options := container.AttachOptions{ Stream: true, - Stdin: !opts.noStdin && c.Config.OpenStdin, + Stdin: !opts.NoStdin && c.Config.OpenStdin, Stdout: true, Stderr: true, DetachKeys: detachKeys, @@ -104,13 +105,13 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { in = dockerCli.In() } - if opts.proxy && !c.Config.Tty { + if opts.Proxy && !c.Config.Tty { sigc := notifyAllSignals() - go ForwardAllSignals(ctx, dockerCli, opts.container, sigc) + go ForwardAllSignals(ctx, dockerCli, opts.Container, sigc) defer signal.StopCatch(sigc) } - resp, errAttach := apiClient.ContainerAttach(ctx, opts.container, options) + resp, errAttach := apiClient.ContainerAttach(ctx, opts.Container, options) if errAttach != nil { return errAttach } @@ -124,13 +125,13 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { // the container and not exit. // // Recheck the container's state to avoid attach block. - _, err = inspectContainerAndCheckState(ctx, apiClient, opts.container) + _, err = inspectContainerAndCheckState(ctx, apiClient, opts.Container) if err != nil { return err } if c.Config.Tty && dockerCli.Out().IsTerminal() { - resizeTTY(ctx, dockerCli, opts.container) + resizeTTY(ctx, dockerCli, opts.Container) } streamer := hijackedIOStreamer{ diff --git a/cli/command/container/exec.go b/cli/command/container/exec.go index 3160aaba83c6..311df7cb7b84 100644 --- a/cli/command/container/exec.go +++ b/cli/command/container/exec.go @@ -52,7 +52,7 @@ func NewExecCommand(dockerCli command.Cli) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { options.Container = args[0] options.Command = args[1:] - return RunExec(dockerCli, options) + return RunExec(context.Background(), dockerCli, options) }, ValidArgsFunction: completion.ContainerNames(dockerCli, false, func(container types.Container) bool { return container.State != "paused" @@ -96,13 +96,12 @@ func NewExecCommand(dockerCli command.Cli) *cobra.Command { } // RunExec executes an `exec` command -func RunExec(dockerCli command.Cli, options ExecOptions) error { +func RunExec(ctx context.Context, dockerCli command.Cli, options ExecOptions) error { execConfig, err := parseExec(options, dockerCli.ConfigFile()) if err != nil { return err } - ctx := context.Background() client := dockerCli.Client() // We need to check the tty _before_ we do the ContainerExecCreate, because diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go index 7b17d3ebf971..858e5bd1899c 100644 --- a/cli/command/container/exec_test.go +++ b/cli/command/container/exec_test.go @@ -195,7 +195,7 @@ func TestRunExec(t *testing.T) { t.Run(testcase.doc, func(t *testing.T) { cli := test.NewFakeCli(&testcase.client) - err := RunExec(cli, testcase.options) + err := RunExec(context.Background(), cli, testcase.options) if testcase.expectedError != "" { assert.ErrorContains(t, err, testcase.expectedError) } else { From a2ec50a461e140c46de1c6109861744a8bdb32e3 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Wed, 8 Nov 2023 14:32:55 +0100 Subject: [PATCH 2/2] make `container` an explicit, required parameter Signed-off-by: Nicolas De Loof --- cli/command/container/attach.go | 21 ++++++++++----------- cli/command/container/exec.go | 12 ++++++------ cli/command/container/exec_test.go | 5 ++--- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go index 9f07fe604115..f3e6da207279 100644 --- a/cli/command/container/attach.go +++ b/cli/command/container/attach.go @@ -22,8 +22,6 @@ type AttachOptions struct { NoStdin bool Proxy bool DetachKeys string - - Container string } func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, args string) (*types.ContainerJSON, error) { @@ -47,14 +45,15 @@ func inspectContainerAndCheckState(ctx context.Context, cli client.APIClient, ar // NewAttachCommand creates a new cobra.Command for `docker attach` func NewAttachCommand(dockerCli command.Cli) *cobra.Command { var opts AttachOptions + var container string cmd := &cobra.Command{ Use: "attach [OPTIONS] CONTAINER", Short: "Attach local standard input, output, and error streams to a running container", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - opts.Container = args[0] - return RunAttach(context.Background(), dockerCli, &opts) + container = args[0] + return RunAttach(context.Background(), dockerCli, container, &opts) }, Annotations: map[string]string{ "aliases": "docker container attach, docker attach", @@ -72,13 +71,13 @@ func NewAttachCommand(dockerCli command.Cli) *cobra.Command { } // RunAttach executes an `attach` command -func RunAttach(ctx context.Context, dockerCli command.Cli, opts *AttachOptions) error { +func RunAttach(ctx context.Context, dockerCli command.Cli, target string, opts *AttachOptions) error { apiClient := dockerCli.Client() // request channel to wait for client - resultC, errC := apiClient.ContainerWait(ctx, opts.Container, "") + resultC, errC := apiClient.ContainerWait(ctx, target, "") - c, err := inspectContainerAndCheckState(ctx, apiClient, opts.Container) + c, err := inspectContainerAndCheckState(ctx, apiClient, target) if err != nil { return err } @@ -107,11 +106,11 @@ func RunAttach(ctx context.Context, dockerCli command.Cli, opts *AttachOptions) if opts.Proxy && !c.Config.Tty { sigc := notifyAllSignals() - go ForwardAllSignals(ctx, dockerCli, opts.Container, sigc) + go ForwardAllSignals(ctx, dockerCli, target, sigc) defer signal.StopCatch(sigc) } - resp, errAttach := apiClient.ContainerAttach(ctx, opts.Container, options) + resp, errAttach := apiClient.ContainerAttach(ctx, target, options) if errAttach != nil { return errAttach } @@ -125,13 +124,13 @@ func RunAttach(ctx context.Context, dockerCli command.Cli, opts *AttachOptions) // the container and not exit. // // Recheck the container's state to avoid attach block. - _, err = inspectContainerAndCheckState(ctx, apiClient, opts.Container) + _, err = inspectContainerAndCheckState(ctx, apiClient, target) if err != nil { return err } if c.Config.Tty && dockerCli.Out().IsTerminal() { - resizeTTY(ctx, dockerCli, opts.Container) + resizeTTY(ctx, dockerCli, target) } streamer := hijackedIOStreamer{ diff --git a/cli/command/container/exec.go b/cli/command/container/exec.go index 311df7cb7b84..db89c68b386a 100644 --- a/cli/command/container/exec.go +++ b/cli/command/container/exec.go @@ -28,7 +28,6 @@ type ExecOptions struct { Privileged bool Env opts.ListOpts Workdir string - Container string Command []string EnvFile opts.ListOpts } @@ -44,15 +43,16 @@ func NewExecOptions() ExecOptions { // NewExecCommand creates a new cobra.Command for `docker exec` func NewExecCommand(dockerCli command.Cli) *cobra.Command { options := NewExecOptions() + var container string cmd := &cobra.Command{ Use: "exec [OPTIONS] CONTAINER COMMAND [ARG...]", Short: "Execute a command in a running container", Args: cli.RequiresMinArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - options.Container = args[0] + container = args[0] options.Command = args[1:] - return RunExec(context.Background(), dockerCli, options) + return RunExec(context.Background(), dockerCli, container, options) }, ValidArgsFunction: completion.ContainerNames(dockerCli, false, func(container types.Container) bool { return container.State != "paused" @@ -96,7 +96,7 @@ func NewExecCommand(dockerCli command.Cli) *cobra.Command { } // RunExec executes an `exec` command -func RunExec(ctx context.Context, dockerCli command.Cli, options ExecOptions) error { +func RunExec(ctx context.Context, dockerCli command.Cli, container string, options ExecOptions) error { execConfig, err := parseExec(options, dockerCli.ConfigFile()) if err != nil { return err @@ -108,7 +108,7 @@ func RunExec(ctx context.Context, dockerCli command.Cli, options ExecOptions) er // otherwise if we error out we will leak execIDs on the server (and // there's no easy way to clean those up). But also in order to make "not // exist" errors take precedence we do a dummy inspect first. - if _, err := client.ContainerInspect(ctx, options.Container); err != nil { + if _, err := client.ContainerInspect(ctx, container); err != nil { return err } if !execConfig.Detach { @@ -119,7 +119,7 @@ func RunExec(ctx context.Context, dockerCli command.Cli, options ExecOptions) er fillConsoleSize(execConfig, dockerCli) - response, err := client.ContainerExecCreate(ctx, options.Container, *execConfig) + response, err := client.ContainerExecCreate(ctx, container, *execConfig) if err != nil { return err } diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go index 858e5bd1899c..e1ff1e61b401 100644 --- a/cli/command/container/exec_test.go +++ b/cli/command/container/exec_test.go @@ -169,8 +169,7 @@ func TestRunExec(t *testing.T) { { doc: "successful detach", options: withDefaultOpts(ExecOptions{ - Container: "thecontainer", - Detach: true, + Detach: true, }), client: fakeClient{execCreateFunc: execCreateWithID}, }, @@ -195,7 +194,7 @@ func TestRunExec(t *testing.T) { t.Run(testcase.doc, func(t *testing.T) { cli := test.NewFakeCli(&testcase.client) - err := RunExec(context.Background(), cli, testcase.options) + err := RunExec(context.Background(), cli, "thecontainer", testcase.options) if testcase.expectedError != "" { assert.ErrorContains(t, err, testcase.expectedError) } else {