From 50da0ad9dfded3aa4e63668b0dba40d45b889c29 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 17 Jun 2025 12:53:44 +0200 Subject: [PATCH] cli/command/container: remove use of ExecOptions.Detach as intermediate This field was added in [moby@5130fe5d38837302e], which added it for use as intermediate struct when parsing CLI flags (through `runconfig.ParseExec`) in [moby@c786a8ee5e9db8f5f]. Commit [moby@9d9dff3d0d9e92adf] rewrote the CLI to use Cobra, and as part of this introduced a separate `execOptions` type in `api/client/container`, however the ExecOptions.Detach field was still used as intermediate field to store the flag's value. Given that the client doesn't use this field, let's remove its use to prevent giving the impression that it's used anywhere. [moby@5130fe5d38837302e]: https://github.com/docker/docker/commit/5130fe5d38837302e72bdc5e4bd1f5fa1df72c7f [moby@c786a8ee5e9db8f5f]: https://github.com/docker/docker/commit/c786a8ee5e9db8f5f609cf8721bd1e1513fb0043 [moby@9d9dff3d0d9e92adf]: https://github.com/docker/docker/commit/9d9dff3d0d9e92adf7c2e59f94c63766659d1d47 Signed-off-by: Sebastiaan van Stijn --- cli/command/container/exec.go | 7 +++---- cli/command/container/exec_test.go | 21 ++++++++++----------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/cli/command/container/exec.go b/cli/command/container/exec.go index 7d521700a5..b491e24a1b 100644 --- a/cli/command/container/exec.go +++ b/cli/command/container/exec.go @@ -99,7 +99,7 @@ func RunExec(ctx context.Context, dockerCLI command.Cli, containerIDorName strin if _, err := apiClient.ContainerInspect(ctx, containerIDorName); err != nil { return err } - if !execOptions.Detach { + if !options.Detach { if err := dockerCLI.In().CheckTty(execOptions.AttachStdin, execOptions.Tty); err != nil { return err } @@ -117,9 +117,9 @@ func RunExec(ctx context.Context, dockerCLI command.Cli, containerIDorName strin return errors.New("exec ID empty") } - if execOptions.Detach { + if options.Detach { return apiClient.ContainerExecStart(ctx, execID, container.ExecStartOptions{ - Detach: execOptions.Detach, + Detach: options.Detach, Tty: execOptions.Tty, ConsoleSize: execOptions.ConsoleSize, }) @@ -223,7 +223,6 @@ func parseExec(execOpts ExecOptions, configFile *configfile.ConfigFile) (*contai Privileged: execOpts.Privileged, Tty: execOpts.TTY, Cmd: execOpts.Command, - Detach: execOpts.Detach, WorkingDir: execOpts.Workdir, } diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go index 7ebc6b08b3..9690d091c4 100644 --- a/cli/command/container/exec_test.go +++ b/cli/command/container/exec_test.go @@ -5,6 +5,7 @@ import ( "errors" "io" "os" + "strconv" "testing" "github.com/docker/cli/cli" @@ -75,8 +76,7 @@ TWO=2 { options: withDefaultOpts(ExecOptions{Detach: true}), expected: container.ExecOptions{ - Detach: true, - Cmd: []string{"command"}, + Cmd: []string{"command"}, }, }, { @@ -86,9 +86,8 @@ TWO=2 Detach: true, }), expected: container.ExecOptions{ - Detach: true, - Tty: true, - Cmd: []string{"command"}, + Tty: true, + Cmd: []string{"command"}, }, }, { @@ -97,7 +96,6 @@ TWO=2 expected: container.ExecOptions{ Cmd: []string{"command"}, DetachKeys: "de", - Detach: true, }, }, { @@ -109,7 +107,6 @@ TWO=2 expected: container.ExecOptions{ Cmd: []string{"command"}, DetachKeys: "ab", - Detach: true, }, }, { @@ -141,10 +138,12 @@ TWO=2 }, } - for _, testcase := range testcases { - execConfig, err := parseExec(testcase.options, &testcase.configFile) - assert.NilError(t, err) - assert.Check(t, is.DeepEqual(testcase.expected, *execConfig)) + for i, testcase := range testcases { + t.Run("test "+strconv.Itoa(i+1), func(t *testing.T) { + execConfig, err := parseExec(testcase.options, &testcase.configFile) + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(testcase.expected, *execConfig)) + }) } }