diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go index bd17696b80..4785434c91 100644 --- a/cli/command/container/attach.go +++ b/cli/command/container/attach.go @@ -72,12 +72,12 @@ func NewAttachCommand(dockerCli command.Cli) *cobra.Command { func runAttach(dockerCli command.Cli, opts *attachOptions) error { ctx := context.Background() - client := dockerCli.Client() + apiClient := dockerCli.Client() // request channel to wait for client - resultC, errC := client.ContainerWait(ctx, opts.container, "") + resultC, errC := apiClient.ContainerWait(ctx, opts.container, "") - c, err := inspectContainerAndCheckState(ctx, client, opts.container) + c, err := inspectContainerAndCheckState(ctx, apiClient, opts.container) if err != nil { return err } @@ -86,8 +86,9 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { return err } + detachKeys := dockerCli.ConfigFile().DetachKeys if opts.detachKeys != "" { - dockerCli.ConfigFile().DetachKeys = opts.detachKeys + detachKeys = opts.detachKeys } options := types.ContainerAttachOptions{ @@ -95,7 +96,7 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { Stdin: !opts.noStdin && c.Config.OpenStdin, Stdout: true, Stderr: true, - DetachKeys: dockerCli.ConfigFile().DetachKeys, + DetachKeys: detachKeys, } var in io.ReadCloser @@ -109,7 +110,7 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error { defer signal.StopCatch(sigc) } - resp, errAttach := client.ContainerAttach(ctx, opts.container, options) + resp, errAttach := apiClient.ContainerAttach(ctx, opts.container, options) if errAttach != nil { return errAttach } @@ -123,7 +124,7 @@ 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, client, opts.container) + _, err = inspectContainerAndCheckState(ctx, apiClient, opts.container) if err != nil { return err } diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 4758714d1a..1ad4ba6c52 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -104,11 +104,11 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, options *createOptio reportError(dockerCli.Err(), "create", err.Error(), true) return cli.StatusError{StatusCode: 125} } - response, err := createContainer(context.Background(), dockerCli, containerCfg, options) + id, err := createContainer(context.Background(), dockerCli, containerCfg, options) if err != nil { return err } - fmt.Fprintln(dockerCli.Out(), response.ID) + _, _ = fmt.Fprintln(dockerCli.Out(), id) return nil } @@ -185,7 +185,7 @@ func newCIDFile(path string) (*cidFile, error) { } //nolint:gocyclo -func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *containerConfig, opts *createOptions) (*container.CreateResponse, error) { +func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *containerConfig, opts *createOptions) (containerID string, err error) { config := containerCfg.Config hostConfig := containerCfg.HostConfig networkingConfig := containerCfg.NetworkingConfig @@ -200,13 +200,13 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c containerIDFile, err := newCIDFile(hostConfig.ContainerIDFile) if err != nil { - return nil, err + return "", err } defer containerIDFile.Close() ref, err := reference.ParseAnyReference(config.Image) if err != nil { - return nil, err + return "", err } if named, ok := ref.(reference.Named); ok { namedRef = reference.TagNameOnly(named) @@ -215,7 +215,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c var err error trustedRef, err = image.TrustedReference(ctx, dockerCli, taggedRef) if err != nil { - return nil, err + return "", err } config.Image = reference.FamiliarString(trustedRef) } @@ -239,14 +239,14 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c if opts.platform != "" && versions.GreaterThanOrEqualTo(dockerCli.Client().ClientVersion(), "1.41") { p, err := platforms.Parse(opts.platform) if err != nil { - return nil, errors.Wrap(err, "error parsing specified platform") + return "", errors.Wrap(err, "error parsing specified platform") } platform = &p } if opts.pull == PullImageAlways { if err := pullAndTagImage(); err != nil { - return nil, err + return "", err } } @@ -262,24 +262,24 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c } if err := pullAndTagImage(); err != nil { - return nil, err + return "", err } var retryErr error response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, platform, opts.name) if retryErr != nil { - return nil, retryErr + return "", retryErr } } else { - return nil, err + return "", err } } for _, w := range response.Warnings { - fmt.Fprintf(dockerCli.Err(), "WARNING: %s\n", w) + _, _ = fmt.Fprintf(dockerCli.Err(), "WARNING: %s\n", w) } err = containerIDFile.Write(response.ID) - return &response, err + return response.ID, err } func warnOnOomKillDisable(hostConfig container.HostConfig, stderr io.Writer) { diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 725b6c9326..7de0dc6ddb 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -79,8 +79,10 @@ func TestCIDFileCloseWithWrite(t *testing.T) { } func TestCreateContainerImagePullPolicy(t *testing.T) { - imageName := "does-not-exist-locally" - containerID := "abcdef" + const ( + imageName = "does-not-exist-locally" + containerID = "abcdef" + ) config := &containerConfig{ Config: &container.Config{ Image: imageName, @@ -91,18 +93,18 @@ func TestCreateContainerImagePullPolicy(t *testing.T) { cases := []struct { PullPolicy string ExpectedPulls int - ExpectedBody container.CreateResponse + ExpectedID string ExpectedErrMsg string ResponseCounter int }{ { PullPolicy: PullImageMissing, ExpectedPulls: 1, - ExpectedBody: container.CreateResponse{ID: containerID}, + ExpectedID: containerID, }, { PullPolicy: PullImageAlways, ExpectedPulls: 1, - ExpectedBody: container.CreateResponse{ID: containerID}, + ExpectedID: containerID, ResponseCounter: 1, // This lets us return a container on the first pull }, { PullPolicy: PullImageNever, @@ -110,50 +112,52 @@ func TestCreateContainerImagePullPolicy(t *testing.T) { ExpectedErrMsg: "error fake not found", }, } - for _, c := range cases { - c := c - pullCounter := 0 + for _, tc := range cases { + tc := tc + t.Run(tc.PullPolicy, func(t *testing.T) { + pullCounter := 0 - client := &fakeClient{ - createContainerFunc: func( - config *container.Config, - hostConfig *container.HostConfig, - networkingConfig *network.NetworkingConfig, - platform *specs.Platform, - containerName string, - ) (container.CreateResponse, error) { - defer func() { c.ResponseCounter++ }() - switch c.ResponseCounter { - case 0: - return container.CreateResponse{}, fakeNotFound{} - default: - return container.CreateResponse{ID: containerID}, nil - } - }, - imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { - defer func() { pullCounter++ }() - return io.NopCloser(strings.NewReader("")), nil - }, - infoFunc: func() (types.Info, error) { - return types.Info{IndexServerAddress: "https://indexserver.example.com"}, nil - }, - } - cli := test.NewFakeCli(client) - body, err := createContainer(context.Background(), cli, config, &createOptions{ - name: "name", - platform: runtime.GOOS, - untrusted: true, - pull: c.PullPolicy, + client := &fakeClient{ + createContainerFunc: func( + config *container.Config, + hostConfig *container.HostConfig, + networkingConfig *network.NetworkingConfig, + platform *specs.Platform, + containerName string, + ) (container.CreateResponse, error) { + defer func() { tc.ResponseCounter++ }() + switch tc.ResponseCounter { + case 0: + return container.CreateResponse{}, fakeNotFound{} + default: + return container.CreateResponse{ID: containerID}, nil + } + }, + imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { + defer func() { pullCounter++ }() + return io.NopCloser(strings.NewReader("")), nil + }, + infoFunc: func() (types.Info, error) { + return types.Info{IndexServerAddress: "https://indexserver.example.com"}, nil + }, + } + fakeCLI := test.NewFakeCli(client) + id, err := createContainer(context.Background(), fakeCLI, config, &createOptions{ + name: "name", + platform: runtime.GOOS, + untrusted: true, + pull: tc.PullPolicy, + }) + + if tc.ExpectedErrMsg != "" { + assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg)) + } else { + assert.Check(t, err) + assert.Check(t, is.Equal(tc.ExpectedID, id)) + } + + assert.Check(t, is.Equal(tc.ExpectedPulls, pullCounter)) }) - - if c.ExpectedErrMsg != "" { - assert.ErrorContains(t, err, c.ExpectedErrMsg) - } else { - assert.NilError(t, err) - assert.Check(t, is.DeepEqual(c.ExpectedBody, *body)) - } - - assert.Check(t, is.Equal(c.ExpectedPulls, pullCounter)) } } diff --git a/cli/command/container/run.go b/cli/command/container/run.go index c7e680e763..fe4969cd0b 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -144,14 +144,14 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio ctx, cancelFun := context.WithCancel(context.Background()) defer cancelFun() - createResponse, err := createContainer(ctx, dockerCli, containerCfg, &opts.createOptions) + containerID, err := createContainer(ctx, dockerCli, containerCfg, &opts.createOptions) if err != nil { reportError(stderr, "run", err.Error(), true) return runStartContainerErr(err) } if opts.sigProxy { sigc := notifyAllSignals() - go ForwardAllSignals(ctx, dockerCli, createResponse.ID, sigc) + go ForwardAllSignals(ctx, dockerCli, containerID, sigc) defer signal.StopCatch(sigc) } @@ -164,26 +164,33 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio waitDisplayID = make(chan struct{}) go func() { defer close(waitDisplayID) - fmt.Fprintln(stdout, createResponse.ID) + _, _ = fmt.Fprintln(stdout, containerID) }() } attach := config.AttachStdin || config.AttachStdout || config.AttachStderr if attach { + detachKeys := dockerCli.ConfigFile().DetachKeys if opts.detachKeys != "" { - dockerCli.ConfigFile().DetachKeys = opts.detachKeys + detachKeys = opts.detachKeys } - closeFn, err := attachContainer(ctx, dockerCli, &errCh, config, createResponse.ID) + closeFn, err := attachContainer(ctx, dockerCli, containerID, &errCh, config, types.ContainerAttachOptions{ + Stream: true, + Stdin: config.AttachStdin, + Stdout: config.AttachStdout, + Stderr: config.AttachStderr, + DetachKeys: detachKeys, + }) if err != nil { return err } defer closeFn() } - statusChan := waitExitOrRemoved(ctx, dockerCli, createResponse.ID, copts.autoRemove) + statusChan := waitExitOrRemoved(ctx, dockerCli, containerID, copts.autoRemove) // start the container - if err := client.ContainerStart(ctx, createResponse.ID, types.ContainerStartOptions{}); err != nil { + if err := client.ContainerStart(ctx, containerID, types.ContainerStartOptions{}); err != nil { // If we have hijackedIOStreamer, we should notify // hijackedIOStreamer we are going to exit and wait // to avoid the terminal are not restored. @@ -201,8 +208,8 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio } if (config.AttachStdin || config.AttachStdout || config.AttachStderr) && config.Tty && dockerCli.Out().IsTerminal() { - if err := MonitorTtySize(ctx, dockerCli, createResponse.ID, false); err != nil { - fmt.Fprintln(stderr, "Error monitoring TTY size:", err) + if err := MonitorTtySize(ctx, dockerCli, containerID, false); err != nil { + _, _ = fmt.Fprintln(stderr, "Error monitoring TTY size:", err) } } @@ -232,15 +239,7 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio return nil } -func attachContainer(ctx context.Context, dockerCli command.Cli, errCh *chan error, config *container.Config, containerID string) (func(), error) { - options := types.ContainerAttachOptions{ - Stream: true, - Stdin: config.AttachStdin, - Stdout: config.AttachStdout, - Stderr: config.AttachStderr, - DetachKeys: dockerCli.ConfigFile().DetachKeys, - } - +func attachContainer(ctx context.Context, dockerCli command.Cli, containerID string, errCh *chan error, config *container.Config, options types.ContainerAttachOptions) (func(), error) { resp, errAttach := dockerCli.Client().ContainerAttach(ctx, containerID, options) if errAttach != nil { return nil, errAttach @@ -250,13 +249,13 @@ func attachContainer(ctx context.Context, dockerCli command.Cli, errCh *chan err out, cerr io.Writer in io.ReadCloser ) - if config.AttachStdin { + if options.Stdin { in = dockerCli.In() } - if config.AttachStdout { + if options.Stdout { out = dockerCli.Out() } - if config.AttachStderr { + if options.Stderr { if config.Tty { cerr = dockerCli.Out() } else { diff --git a/cli/command/container/start.go b/cli/command/container/start.go index 9a8a083e6b..b50f6ba19a 100644 --- a/cli/command/container/start.go +++ b/cli/command/container/start.go @@ -94,8 +94,9 @@ func RunStart(dockerCli command.Cli, opts *StartOptions) error { defer signal.StopCatch(sigc) } + detachKeys := dockerCli.ConfigFile().DetachKeys if opts.DetachKeys != "" { - dockerCli.ConfigFile().DetachKeys = opts.DetachKeys + detachKeys = opts.DetachKeys } options := types.ContainerAttachOptions{ @@ -103,7 +104,7 @@ func RunStart(dockerCli command.Cli, opts *StartOptions) error { Stdin: opts.OpenStdin && c.Config.OpenStdin, Stdout: true, Stderr: true, - DetachKeys: dockerCli.ConfigFile().DetachKeys, + DetachKeys: detachKeys, } var in io.ReadCloser