Browse Source

Implicit copy-from with reference

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Tonis Tiigi 8 years ago
parent
commit
73b4b8ed7f

+ 28 - 16
builder/dockerfile/dispatchers.go

@@ -187,25 +187,16 @@ func dispatchCopy(b *Builder, args []string, attributes map[string]bool, origina
 		return err
 	}
 
-	var contextID *int
+	var im *imageMount
 	if flFrom.IsUsed() {
-		flFrom.Value = strings.ToLower(flFrom.Value)
-		if context, ok := b.imageContexts.byName[flFrom.Value]; ok {
-			contextID = &context
-		} else {
-			var err error
-			context, err := strconv.Atoi(flFrom.Value)
-			if err != nil {
-				return errors.Wrap(err, "from expects an integer value corresponding to the context number")
-			}
-			if err := b.imageContexts.validate(context); err != nil {
-				return err
-			}
-			contextID = &context
+		var err error
+		im, err = b.imageContexts.get(flFrom.Value)
+		if err != nil {
+			return err
 		}
 	}
 
-	return b.runContextCommand(args, false, false, "COPY", contextID)
+	return b.runContextCommand(args, false, false, "COPY", im)
 }
 
 // FROM imagename
@@ -232,7 +223,7 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 	var image builder.Image
 
 	b.resetImageCache()
-	if err := b.imageContexts.new(ctxName); err != nil {
+	if _, err := b.imageContexts.new(ctxName, true); err != nil {
 		return err
 	}
 
@@ -844,3 +835,24 @@ func getShell(c *container.Config) []string {
 	}
 	return append([]string{}, c.Shell[:]...)
 }
+
+// mountByRef creates an imageMount from a reference. pulling the image if needed.
+func mountByRef(b *Builder, name string) (*imageMount, error) {
+	var image builder.Image
+	if !b.options.PullParent {
+		image, _ = b.docker.GetImageOnBuild(name)
+	}
+	if image == nil {
+		var err error
+		image, err = b.docker.PullOnBuild(b.clientCtx, name, b.options.AuthConfigs, b.Output)
+		if err != nil {
+			return nil, err
+		}
+	}
+	im, err := b.imageContexts.new("", false)
+	if err != nil {
+		return nil, err
+	}
+	im.id = image.ImageID()
+	return im, nil
+}

+ 78 - 44
builder/dockerfile/imagecontext.go

