diff --git a/api/types/backend/build.go b/api/types/backend/build.go index 5de35e6d88..db3b26f070 100644 --- a/api/types/backend/build.go +++ b/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 } diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 1935ac85c5..cc6d0990dc 100644 --- a/builder/dockerfile/dispatchers.go +++ b/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 } diff --git a/builder/dockerfile/imagecontext.go b/builder/dockerfile/imagecontext.go index 2a5b6e2579..4e2a40fc1a 100644 --- a/builder/dockerfile/imagecontext.go +++ b/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 { - if err := im.unmount(); err != nil { - logrus.Error(err) - retErr = err - } - } - for _, im := range m.withoutID { + for _, im := range m.mounts { 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 diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 213e64a52e..6f232df3db 100644 --- a/builder/dockerfile/internals.go +++ b/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) } diff --git a/daemon/build.go b/daemon/build.go index 3faa2affd4..93a48d51fe 100644 --- a/daemon/build.go +++ b/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 { - image, _ := daemon.GetImage(refOrID) + if opts.PullOption != backend.PullOptionForcePull { + 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) diff --git a/integration-cli/docker_api_build_test.go b/integration-cli/docker_api_build_test.go index 64ad7c3251..7a7549a47e 100644 --- a/integration-cli/docker_api_build_test.go +++ b/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 {