Merge pull request #489 from dnephin/fix-container-run

Fix crash in container run after pulling an image

cli/command/container/client_test.go

@@ -1,7 +1,11 @@
 package container
 
 import (
+	"io"
+
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/client"
 	"golang.org/x/net/context"
 )
@@ -11,6 +15,9 @@ type fakeClient struct {
 	inspectFunc         func(string) (types.ContainerJSON, error)
 	execInspectFunc     func(execID string) (types.ContainerExecInspect, error)
 	execCreateFunc      func(container string, config types.ExecConfig) (types.IDResponse, error)
+	createContainerFunc func(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, containerName string) (container.ContainerCreateCreatedBody, error)
+	imageCreateFunc     func(parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error)
+	infoFunc            func() (types.Info, error)
 }
 
 func (f *fakeClient) ContainerInspect(_ context.Context, containerID string) (types.ContainerJSON, error) {
@@ -37,3 +44,30 @@ func (f *fakeClient) ContainerExecInspect(_ context.Context, execID string) (typ
 func (f *fakeClient) ContainerExecStart(ctx context.Context, execID string, config types.ExecStartCheck) error {
 	return nil
 }
+
+func (f *fakeClient) ContainerCreate(
+	_ context.Context,
+	config *container.Config,
+	hostConfig *container.HostConfig,
+	networkingConfig *network.NetworkingConfig,
+	containerName string,
+) (container.ContainerCreateCreatedBody, error) {
+	if f.createContainerFunc != nil {
+		return f.createContainerFunc(config, hostConfig, networkingConfig, containerName)
+	}
+	return container.ContainerCreateCreatedBody{}, nil
+}
+
+func (f *fakeClient) ImageCreate(ctx context.Context, parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) {
+	if f.imageCreateFunc != nil {
+		return f.imageCreateFunc(parentReference, options)
+	}
+	return nil, nil
+}
+
+func (f *fakeClient) Info(_ context.Context) (types.Info, error) {
+	if f.infoFunc != nil {
+		return f.infoFunc()
+	}
+	return types.Info{}, nil
+}

cli/command/container/create.go

@@ -198,7 +198,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig
 			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, stderr); err != nil {
+			if err := pullImage(ctx, dockerCli, config.Image, stderr); err != nil {
 				return nil, err
 			}
 			if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil {
@@ -212,9 +212,10 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig
 			if retryErr != nil {
 				return nil, retryErr
 			}
-		}
+		} else {
 			return nil, err
 		}
+	}
 
 	for _, warning := range response.Warnings {
 		fmt.Fprintf(stderr, "WARNING: %s\n", warning)

cli/command/container/create_test.go

@@ -1,13 +1,20 @@
 package container
 
 import (
+	"context"
+	"io"
+	"io/ioutil"
 	"os"
+	"strings"
 	"testing"
 
-	"io/ioutil"
+	"github.com/docker/cli/internal/test"
-
 	"github.com/docker/cli/internal/test/testutil"
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/network"
 	"github.com/gotestyourself/gotestyourself/fs"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -62,3 +69,52 @@ func TestCIDFileCloseWithWrite(t *testing.T) {
 	_, err = os.Stat(path)
 	require.NoError(t, err)
 }
+
+func TestCreateContainerPullsImageIfMissing(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, "name")
+	require.NoError(t, err)
+	expected := container.ContainerCreateCreatedBody{ID: containerID}
+	assert.Equal(t, expected, *body)
+	stderr := cli.ErrBuffer().String()
+	assert.Contains(t, stderr, "Unable to find image 'does-not-exist-locally:latest' locally")
+}
+
+type fakeNotFound struct{}
+
+func (f fakeNotFound) NotFound() bool { return true }
+func (f fakeNotFound) Error() string  { return "error fake not found" }

cli/command/trust.go

@@ -12,6 +12,10 @@ var (
 	untrusted bool
 )
 
+func init() {
+	untrusted = !getDefaultTrustState()
+}
+
 // AddTrustVerificationFlags adds content trust flags to the provided flagset
 func AddTrustVerificationFlags(fs *pflag.FlagSet) {
 	trusted := getDefaultTrustState()