Skip to content

Commit 50da0ad

Browse files
committed
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]: moby/moby@5130fe5 [moby@c786a8ee5e9db8f5f]: moby/moby@c786a8e [moby@9d9dff3d0d9e92adf]: moby/moby@9d9dff3 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent dbb5872 commit 50da0ad

2 files changed

Lines changed: 13 additions & 15 deletions

File tree

cli/command/container/exec.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func RunExec(ctx context.Context, dockerCLI command.Cli, containerIDorName strin
9999
if _, err := apiClient.ContainerInspect(ctx, containerIDorName); err != nil {
100100
return err
101101
}
102-
if !execOptions.Detach {
102+
if !options.Detach {
103103
if err := dockerCLI.In().CheckTty(execOptions.AttachStdin, execOptions.Tty); err != nil {
104104
return err
105105
}
@@ -117,9 +117,9 @@ func RunExec(ctx context.Context, dockerCLI command.Cli, containerIDorName strin
117117
return errors.New("exec ID empty")
118118
}
119119

120-
if execOptions.Detach {
120+
if options.Detach {
121121
return apiClient.ContainerExecStart(ctx, execID, container.ExecStartOptions{
122-
Detach: execOptions.Detach,
122+
Detach: options.Detach,
123123
Tty: execOptions.Tty,
124124
ConsoleSize: execOptions.ConsoleSize,
125125
})
@@ -223,7 +223,6 @@ func parseExec(execOpts ExecOptions, configFile *configfile.ConfigFile) (*contai
223223
Privileged: execOpts.Privileged,
224224
Tty: execOpts.TTY,
225225
Cmd: execOpts.Command,
226-
Detach: execOpts.Detach,
227226
WorkingDir: execOpts.Workdir,
228227
}
229228

cli/command/container/exec_test.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"io"
77
"os"
8+
"strconv"
89
"testing"
910

1011
"github.com/docker/cli/cli"
@@ -75,8 +76,7 @@ TWO=2
7576
{
7677
options: withDefaultOpts(ExecOptions{Detach: true}),
7778
expected: container.ExecOptions{
78-
Detach: true,
79-
Cmd: []string{"command"},
79+
Cmd: []string{"command"},
8080
},
8181
},
8282
{
@@ -86,9 +86,8 @@ TWO=2
8686
Detach: true,
8787
}),
8888
expected: container.ExecOptions{
89-
Detach: true,
90-
Tty: true,
91-
Cmd: []string{"command"},
89+
Tty: true,
90+
Cmd: []string{"command"},
9291
},
9392
},
9493
{
@@ -97,7 +96,6 @@ TWO=2
9796
expected: container.ExecOptions{
9897
Cmd: []string{"command"},
9998
DetachKeys: "de",
100-
Detach: true,
10199
},
102100
},
103101
{
@@ -109,7 +107,6 @@ TWO=2
109107
expected: container.ExecOptions{
110108
Cmd: []string{"command"},
111109
DetachKeys: "ab",
112-
Detach: true,
113110
},
114111
},
115112
{
@@ -141,10 +138,12 @@ TWO=2
141138
},
142139
}
143140

144-
for _, testcase := range testcases {
145-
execConfig, err := parseExec(testcase.options, &testcase.configFile)
146-
assert.NilError(t, err)
147-
assert.Check(t, is.DeepEqual(testcase.expected, *execConfig))
141+
for i, testcase := range testcases {
142+
t.Run("test "+strconv.Itoa(i+1), func(t *testing.T) {
143+
execConfig, err := parseExec(testcase.options, &testcase.configFile)
144+
assert.NilError(t, err)
145+
assert.Check(t, is.DeepEqual(testcase.expected, *execConfig))
146+
})
148147
}
149148
}
150149

0 commit comments

Comments
 (0)