builder: fix copy —from conflict with force pull

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

api/types/backend/build.go

@@ -7,6 +7,18 @@ import (
 	"github.com/docker/docker/pkg/streamformatter"
 )
 
+// PullOption defines different modes for accessing images
+type PullOption int
+
+const (
+	// PullOptionNoPull only returns local images
+	PullOptionNoPull PullOption = iota
+	// PullOptionForcePull always tries to pull a ref from the registry first
+	PullOptionForcePull
+	// PullOptionPreferLocal uses local image if it exists, otherwise pulls
+	PullOptionPreferLocal
+)
+
 // ProgressWriter is a data object to transport progress streams to the client
 type ProgressWriter struct {
 	Output             io.Writer
@@ -25,7 +37,7 @@ type BuildConfig struct {
 
 // GetImageAndLayerOptions are the options supported by GetImageAndReleasableLayer
 type GetImageAndLayerOptions struct {
-	ForcePull  bool
+	PullOption PullOption
 	AuthConfig map[string]types.AuthConfig
 	Output     io.Writer
 }

builder/dockerfile/dispatchers.go

@@ -196,6 +196,7 @@ func (b *Builder) getImageMount(fromFlag *Flag) (*imageMount, error) {
 		return nil, nil
 	}
 
+	var localOnly bool
 	imageRefOrID := fromFlag.Value
 	stage, err := b.buildStages.get(fromFlag.Value)
 	if err != nil {
@@ -203,8 +204,9 @@ func (b *Builder) getImageMount(fromFlag *Flag) (*imageMount, error) {
 	}
 	if stage != nil {
 		imageRefOrID = stage.ImageID()
+		localOnly = true
 	}
-	return b.imageSources.Get(imageRefOrID)
+	return b.imageSources.Get(imageRefOrID, localOnly)
 }
 
 // FROM imagename[:tag | @digest] [AS build-stage-name]
@@ -266,8 +268,10 @@ func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, err
 		return nil, err
 	}
 
+	var localOnly bool
 	if stage, ok := b.buildStages.getByName(name); ok {
 		name = stage.ImageID()
+		localOnly = true
 	}
 
 	// Windows cannot support a container with no base image.
@@ -277,7 +281,7 @@ func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, err
 		}
 		return scratchImage, nil
 	}
-	imageMount, err := b.imageSources.Get(name)
+	imageMount, err := b.imageSources.Get(name, localOnly)
 	if err != nil {
 		return nil, err
 	}

builder/dockerfile/imagecontext.go

@@ -86,21 +86,29 @@ func (s *buildStages) update(imageID string) {
 	s.sequence[len(s.sequence)-1].update(imageID)
 }
 
