diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 868e4fb4ba..0141775b5a 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -22,10 +22,18 @@ import ( "github.com/spf13/pflag" ) +// Pull constants +const ( + PullImageAlways = "always" + PullImageMissing = "missing" // Default (matches previous behavior) + PullImageNever = "never" +) + type createOptions struct { name string platform string untrusted bool + pull string // alway, missing, never } // NewCreateCommand creates a new cobra.Command for `docker create` @@ -50,6 +58,8 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command { flags.SetInterspersed(false) flags.StringVar(&opts.name, "name", "", "Assign a name to the container") + flags.StringVar(&opts.pull, "pull", PullImageMissing, + `Pull image before creating ("`+PullImageAlways+`"|"`+PullImageMissing+`"|"`+PullImageNever+`")`) // Add an explicit help that doesn't have a `-h` to prevent the conflict // with hostname @@ -175,6 +185,7 @@ func newCIDFile(path string) (*cidFile, error) { return &cidFile{path: path, file: f}, nil } +// nolint: gocyclo func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig *containerConfig, opts *createOptions) (*container.ContainerCreateCreatedBody, error) { config := containerConfig.Config hostConfig := containerConfig.HostConfig @@ -212,24 +223,32 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig } } - //create the container + pullAndTagImage := func() error { + if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil { + return err + } + if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil { + return image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef) + } + return nil + } + + if opts.pull == PullImageAlways { + if err := pullAndTagImage(); err != nil { + return nil, err + } + } + response, err := dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) - - //if image not found try to pull it if err != nil { - if apiclient.IsErrNotFound(err) && namedRef != nil { - fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef)) - + // Pull image if it does not exist locally and we have the PullImageMissing option. Default behavior. + if apiclient.IsErrNotFound(err) && namedRef != nil && opts.pull == PullImageMissing { // we don't want to write to stdout anything apart from container.ID - if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil { + fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef)) + if err := pullAndTagImage(); err != nil { return nil, err } - if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil { - if err := image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef); err != nil { - return nil, err - } - } - // Retry + var retryErr error response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) if retryErr != nil { diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 29912d44dc..fe2651ca47 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -18,7 +18,6 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" "github.com/google/go-cmp/cmp" - "github.com/pkg/errors" "gotest.tools/assert" is "gotest.tools/assert/cmp" "gotest.tools/fs" @@ -76,54 +75,82 @@ func TestCIDFileCloseWithWrite(t *testing.T) { assert.NilError(t, err) } -func TestCreateContainerPullsImageIfMissing(t *testing.T) { +func TestCreateContainerImagePullPolicy(t *testing.T) { imageName := "does-not-exist-locally" - responseCounter := 0 containerID := "abcdef" - - client := &fakeClient{ - createContainerFunc: func( - config *container.Config, - hostConfig *container.HostConfig, - networkingConfig *network.NetworkingConfig, - containerName string, - ) (container.ContainerCreateCreatedBody, error) { - defer func() { responseCounter++ }() - switch responseCounter { - case 0: - return container.ContainerCreateCreatedBody{}, fakeNotFound{} - case 1: - return container.ContainerCreateCreatedBody{ID: containerID}, nil - default: - return container.ContainerCreateCreatedBody{}, errors.New("unexpected") - } - }, - imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { - return ioutil.NopCloser(strings.NewReader("")), nil - }, - infoFunc: func() (types.Info, error) { - return types.Info{IndexServerAddress: "http://indexserver"}, nil - }, - } - cli := test.NewFakeCli(client) config := &containerConfig{ Config: &container.Config{ Image: imageName, }, HostConfig: &container.HostConfig{}, } - body, err := createContainer(context.Background(), cli, config, &createOptions{ - name: "name", - platform: runtime.GOOS, - untrusted: true, - }) - assert.NilError(t, err) - expected := container.ContainerCreateCreatedBody{ID: containerID} - assert.Check(t, is.DeepEqual(expected, *body)) - stderr := cli.ErrBuffer().String() - assert.Check(t, is.Contains(stderr, "Unable to find image 'does-not-exist-locally:latest' locally")) -} + cases := []struct { + PullPolicy string + ExpectedPulls int + ExpectedBody container.ContainerCreateCreatedBody + ExpectedErrMsg string + ResponseCounter int + }{ + { + PullPolicy: PullImageMissing, + ExpectedPulls: 1, + ExpectedBody: container.ContainerCreateCreatedBody{ID: containerID}, + }, { + PullPolicy: PullImageAlways, + ExpectedPulls: 1, + ExpectedBody: container.ContainerCreateCreatedBody{ID: containerID}, + ResponseCounter: 1, // This lets us return a container on the first pull + }, { + PullPolicy: PullImageNever, + ExpectedPulls: 0, + ExpectedErrMsg: "error fake not found", + }, + } + for _, c := range cases { + pullCounter := 0 + + client := &fakeClient{ + createContainerFunc: func( + config *container.Config, + hostConfig *container.HostConfig, + networkingConfig *network.NetworkingConfig, + containerName string, + ) (container.ContainerCreateCreatedBody, error) { + defer func() { c.ResponseCounter++ }() + switch c.ResponseCounter { + case 0: + return container.ContainerCreateCreatedBody{}, fakeNotFound{} + default: + return container.ContainerCreateCreatedBody{ID: containerID}, nil + } + }, + imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { + defer func() { pullCounter++ }() + return ioutil.NopCloser(strings.NewReader("")), nil + }, + infoFunc: func() (types.Info, error) { + return types.Info{IndexServerAddress: "http://indexserver"}, nil + }, + } + cli := test.NewFakeCli(client) + body, err := createContainer(context.Background(), cli, config, &createOptions{ + name: "name", + platform: runtime.GOOS, + untrusted: true, + pull: c.PullPolicy, + }) + + 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)) + } +} func TestNewCreateCommandWithContentTrustErrors(t *testing.T) { testCases := []struct { name string diff --git a/cli/command/container/run.go b/cli/command/container/run.go index 2ce3f3679c..93f84a711a 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -56,6 +56,8 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command { flags.BoolVar(&opts.sigProxy, "sig-proxy", true, "Proxy received signals to the process") flags.StringVar(&opts.name, "name", "", "Assign a name to the container") flags.StringVar(&opts.detachKeys, "detach-keys", "", "Override the key sequence for detaching a container") + flags.StringVar(&opts.createOptions.pull, "pull", PullImageMissing, + `Pull image before running ("`+PullImageAlways+`"|"`+PullImageMissing+`"|"`+PullImageNever+`")`) // Add an explicit help that doesn't have a `-h` to prevent the conflict // with hostname