@@ -1,6 +1,8 @@
 package dockerfile
 
 import (
+	"strconv"
+	"strings"
 	"sync"
 
 	"github.com/Sirupsen/logrus"
@@ -14,28 +16,25 @@ import (
 type imageContexts struct {
 	b      *Builder
 	list   []*imageMount
-	byName map[string]int
+	byName map[string]*imageMount
 	cache  *pathCache
 }
 
-type imageMount struct {
-	id      string
-	ctx     builder.Context
-	release func() error
-}
-
-func (ic *imageContexts) new(name string) error {
+func (ic *imageContexts) new(name string, increment bool) (*imageMount, error) {
+	im := &imageMount{ic: ic}
 	if len(name) > 0 {
 		if ic.byName == nil {
-			ic.byName = make(map[string]int)
+			ic.byName = make(map[string]*imageMount)
 		}
 		if _, ok := ic.byName[name]; ok {
-			return errors.Errorf("duplicate name %s", name)
+			return nil, errors.Errorf("duplicate name %s", name)
 		}
-		ic.byName[name] = len(ic.list)
+		ic.byName[name] = im
 	}
-	ic.list = append(ic.list, &imageMount{})
-	return nil
+	if increment {
+		ic.list = append(ic.list, im)
+	}
+	return im, nil
 }
 
 func (ic *imageContexts) update(imageID string) {
@@ -53,59 +52,94 @@ func (ic *imageContexts) validate(i int) error {
 	return nil
 }
 
-func (ic *imageContexts) context(i int) (builder.Context, error) {
-	if err := ic.validate(i); err != nil {
-		return nil, err
-	}
-	im := ic.list[i]
-	if im.ctx == nil {
-		if im.id == "" {
-			return nil, errors.Errorf("could not copy from empty context")
-		}
-		p, release, err := ic.b.docker.MountImage(im.id)
-		if err != nil {
-			return nil, errors.Wrapf(err, "failed to mount %s", im.id)
+func (ic *imageContexts) get(indexOrName string) (*imageMount, error) {
+	index, err := strconv.Atoi(indexOrName)
+	if err == nil {
+		if err := ic.validate(index); err != nil {
+			return nil, err
 		}
-		ctx, err := remotecontext.NewLazyContext(p)
-		if err != nil {
-			return nil, errors.Wrapf(err, "failed to create lazycontext for %s", p)
-		}
-		logrus.Debugf("mounted image: %s %s", im.id, p)
-		im.release = release
-		im.ctx = ctx
+		return ic.list[index], nil
 	}
-	return im.ctx, nil
+	if im, ok := ic.byName[strings.ToLower(indexOrName)]; ok {
+		return im, nil
+	}
+	im, err := mountByRef(ic.b, indexOrName)
+	if err != nil {
+		return nil, errors.Wrapf(err, "invalid from flag value %s", indexOrName)
+	}
+	return im, nil
 }
 
 func (ic *imageContexts) unmount() (retErr error) {
 	for _, im := range ic.list {
-		if im.release != nil {
-			if err := im.release(); err != nil {
-				logrus.Error(errors.Wrapf(err, "failed to unmount previous build image"))
-				retErr = err
-			}
+		if err := im.unmount(); err != nil {
+			logrus.Error(err)
+			retErr = err
+		}
+	}
+	for _, im := range ic.byName {
+		if err := im.unmount(); err != nil {
+			logrus.Error(err)
+			retErr = err
 		}
 	}
 	return
 }
 
-func (ic *imageContexts) getCache(i int, path string) (interface{}, bool) {
+func (ic *imageContexts) getCache(id, path string) (interface{}, bool) {
 	if ic.cache != nil {
-		im := ic.list[i]
-		if im.id == "" {
+		if id == "" {
 			return nil, false
 		}
-		return ic.cache.get(im.id + path)
+		return ic.cache.get(id + path)
 	}
 	return nil, false
 }
 
-func (ic *imageContexts) setCache(i int, path string, v interface{}) {
+func (ic *imageContexts) setCache(id, path string, v interface{}) {
 	if ic.cache != nil {
-		ic.cache.set(ic.list[i].id+path, v)
+		ic.cache.set(id+path, v)
 	}
 }
 
+// imageMount is a reference for getting access to a buildcontext that is backed
+// by an existing image
+type imageMount struct {
+	id      string
+	ctx     builder.Context
+	release func() error
+	ic      *imageContexts
+}
+
+func (im *imageMount) context() (builder.Context, error) {
+	if im.ctx == nil {
+		if im.id == "" {
+			return nil, errors.Errorf("could not copy from empty context")
+		}
+		p, release, err := im.ic.b.docker.MountImage(im.id)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to mount %s", im.id)
+		}
+		ctx, err := remotecontext.NewLazyContext(p)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to create lazycontext for %s", p)
+		}
+		im.release = release
+		im.ctx = ctx
+	}
+	return im.ctx, nil
+}
+
+func (im *imageMount) unmount() error {
+	if im.release != nil {
+		if err := im.release(); err != nil {
+			return errors.Wrapf(err, "failed to unmount previous build image %s", im.id)
+		}
+		im.release = nil
+	}
+	return nil
+}
+
 type pathCache struct {
 	mu    sync.Mutex
 	items map[string]interface{}

+ 11 - 11
builder/dockerfile/internals.go

@@ -94,7 +94,7 @@ type copyInfo struct {
 	decompress bool
 }
 
-func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalDecompression bool, cmdName string, contextID *int) error {
+func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalDecompression bool, cmdName string, imageSource *imageMount) error {
 	if len(args) < 2 {
 		return fmt.Errorf("Invalid %s format - at least two arguments required", cmdName)
 	}
@@ -128,7 +128,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
 			continue
 		}
 		// not a URL
-		subInfos, err := b.calcCopyInfo(cmdName, orig, allowLocalDecompression, true, contextID)
+		subInfos, err := b.calcCopyInfo(cmdName, orig, allowLocalDecompression, true, imageSource)
 		if err != nil {
 			return err
 		}
@@ -298,13 +298,13 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) {
 	return &builder.HashedFileInfo{FileInfo: builder.PathFileInfo{FileInfo: tmpFileSt, FilePath: tmpFileName}, FileHash: hash}, nil
 }
 
-func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression, allowWildcards bool, contextID *int) ([]copyInfo, error) {
+func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression, allowWildcards bool, imageSource *imageMount) ([]copyInfo, error) {
 
 	// Work in daemon-specific OS filepath semantics
 	origPath = filepath.FromSlash(origPath)
 
 	// validate windows paths from other images
-	if contextID != nil && runtime.GOOS == "windows" {
+	if imageSource != nil && runtime.GOOS == "windows" {
 		forbid := regexp.MustCompile("(?i)^" + string(os.PathSeparator) + "?(windows(" + string(os.PathSeparator) + ".+)?)?$")
 		if p := filepath.Clean(origPath); p == "." || forbid.MatchString(p) {
 			return nil, errors.Errorf("copy from %s is not allowed on windows", origPath)
@@ -318,8 +318,8 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 
 	context := b.context
 	var err error
-	if contextID != nil {
-		context, err = b.imageContexts.context(*contextID)
+	if imageSource != nil {
+		context, err = imageSource.context()
 		if err != nil {
 			return nil, err
 		}
@@ -346,7 +346,7 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 
 			// Note we set allowWildcards to false in case the name has
 			// a * in it
-			subInfos, err := b.calcCopyInfo(cmdName, path, allowLocalDecompression, false, contextID)
+			subInfos, err := b.calcCopyInfo(cmdName, path, allowLocalDecompression, false, imageSource)
 			if err != nil {
 				return err
 			}
@@ -370,9 +370,9 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 	if !handleHash {
 		return copyInfos, nil
 	}
-	if contextID != nil {
+	if imageSource != nil {
 		// fast-cache based on imageID
-		if h, ok := b.imageContexts.getCache(*contextID, origPath); ok {
+		if h, ok := b.imageContexts.getCache(imageSource.id, origPath); ok {
 			hfi.SetHash(h.(string))
 			return copyInfos, nil
 		}
@@ -401,8 +401,8 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 	hasher := sha256.New()
 	hasher.Write([]byte(strings.Join(subfiles, ",")))
 	hfi.SetHash("dir:" + hex.EncodeToString(hasher.Sum(nil)))
-	if contextID != nil {
-		b.imageContexts.setCache(*contextID, origPath, hfi.Hash())
+	if imageSource != nil {
+		b.imageContexts.setCache(imageSource.id, origPath, hfi.Hash())
 	}
 
 	return copyInfos, nil

+ 30 - 2
integration-cli/docker_cli_build_test.go

@@ -5831,7 +5831,7 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFSErrors(c *check.C) {
 
 	buildImage("build1", withExternalBuildContext(ctx)).Assert(c, icmd.Expected{
 		ExitCode: 1,
-		Err:      "from expects an integer value corresponding to the context number",
+		Err:      "invalid from flag value foo",
 	})
 
 	dockerfile = `
@@ -5861,7 +5861,7 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFSErrors(c *check.C) {
 
 	buildImage("build1", withExternalBuildContext(ctx)).Assert(c, icmd.Expected{
 		ExitCode: 1,
-		Err:      "invalid context value bar",
+		Err:      "invalid from flag value bar",
 	})
 
 	dockerfile = `
@@ -5913,6 +5913,34 @@ func (s *DockerSuite) TestBuildCopyFromPreviousFrom(c *check.C) {
 	c.Assert(strings.TrimSpace(out), check.Equals, "def")
 }
 
+func (s *DockerSuite) TestBuildCopyFromImplicitFrom(c *check.C) {
+	dockerfile := `
+		FROM busybox
+		COPY --from=busybox /etc/passwd /mypasswd
+		RUN cmp /etc/passwd /mypasswd`
+
+	if DaemonIsWindows() {
+		dockerfile = `
+			FROM busybox
+			COPY --from=busybox License.txt foo`
+	}
+
+	ctx := fakeContext(c, dockerfile, map[string]string{
+		"Dockerfile": dockerfile,
+	})
+	defer ctx.Close()
+
+	result := buildImage("build1", withExternalBuildContext(ctx))
+	result.Assert(c, icmd.Success)
+
+	if DaemonIsWindows() {
+		out, _ := dockerCmd(c, "run", "build1", "cat", "License.txt")
+		c.Assert(len(out), checker.GreaterThan, 10)
+		out2, _ := dockerCmd(c, "run", "build1", "cat", "foo")
+		c.Assert(out, check.Equals, out2)
+	}
+}
+
 // TestBuildOpaqueDirectory tests that a build succeeds which
 // creates opaque directories.
 // See https://github.com/docker/docker/issues/25244