-type getAndMountFunc func(string) (builder.Image, builder.ReleaseableLayer, error)
+type getAndMountFunc func(string, bool) (builder.Image, builder.ReleaseableLayer, error)
 
 // imageSources mounts images and provides a cache for mounted images. It tracks
 // all images so they can be unmounted at the end of the build.
 type imageSources struct {
 	byImageID map[string]*imageMount
-	withoutID []*imageMount
+	mounts    []*imageMount
 	getImage  getAndMountFunc
 	cache     pathCache // TODO: remove
 }
 
 func newImageSources(ctx context.Context, options builderOptions) *imageSources {
-	getAndMount := func(idOrRef string) (builder.Image, builder.ReleaseableLayer, error) {
+	getAndMount := func(idOrRef string, localOnly bool) (builder.Image, builder.ReleaseableLayer, error) {
+		pullOption := backend.PullOptionNoPull
+		if !localOnly {
+			if options.Options.PullParent {
+				pullOption = backend.PullOptionForcePull
+			} else {
+				pullOption = backend.PullOptionPreferLocal
+			}
+		}
 		return options.Backend.GetImageAndReleasableLayer(ctx, idOrRef, backend.GetImageAndLayerOptions{
-			ForcePull:  options.Options.PullParent,
+			PullOption: pullOption,
 			AuthConfig: options.Options.AuthConfigs,
 			Output:     options.ProgressWriter.Output,
 		})
@@ -112,12 +120,12 @@ func newImageSources(ctx context.Context, options builderOptions) *imageSources
 	}
 }
 
-func (m *imageSources) Get(idOrRef string) (*imageMount, error) {
+func (m *imageSources) Get(idOrRef string, localOnly bool) (*imageMount, error) {
 	if im, ok := m.byImageID[idOrRef]; ok {
 		return im, nil
 	}
 
-	image, layer, err := m.getImage(idOrRef)
+	image, layer, err := m.getImage(idOrRef, localOnly)
 	if err != nil {
 		return nil, err
 	}
@@ -127,13 +135,7 @@ func (m *imageSources) Get(idOrRef string) (*imageMount, error) {
 }
 
 func (m *imageSources) Unmount() (retErr error) {
-	for _, im := range m.byImageID {
+	for _, im := range m.mounts {
-		if err := im.unmount(); err != nil {
-			logrus.Error(err)
-			retErr = err
-		}
-	}
-	for _, im := range m.withoutID {
 		if err := im.unmount(); err != nil {
 			logrus.Error(err)
 			retErr = err
@@ -146,10 +148,10 @@ func (m *imageSources) Add(im *imageMount) {
 	switch im.image {
 	case nil:
 		im.image = &dockerimage.Image{}
-		m.withoutID = append(m.withoutID, im)
 	default:
 		m.byImageID[im.image.ImageID()] = im
 	}
+	m.mounts = append(m.mounts, im)
 }
 
 // imageMount is a reference to an image that can be used as a builder.Source

builder/dockerfile/internals.go

@@ -116,7 +116,7 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
 		return err
 	}
 
-	imageMount, err := b.imageSources.Get(state.imageID)
+	imageMount, err := b.imageSources.Get(state.imageID, true)
 	if err != nil {
 		return errors.Wrapf(err, "failed to get destination image %q", state.imageID)
 	}

daemon/build.go

@@ -18,6 +18,7 @@ import (
 )
 
 type releaseableLayer struct {
+	released   bool
 	layerStore layer.Store
 	roLayer    layer.Layer
 	rwLayer    layer.RWLayer
@@ -70,6 +71,10 @@ func (rl *releaseableLayer) DiffID() layer.DiffID {
 }
 
 func (rl *releaseableLayer) Release() error {
+	if rl.released {
+		return nil
+	}
+	rl.released = true
 	rl.releaseRWLayer()
 	return rl.releaseROLayer()
 }
@@ -143,8 +148,11 @@ func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID st
 		return nil, layer, err
 	}
 
-	if !opts.ForcePull {
+	if opts.PullOption != backend.PullOptionForcePull {
-		image, _ := daemon.GetImage(refOrID)
+		image, err := daemon.GetImage(refOrID)
+		if err != nil && opts.PullOption == backend.PullOptionNoPull {
+			return nil, nil, err
+		}
 		// TODO: shouldn't we error out if error is different from "not found" ?
 		if image != nil {
 			layer, err := newReleasableLayerForImage(image, daemon.layerStore)

integration-cli/docker_api_build_test.go

@@ -4,11 +4,14 @@ import (
 	"archive/tar"
 	"bytes"
 	"encoding/json"
+	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"regexp"
 	"strings"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli/build/fakecontext"
 	"github.com/docker/docker/integration-cli/cli/build/fakegit"
@@ -322,6 +325,44 @@ func (s *DockerSuite) TestBuildOnBuildCache(c *check.C) {
 	assert.Equal(c, parentID, image.Parent)
 }
 
+func (s *DockerRegistrySuite) TestBuildCopyFromForcePull(c *check.C) {
+	client, err := request.NewClient()
+	require.NoError(c, err)
+
+	repoName := fmt.Sprintf("%v/dockercli/busybox", privateRegistryURL)
+	// tag the image to upload it to the private registry
+	err = client.ImageTag(context.TODO(), "busybox", repoName)
+	assert.Nil(c, err)
+	// push the image to the registry
+	rc, err := client.ImagePush(context.TODO(), repoName, types.ImagePushOptions{RegistryAuth: "{}"})
+	assert.Nil(c, err)
+	_, err = io.Copy(ioutil.Discard, rc)
+	assert.Nil(c, err)
+
+	dockerfile := fmt.Sprintf(`
+		FROM %s AS foo
+		RUN touch abc
+		FROM %s
+		COPY --from=foo /abc /
+		`, repoName, repoName)
+
+	ctx := fakecontext.New(c, "",
+		fakecontext.WithDockerfile(dockerfile),
+	)
+	defer ctx.Close()
+
+	res, body, err := request.Post(
+		"/build?pull=1",
+		request.RawContent(ctx.AsTarReader(c)),
+		request.ContentType("application/x-tar"))
+	require.NoError(c, err)
+	assert.Equal(c, http.StatusOK, res.StatusCode)
+
+	out, err := testutil.ReadBody(body)
+	require.NoError(c, err)
+	assert.Contains(c, string(out), "Successfully built")
+}
+
 type buildLine struct {
 	Stream string
 	Aux    struct {