Merge pull request #1498 from Zanadar/34394-run-pull-flag

Adds flag modifying pull behavior for running and creating containers

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 {

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

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