From 23e26f40feb71d6c45c495e6fd9f0010c601cac9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 8 Jun 2023 16:46:15 +0200 Subject: [PATCH] cli/command/container: createContainer(): return container-ID This function returned the whole response, but we already handled the warnings included in the response as part of the function. All consumers of this function only used the container-ID, so let's simplify and return just that (it's a non-exported func, so we can change the signature again if we really need it). Signed-off-by: Sebastiaan van Stijn --- cli/command/container/create.go | 26 +++++++++++++------------- cli/command/container/create_test.go | 10 +++++----- cli/command/container/run.go | 16 ++++++++-------- 3 files changed, 26 insertions(+), 26 deletions(-) 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 be7c039eb1..7de0dc6ddb 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -93,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, @@ -142,7 +142,7 @@ func TestCreateContainerImagePullPolicy(t *testing.T) { }, } fakeCLI := test.NewFakeCli(client) - body, err := createContainer(context.Background(), fakeCLI, config, &createOptions{ + id, err := createContainer(context.Background(), fakeCLI, config, &createOptions{ name: "name", platform: runtime.GOOS, untrusted: true, @@ -153,7 +153,7 @@ func TestCreateContainerImagePullPolicy(t *testing.T) { assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg)) } else { assert.Check(t, err) - assert.Check(t, is.DeepEqual(tc.ExpectedBody, *body)) + assert.Check(t, is.Equal(tc.ExpectedID, id)) } assert.Check(t, is.Equal(tc.ExpectedPulls, pullCounter)) diff --git a/cli/command/container/run.go b/cli/command/container/run.go index c7e680e763..cfb7a7fb75 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,7 +164,7 @@ 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 @@ -173,17 +173,17 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio dockerCli.ConfigFile().DetachKeys = opts.detachKeys } - closeFn, err := attachContainer(ctx, dockerCli, &errCh, config, createResponse.ID) + closeFn, err := attachContainer(ctx, dockerCli, &errCh, config, containerID) 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 +201,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) } }