From 22cd418967f09179520dc61f49fa3697bea0daf8 Mon Sep 17 00:00:00 2001 From: Zander Mackie Date: Mon, 5 Nov 2018 08:12:22 -0500 Subject: [PATCH 1/6] Adds flag modifying pull behavior for running and creating containers - Follows the proposal on issue [#34394](https://github.com/moby/moby/issues/34394) - Maintains current behavior as default (Pull image if missing) - Adds tristate flag allowing modification (PullMissing, PullAlways, PullNever) Signed-off-by: Zander Mackie --- cli/command/container/create.go | 75 +++++++++++++++++++++------- cli/command/container/create_test.go | 47 +++++++++++++++++ cli/command/container/run.go | 2 + 3 files changed, 106 insertions(+), 18 deletions(-) diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 868e4fb4ba..22e7470401 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 bahevior) + 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 @@ -213,31 +224,59 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig } //create the container - response, err := dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) + var response container.ContainerCreateCreatedBody + if opts.pull == PullImageMissing { // Pull image only if it does not exist locally. Default. + 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)) + //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)) - // 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 { - return nil, err - } - if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil { - if err := image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef); err != nil { + // 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 { 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 { + return nil, retryErr + } + } else { + return nil, err } - // Retry - var retryErr error - response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) - if retryErr != nil { - return nil, retryErr - } - } else { + } + + } else if opts.pull == PullImageAlways { // Always try and pull the image. + if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); 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 + } + } + response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) + if err != nil { + return nil, err + } + + } else if opts.pull == PullImageNever { // Never try and pull the image + response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) + + if err != nil { + if apiclient.IsErrNotFound(err) && namedRef != nil { + fmt.Fprintf(stderr, "Unable to find image '%s' locally\nWill not pull due to '%s'", reference.FamiliarString(namedRef), opts.pull) + } + } + } else { // We got something weird + return nil, errors.Errorf("Unknown pull option : %s", opts.pull) } for _, warning := range response.Warnings { diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 29912d44dc..4db851e8fc 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -116,6 +116,7 @@ func TestCreateContainerPullsImageIfMissing(t *testing.T) { name: "name", platform: runtime.GOOS, untrusted: true, + pull: PullImageMissing, }) assert.NilError(t, err) expected := container.ContainerCreateCreatedBody{ID: containerID} @@ -124,6 +125,52 @@ func TestCreateContainerPullsImageIfMissing(t *testing.T) { assert.Check(t, is.Contains(stderr, "Unable to find image 'does-not-exist-locally:latest' locally")) } +func TestCreateContainerNeverPullsImage(t *testing.T) { + imageName := "does-not-exist-locally" + responseCounter := 0 + + 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{} + 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, + pull: PullImageNever, + }) + assert.NilError(t, err) + expected := container.ContainerCreateCreatedBody{} + 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")) +} + 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 From a06b5db5942770e790f47f56a38c34cd78ae51c2 Mon Sep 17 00:00:00 2001 From: Ravi Shekhar Jethani Date: Tue, 20 Nov 2018 06:54:27 -0500 Subject: [PATCH 2/6] Update cli/command/container/create.go Co-Authored-By: Zanadar Signed-off-by: Zander Mackie --- cli/command/container/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 22e7470401..a41c92d7ae 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -25,7 +25,7 @@ import ( // Pull constants const ( PullImageAlways = "always" - PullImageMissing = "missing" // Default (matches previous bahevior) + PullImageMissing = "missing" // Default (matches previous behavior) PullImageNever = "never" ) From 965664d89bbe0d09028381039df8902c1787dbe8 Mon Sep 17 00:00:00 2001 From: Zander Mackie Date: Sat, 16 Feb 2019 18:41:56 -0500 Subject: [PATCH 3/6] Improve flow pull behavior before container creation. - Also improve test coverage Signed-off-by: Zander Mackie --- cli/command/container/create.go | 68 +++++++++++----------------- cli/command/container/create_test.go | 49 ++++++++++++++++++-- 2 files changed, 71 insertions(+), 46 deletions(-) diff --git a/cli/command/container/create.go b/cli/command/container/create.go index a41c92d7ae..e2043425f7 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -223,37 +223,10 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig } } - //create the container + //create the container, pulling the image (or not) based on opts.pull var response container.ContainerCreateCreatedBody - if opts.pull == PullImageMissing { // Pull image only if it does not exist locally. Default. - 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)) - - // 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 { - 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 { - return nil, retryErr - } - } else { - return nil, err - } - } - - } else if opts.pull == PullImageAlways { // Always try and pull the image. + if opts.pull == PullImageAlways { if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil { return nil, err } @@ -262,21 +235,32 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig return nil, err } } - response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) - if err != nil { + } + + response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) + + if err != nil { + // 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 { + fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef)) + // 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 { + 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 + } + } + + var retryErr error + response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) + if retryErr != nil { + return nil, retryErr + } + } else { return nil, err } - - } else if opts.pull == PullImageNever { // Never try and pull the image - response, err = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) - - if err != nil { - if apiclient.IsErrNotFound(err) && namedRef != nil { - fmt.Fprintf(stderr, "Unable to find image '%s' locally\nWill not pull due to '%s'", reference.FamiliarString(namedRef), opts.pull) - } - } - } else { // We got something weird - return nil, errors.Errorf("Unknown pull option : %s", opts.pull) } for _, warning := range response.Warnings { diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 4db851e8fc..539124f78f 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -158,17 +158,58 @@ func TestCreateContainerNeverPullsImage(t *testing.T) { }, HostConfig: &container.HostConfig{}, } - body, err := createContainer(context.Background(), cli, config, &createOptions{ + _, err := createContainer(context.Background(), cli, config, &createOptions{ name: "name", platform: runtime.GOOS, untrusted: true, pull: PullImageNever, }) + assert.ErrorContains(t, err, "fake not found") +} + +func TestCreateContainerAlwaysPullsImage(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{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, + pull: PullImageAlways, + }) assert.NilError(t, err) - expected := container.ContainerCreateCreatedBody{} + 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")) } func TestNewCreateCommandWithContentTrustErrors(t *testing.T) { From ec56136d61c30b5be86f3e47e831400f1be6f4b6 Mon Sep 17 00:00:00 2001 From: Zander Mackie Date: Wed, 20 Feb 2019 07:12:24 -0500 Subject: [PATCH 4/6] Use closure for common pulling and tagging bevior in container creation Signed-off-by: Zander Mackie --- cli/command/container/create.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/cli/command/container/create.go b/cli/command/container/create.go index e2043425f7..0141775b5a 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -223,35 +223,31 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig } } - //create the container, pulling the image (or not) based on opts.pull - var response container.ContainerCreateCreatedBody - - if opts.pull == PullImageAlways { + pullAndTagImage := func() error { if err := pullImage(ctx, dockerCli, config.Image, opts.platform, stderr); err != nil { - return nil, err + return err } if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil { - if err := image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef); err != nil { - return nil, err - } + 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) - + response, err := dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) if err != nil { // 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 { - fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef)) // 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 - } - } var retryErr error response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, opts.name) From ffba7659ccd877234fdb4b99d185165c38f117ec Mon Sep 17 00:00:00 2001 From: Zander Mackie Date: Fri, 1 Mar 2019 07:00:25 -0500 Subject: [PATCH 5/6] Improve testing of never pull and always pull scenarios Signed-off-by: Zander Mackie --- cli/command/container/create_test.go | 38 ++++++++++++++++++---------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 539124f78f..ebc217f512 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -128,6 +128,7 @@ func TestCreateContainerPullsImageIfMissing(t *testing.T) { func TestCreateContainerNeverPullsImage(t *testing.T) { imageName := "does-not-exist-locally" responseCounter := 0 + pullCounter := 0 client := &fakeClient{ createContainerFunc: func( @@ -145,7 +146,13 @@ func TestCreateContainerNeverPullsImage(t *testing.T) { } }, imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { - return ioutil.NopCloser(strings.NewReader("")), nil + defer func() { pullCounter++ }() + switch pullCounter { + case 0: + return ioutil.NopCloser(strings.NewReader("")), nil + default: + return nil, errors.New("unexpected pull") + } }, infoFunc: func() (types.Info, error) { return types.Info{IndexServerAddress: "http://indexserver"}, nil @@ -169,7 +176,9 @@ func TestCreateContainerNeverPullsImage(t *testing.T) { func TestCreateContainerAlwaysPullsImage(t *testing.T) { imageName := "does-not-exist-locally" + pullTries := 7 responseCounter := 0 + pullCounter := 0 containerID := "abcdef" client := &fakeClient{ @@ -181,13 +190,12 @@ func TestCreateContainerAlwaysPullsImage(t *testing.T) { ) (container.ContainerCreateCreatedBody, error) { defer func() { responseCounter++ }() switch responseCounter { - case 0: - return container.ContainerCreateCreatedBody{ID: containerID}, nil default: - return container.ContainerCreateCreatedBody{}, errors.New("unexpected") + 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) { @@ -201,15 +209,19 @@ func TestCreateContainerAlwaysPullsImage(t *testing.T) { }, HostConfig: &container.HostConfig{}, } - body, err := createContainer(context.Background(), cli, config, &createOptions{ - name: "name", - platform: runtime.GOOS, - untrusted: true, - pull: PullImageAlways, - }) - assert.NilError(t, err) - expected := container.ContainerCreateCreatedBody{ID: containerID} - assert.Check(t, is.DeepEqual(expected, *body)) + for i := 0; i < pullTries; i++ { + body, err := createContainer(context.Background(), cli, config, &createOptions{ + name: "name", + platform: runtime.GOOS, + untrusted: true, + pull: PullImageAlways, + }) + assert.NilError(t, err) + expected := container.ContainerCreateCreatedBody{ID: containerID} + assert.Check(t, is.DeepEqual(expected, *body)) + } + + assert.Check(t, is.Equal(responseCounter, pullCounter)) } func TestNewCreateCommandWithContentTrustErrors(t *testing.T) { From 483c53ad9df07917be247ff1093ce7858f588791 Mon Sep 17 00:00:00 2001 From: Zander Mackie Date: Thu, 7 Mar 2019 07:34:26 -0500 Subject: [PATCH 6/6] Use single table for all ContainerImagePullPolicy tests - Cleans up assertions - Centralizes and simplifies handler functions Signed-off-by: Zander Mackie --- cli/command/container/create_test.go | 189 ++++++++------------------- 1 file changed, 58 insertions(+), 131 deletions(-) diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index ebc217f512..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,154 +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, - pull: PullImageMissing, - }) - 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")) -} -func TestCreateContainerNeverPullsImage(t *testing.T) { - imageName := "does-not-exist-locally" - responseCounter := 0 - pullCounter := 0 - - 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{} - default: - return container.ContainerCreateCreatedBody{}, errors.New("unexpected") - } + 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", }, - imageCreateFunc: func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { - defer func() { pullCounter++ }() - switch pullCounter { - case 0: + } + 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 - default: - return nil, errors.New("unexpected pull") - } - }, - 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{}, - } - _, err := createContainer(context.Background(), cli, config, &createOptions{ - name: "name", - platform: runtime.GOOS, - untrusted: true, - pull: PullImageNever, - }) - assert.ErrorContains(t, err, "fake not found") -} - -func TestCreateContainerAlwaysPullsImage(t *testing.T) { - imageName := "does-not-exist-locally" - pullTries := 7 - responseCounter := 0 - pullCounter := 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 { - 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) - config := &containerConfig{ - Config: &container.Config{ - Image: imageName, - }, - HostConfig: &container.HostConfig{}, - } - for i := 0; i < pullTries; i++ { + }, + 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: PullImageAlways, + pull: c.PullPolicy, }) - assert.NilError(t, err) - expected := container.ContainerCreateCreatedBody{ID: containerID} - assert.Check(t, is.DeepEqual(expected, *body)) + + 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)) } - - assert.Check(t, is.Equal(responseCounter, pullCounter)) } - func TestNewCreateCommandWithContentTrustErrors(t *testing.T) { testCases := []struct { name string