From 22cd418967f09179520dc61f49fa3697bea0daf8 Mon Sep 17 00:00:00 2001 From: Zander Mackie Date: Mon, 5 Nov 2018 08:12:22 -0500 Subject: [PATCH] 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