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 <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2023-06-08 16:46:15 +02:00
parent c2c6fbe23c
commit 23e26f40fe
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
3 changed files with 26 additions and 26 deletions

View File

@ -104,11 +104,11 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, options *createOptio
reportError(dockerCli.Err(), "create", err.Error(), true) reportError(dockerCli.Err(), "create", err.Error(), true)
return cli.StatusError{StatusCode: 125} return cli.StatusError{StatusCode: 125}
} }
response, err := createContainer(context.Background(), dockerCli, containerCfg, options) id, err := createContainer(context.Background(), dockerCli, containerCfg, options)
if err != nil { if err != nil {
return err return err
} }
fmt.Fprintln(dockerCli.Out(), response.ID) _, _ = fmt.Fprintln(dockerCli.Out(), id)
return nil return nil
} }
@ -185,7 +185,7 @@ func newCIDFile(path string) (*cidFile, error) {
} }
//nolint:gocyclo //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 config := containerCfg.Config
hostConfig := containerCfg.HostConfig hostConfig := containerCfg.HostConfig
networkingConfig := containerCfg.NetworkingConfig networkingConfig := containerCfg.NetworkingConfig
@ -200,13 +200,13 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
containerIDFile, err := newCIDFile(hostConfig.ContainerIDFile) containerIDFile, err := newCIDFile(hostConfig.ContainerIDFile)
if err != nil { if err != nil {
return nil, err return "", err
} }
defer containerIDFile.Close() defer containerIDFile.Close()
ref, err := reference.ParseAnyReference(config.Image) ref, err := reference.ParseAnyReference(config.Image)
if err != nil { if err != nil {
return nil, err return "", err
} }
if named, ok := ref.(reference.Named); ok { if named, ok := ref.(reference.Named); ok {
namedRef = reference.TagNameOnly(named) namedRef = reference.TagNameOnly(named)
@ -215,7 +215,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
var err error var err error
trustedRef, err = image.TrustedReference(ctx, dockerCli, taggedRef) trustedRef, err = image.TrustedReference(ctx, dockerCli, taggedRef)
if err != nil { if err != nil {
return nil, err return "", err
} }
config.Image = reference.FamiliarString(trustedRef) 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") { if opts.platform != "" && versions.GreaterThanOrEqualTo(dockerCli.Client().ClientVersion(), "1.41") {
p, err := platforms.Parse(opts.platform) p, err := platforms.Parse(opts.platform)
if err != nil { if err != nil {
return nil, errors.Wrap(err, "error parsing specified platform") return "", errors.Wrap(err, "error parsing specified platform")
} }
platform = &p platform = &p
} }
if opts.pull == PullImageAlways { if opts.pull == PullImageAlways {
if err := pullAndTagImage(); err != nil { 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 { if err := pullAndTagImage(); err != nil {
return nil, err return "", err
} }
var retryErr error var retryErr error
response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, platform, opts.name) response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, platform, opts.name)
if retryErr != nil { if retryErr != nil {
return nil, retryErr return "", retryErr
} }
} else { } else {
return nil, err return "", err
} }
} }
for _, w := range response.Warnings { 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) err = containerIDFile.Write(response.ID)
return &response, err return response.ID, err
} }
func warnOnOomKillDisable(hostConfig container.HostConfig, stderr io.Writer) { func warnOnOomKillDisable(hostConfig container.HostConfig, stderr io.Writer) {

View File

@ -93,18 +93,18 @@ func TestCreateContainerImagePullPolicy(t *testing.T) {
cases := []struct { cases := []struct {
PullPolicy string PullPolicy string
ExpectedPulls int ExpectedPulls int
ExpectedBody container.CreateResponse ExpectedID string
ExpectedErrMsg string ExpectedErrMsg string
ResponseCounter int ResponseCounter int
}{ }{
{ {
PullPolicy: PullImageMissing, PullPolicy: PullImageMissing,
ExpectedPulls: 1, ExpectedPulls: 1,
ExpectedBody: container.CreateResponse{ID: containerID}, ExpectedID: containerID,
}, { }, {
PullPolicy: PullImageAlways, PullPolicy: PullImageAlways,
ExpectedPulls: 1, ExpectedPulls: 1,
ExpectedBody: container.CreateResponse{ID: containerID}, ExpectedID: containerID,
ResponseCounter: 1, // This lets us return a container on the first pull ResponseCounter: 1, // This lets us return a container on the first pull
}, { }, {
PullPolicy: PullImageNever, PullPolicy: PullImageNever,
@ -142,7 +142,7 @@ func TestCreateContainerImagePullPolicy(t *testing.T) {
}, },
} }
fakeCLI := test.NewFakeCli(client) fakeCLI := test.NewFakeCli(client)
body, err := createContainer(context.Background(), fakeCLI, config, &createOptions{ id, err := createContainer(context.Background(), fakeCLI, config, &createOptions{
name: "name", name: "name",
platform: runtime.GOOS, platform: runtime.GOOS,
untrusted: true, untrusted: true,
@ -153,7 +153,7 @@ func TestCreateContainerImagePullPolicy(t *testing.T) {
assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg)) assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg))
} else { } else {
assert.Check(t, err) 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)) assert.Check(t, is.Equal(tc.ExpectedPulls, pullCounter))

View File

@ -144,14 +144,14 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio
ctx, cancelFun := context.WithCancel(context.Background()) ctx, cancelFun := context.WithCancel(context.Background())
defer cancelFun() defer cancelFun()
createResponse, err := createContainer(ctx, dockerCli, containerCfg, &opts.createOptions) containerID, err := createContainer(ctx, dockerCli, containerCfg, &opts.createOptions)
if err != nil { if err != nil {
reportError(stderr, "run", err.Error(), true) reportError(stderr, "run", err.Error(), true)
return runStartContainerErr(err) return runStartContainerErr(err)
} }
if opts.sigProxy { if opts.sigProxy {
sigc := notifyAllSignals() sigc := notifyAllSignals()
go ForwardAllSignals(ctx, dockerCli, createResponse.ID, sigc) go ForwardAllSignals(ctx, dockerCli, containerID, sigc)
defer signal.StopCatch(sigc) defer signal.StopCatch(sigc)
} }
@ -164,7 +164,7 @@ func runContainer(dockerCli command.Cli, opts *runOptions, copts *containerOptio
waitDisplayID = make(chan struct{}) waitDisplayID = make(chan struct{})
go func() { go func() {
defer close(waitDisplayID) defer close(waitDisplayID)
fmt.Fprintln(stdout, createResponse.ID) _, _ = fmt.Fprintln(stdout, containerID)
}() }()
} }
attach := config.AttachStdin || config.AttachStdout || config.AttachStderr 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 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 { if err != nil {
return err return err
} }
defer closeFn() defer closeFn()
} }
statusChan := waitExitOrRemoved(ctx, dockerCli, createResponse.ID, copts.autoRemove) statusChan := waitExitOrRemoved(ctx, dockerCli, containerID, copts.autoRemove)
// start the container // 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 // If we have hijackedIOStreamer, we should notify
// hijackedIOStreamer we are going to exit and wait // hijackedIOStreamer we are going to exit and wait
// to avoid the terminal are not restored. // 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 (config.AttachStdin || config.AttachStdout || config.AttachStderr) && config.Tty && dockerCli.Out().IsTerminal() {
if err := MonitorTtySize(ctx, dockerCli, createResponse.ID, false); err != nil { if err := MonitorTtySize(ctx, dockerCli, containerID, false); err != nil {
fmt.Fprintln(stderr, "Error monitoring TTY size:", err) _, _ = fmt.Fprintln(stderr, "Error monitoring TTY size:", err)
} }
} }