Browse Source

Refactor imageContexts into two different structs.

buildStages now tracks the imageID and runConfig for a build stage

imageMounter tracks image mounts so they can released when the build ends.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
Daniel Nephin 8 years ago
parent
commit
6c28e8edd5

+ 1 - 1
api/types/backend/build.go

@@ -23,7 +23,7 @@ type BuildConfig struct {
 	Options        *types.ImageBuildOptions
 	Options        *types.ImageBuildOptions
 }
 }
 
 
-// GetImageAndLayerOptions are the options supported by GetImageAndLayer
+// GetImageAndLayerOptions are the options supported by GetImageAndReleasableLayer
 type GetImageAndLayerOptions struct {
 type GetImageAndLayerOptions struct {
 	ForcePull  bool
 	ForcePull  bool
 	AuthConfig map[string]types.AuthConfig
 	AuthConfig map[string]types.AuthConfig

+ 7 - 2
builder/builder.go

@@ -34,7 +34,7 @@ type Source interface {
 
 
 // Backend abstracts calls to a Docker Daemon.
 // Backend abstracts calls to a Docker Daemon.
 type Backend interface {
 type Backend interface {
-	GetImageAndLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (Image, ReleaseableLayer, error)
+	ImageBackend
 
 
 	// ContainerAttachRaw attaches to container.
 	// ContainerAttachRaw attaches to container.
 	ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool, attached chan struct{}) error
 	ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool, attached chan struct{}) error
@@ -60,6 +60,11 @@ type Backend interface {
 	CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error
 	CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error
 }
 }
 
 
+// ImageBackend are the interface methods required from an image component
+type ImageBackend interface {
+	GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (Image, ReleaseableLayer, error)
+}
+
 // Result is the output produced by a Builder
 // Result is the output produced by a Builder
 type Result struct {
 type Result struct {
 	ImageID   string
 	ImageID   string
@@ -89,5 +94,5 @@ type Image interface {
 // ReleaseableLayer is an image layer that can be mounted and released
 // ReleaseableLayer is an image layer that can be mounted and released
 type ReleaseableLayer interface {
 type ReleaseableLayer interface {
 	Release() error
 	Release() error
-	Mount(string) (string, error)
+	Mount() (string, error)
 }
 }

+ 5 - 3
builder/dockerfile/builder.go

@@ -102,11 +102,12 @@ type Builder struct {
 	clientCtx context.Context
 	clientCtx context.Context
 
 
 	tmpContainers map[string]struct{}
 	tmpContainers map[string]struct{}
-	imageContexts *imageContexts // helper for storing contexts from builds
+	buildStages   *buildStages
 	disableCommit bool
 	disableCommit bool
 	cacheBusted   bool
 	cacheBusted   bool
 	buildArgs     *buildArgs
 	buildArgs     *buildArgs
 	imageCache    builder.ImageCache
 	imageCache    builder.ImageCache
+	imageSources  *imageSources
 }
 }
 
 
 // newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options.
 // newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options.
@@ -125,7 +126,8 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
 		docker:        options.Backend,
 		docker:        options.Backend,
 		tmpContainers: map[string]struct{}{},
 		tmpContainers: map[string]struct{}{},
 		buildArgs:     newBuildArgs(config.BuildArgs),
 		buildArgs:     newBuildArgs(config.BuildArgs),
-		imageContexts: &imageContexts{},
+		buildStages:   newBuildStages(),
+		imageSources:  newImageSources(clientCtx, options),
 	}
 	}
 	return b
 	return b
 }
 }
@@ -140,7 +142,7 @@ func (b *Builder) resetImageCache() {
 // Build runs the Dockerfile builder by parsing the Dockerfile and executing
 // Build runs the Dockerfile builder by parsing the Dockerfile and executing
 // the instructions from the file.
 // the instructions from the file.
 func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*builder.Result, error) {
 func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*builder.Result, error) {
-	defer b.imageContexts.unmount()
+	defer b.imageSources.Unmount()
 
 
 	// TODO: Remove source field from Builder
 	// TODO: Remove source field from Builder
 	b.source = source
 	b.source = source

+ 22 - 20
builder/dockerfile/dispatchers.go

@@ -20,9 +20,9 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/api/types/strslice"
+	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/dockerfile/parser"
 	"github.com/docker/docker/builder/dockerfile/parser"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/go-connections/nat"
 	"github.com/docker/go-connections/nat"
@@ -174,14 +174,19 @@ func dispatchCopy(req dispatchRequest) error {
 
 
 func (b *Builder) getImageMount(fromFlag *Flag) (*imageMount, error) {
 func (b *Builder) getImageMount(fromFlag *Flag) (*imageMount, error) {
 	if !fromFlag.IsUsed() {
 	if !fromFlag.IsUsed() {
-		// TODO: this could return the mount in the default case as well
+		// TODO: this could return the source in the default case as well?
 		return nil, nil
 		return nil, nil
 	}
 	}
-	im, err := b.imageContexts.getMount(fromFlag.Value)
-	if err != nil || im != nil {
-		return im, err
+
+	imageRefOrID := fromFlag.Value
+	stage, err := b.buildStages.get(fromFlag.Value)
+	if err != nil {
+		return nil, err
+	}
+	if stage != nil {
+		imageRefOrID = stage.ImageID()
 	}
 	}
-	return b.getImage(fromFlag.Value)
+	return b.imageSources.Get(imageRefOrID)
 }
 }
 
 
 // FROM imagename[:tag | @digest] [AS build-stage-name]
 // FROM imagename[:tag | @digest] [AS build-stage-name]
@@ -201,7 +206,7 @@ func from(req dispatchRequest) error {
 	if err != nil {
 	if err != nil {
 		return err
 		return err
 	}
 	}
-	if err := req.builder.imageContexts.add(stageName, image); err != nil {
+	if err := req.builder.buildStages.add(stageName, image); err != nil {
 		return err
 		return err
 	}
 	}
 	req.state.beginStage(stageName, image)
 	req.state.beginStage(stageName, image)
@@ -229,7 +234,12 @@ func parseBuildStageName(args []string) (string, error) {
 	return stageName, nil
 	return stageName, nil
 }
 }
 
 
-func (b *Builder) getFromImage(shlex *ShellLex, name string) (*imageMount, error) {
+// scratchImage is used as a token for the empty base image. It uses buildStage
+// as a convenient implementation of builder.Image, but is not actually a
+// buildStage.
+var scratchImage builder.Image = &buildStage{}
+
+func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, error) {
 	substitutionArgs := []string{}
 	substitutionArgs := []string{}
 	for key, value := range b.buildArgs.GetAllMeta() {
 	for key, value := range b.buildArgs.GetAllMeta() {
 		substitutionArgs = append(substitutionArgs, key+"="+value)
 		substitutionArgs = append(substitutionArgs, key+"="+value)
@@ -240,7 +250,7 @@ func (b *Builder) getFromImage(shlex *ShellLex, name string) (*imageMount, error
 		return nil, err
 		return nil, err
 	}
 	}
 
 
-	if im, ok := b.imageContexts.byName[name]; ok {
+	if im, ok := b.buildStages.getByName(name); ok {
 		return im, nil
 		return im, nil
 	}
 	}
 
 
@@ -249,21 +259,13 @@ func (b *Builder) getFromImage(shlex *ShellLex, name string) (*imageMount, error
 		if runtime.GOOS == "windows" {
 		if runtime.GOOS == "windows" {
 			return nil, errors.New("Windows does not support FROM scratch")
 			return nil, errors.New("Windows does not support FROM scratch")
 		}
 		}
-		return newImageMount(nil, nil), nil
+		return scratchImage, nil
 	}
 	}
-	return b.getImage(name)
-}
-
-func (b *Builder) getImage(name string) (*imageMount, error) {
-	image, layer, err := b.docker.GetImageAndLayer(b.clientCtx, name, backend.GetImageAndLayerOptions{
-		ForcePull:  b.options.PullParent,
-		AuthConfig: b.options.AuthConfigs,
-		Output:     b.Output,
-	})
+	imageMount, err := b.imageSources.Get(name)
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
-	return newImageMount(image, layer), nil
+	return imageMount.Image(), nil
 }
 }
 
 
 func processOnBuild(req dispatchRequest) error {
 func processOnBuild(req dispatchRequest) error {

+ 7 - 2
builder/dockerfile/dispatchers_test.go

@@ -49,15 +49,20 @@ func defaultDispatchReq(builder *Builder, args ...string) dispatchRequest {
 
 
 func newBuilderWithMockBackend() *Builder {
 func newBuilderWithMockBackend() *Builder {
 	mockBackend := &MockBackend{}
 	mockBackend := &MockBackend{}
+	ctx := context.Background()
 	b := &Builder{
 	b := &Builder{
 		options:       &types.ImageBuildOptions{},
 		options:       &types.ImageBuildOptions{},
 		docker:        mockBackend,
 		docker:        mockBackend,
 		buildArgs:     newBuildArgs(make(map[string]*string)),
 		buildArgs:     newBuildArgs(make(map[string]*string)),
 		tmpContainers: make(map[string]struct{}),
 		tmpContainers: make(map[string]struct{}),
 		Stdout:        new(bytes.Buffer),
 		Stdout:        new(bytes.Buffer),
-		clientCtx:     context.Background(),
+		clientCtx:     ctx,
 		disableCommit: true,
 		disableCommit: true,
-		imageContexts: &imageContexts{},
+		imageSources: newImageSources(ctx, builderOptions{
+			Options: &types.ImageBuildOptions{},
+			Backend: mockBackend,
+		}),
+		buildStages: newBuildStages(),
 	}
 	}
 	return b
 	return b
 }
 }

+ 122 - 71
builder/dockerfile/imagecontext.go

@@ -5,10 +5,12 @@ import (
 	"strings"
 	"strings"
 
 
 	"github.com/Sirupsen/logrus"
 	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/remotecontext"
 	"github.com/docker/docker/builder/remotecontext"
 	"github.com/pkg/errors"
 	"github.com/pkg/errors"
+	"golang.org/x/net/context"
 )
 )
 
 
 type pathCache interface {
 type pathCache interface {
@@ -16,66 +18,128 @@ type pathCache interface {
 	Store(key, value interface{})
 	Store(key, value interface{})
 }
 }
 
 
-// imageContexts is a helper for stacking up built image rootfs and reusing
-// them as contexts
-type imageContexts struct {
-	list   []*imageMount
-	byName map[string]*imageMount
-	cache  pathCache
+type buildStage struct {
+	id     string
+	config *container.Config
 }
 }
 
 
-func (ic *imageContexts) add(name string, im *imageMount) error {
-	if len(name) > 0 {
-		if ic.byName == nil {
-			ic.byName = make(map[string]*imageMount)
-		}
-		if _, ok := ic.byName[name]; ok {
-			return errors.Errorf("duplicate name %s", name)
-		}
-		ic.byName[name] = im
-	}
-	ic.list = append(ic.list, im)
-	return nil
+func newBuildStageFromImage(image builder.Image) *buildStage {
+	return &buildStage{id: image.ImageID(), config: image.RunConfig()}
 }
 }
 
 
-func (ic *imageContexts) update(imageID string, runConfig *container.Config) {
-	ic.list[len(ic.list)-1].update(imageID, runConfig)
+func (b *buildStage) ImageID() string {
+	return b.id
 }
 }
 
 
-func (ic *imageContexts) validate(i int) error {
-	if i < 0 || i >= len(ic.list)-1 {
-		if i == len(ic.list)-1 {
-			return errors.New("refers to current build stage")
-		}
-		return errors.New("index out of bounds")
-	}
-	return nil
+func (b *buildStage) RunConfig() *container.Config {
+	return b.config
 }
 }
 
 
-func (ic *imageContexts) getMount(indexOrName string) (*imageMount, error) {
+func (b *buildStage) update(imageID string, runConfig *container.Config) {
+	b.id = imageID
+	b.config = runConfig
+}
+
+var _ builder.Image = &buildStage{}
+
+// buildStages tracks each stage of a build so they can be retrieved by index
+// or by name.
+type buildStages struct {
+	sequence []*buildStage
+	byName   map[string]*buildStage
+}
+
+func newBuildStages() *buildStages {
+	return &buildStages{byName: make(map[string]*buildStage)}
+}
+
+func (s *buildStages) getByName(name string) (builder.Image, bool) {
+	stage, ok := s.byName[strings.ToLower(name)]
+	return stage, ok
+}
+
+func (s *buildStages) get(indexOrName string) (builder.Image, error) {
 	index, err := strconv.Atoi(indexOrName)
 	index, err := strconv.Atoi(indexOrName)
 	if err == nil {
 	if err == nil {
-		if err := ic.validate(index); err != nil {
+		if err := s.validateIndex(index); err != nil {
 			return nil, err
 			return nil, err
 		}
 		}
-		return ic.list[index], nil
+		return s.sequence[index], nil
 	}
 	}
-	if im, ok := ic.byName[strings.ToLower(indexOrName)]; ok {
+	if im, ok := s.byName[strings.ToLower(indexOrName)]; ok {
 		return im, nil
 		return im, nil
 	}
 	}
 	return nil, nil
 	return nil, nil
 }
 }
 
 
-func (ic *imageContexts) unmount() (retErr error) {
-	for _, iml := range append([][]*imageMount{}, ic.list, ic.implicitMounts) {
-		for _, im := range iml {
-			if err := im.unmount(); err != nil {
-				logrus.Error(err)
-				retErr = err
-			}
+func (s *buildStages) validateIndex(i int) error {
+	if i < 0 || i >= len(s.sequence)-1 {
+		if i == len(s.sequence)-1 {
+			return errors.New("refers to current build stage")
 		}
 		}
+		return errors.New("index out of bounds")
 	}
 	}
-	for _, im := range ic.byName {
+	return nil
+}
+
+func (s *buildStages) add(name string, image builder.Image) error {
+	stage := newBuildStageFromImage(image)
+	name = strings.ToLower(name)
+	if len(name) > 0 {
+		if _, ok := s.byName[name]; ok {
+			return errors.Errorf("duplicate name %s", name)
+		}
+		s.byName[name] = stage
+	}
+	s.sequence = append(s.sequence, stage)
+	return nil
+}
+
+func (s *buildStages) update(imageID string, runConfig *container.Config) {
+	s.sequence[len(s.sequence)-1].update(imageID, runConfig)
+}
+
+type getAndMountFunc func(string) (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
+	getImage  getAndMountFunc
+	cache     pathCache // TODO: remove
+}
+
+func newImageSources(ctx context.Context, options builderOptions) *imageSources {
+	getAndMount := func(idOrRef string) (builder.Image, builder.ReleaseableLayer, error) {
+		return options.Backend.GetImageAndReleasableLayer(ctx, idOrRef, backend.GetImageAndLayerOptions{
+			ForcePull:  options.Options.PullParent,
+			AuthConfig: options.Options.AuthConfigs,
+			Output:     options.ProgressWriter.Output,
+		})
+	}
+
+	return &imageSources{
+		byImageID: make(map[string]*imageMount),
+		getImage:  getAndMount,
+	}
+}
+
+func (m *imageSources) Get(idOrRef string) (*imageMount, error) {
+	if im, ok := m.byImageID[idOrRef]; ok {
+		return im, nil
+	}
+
+	image, layer, err := m.getImage(idOrRef)
+	if err != nil {
+		return nil, err
+	}
+	im := newImageMount(image, layer)
+	m.byImageID[image.ImageID()] = im
+	return im, nil
+}
+
+func (m *imageSources) Unmount() (retErr error) {
+	for _, im := range m.byImageID {
 		if err := im.unmount(); err != nil {
 		if err := im.unmount(); err != nil {
 			logrus.Error(err)
 			logrus.Error(err)
 			retErr = err
 			retErr = err
@@ -84,47 +148,43 @@ func (ic *imageContexts) unmount() (retErr error) {
 	return
 	return
 }
 }
 
 
-// TODO: remove getCache/setCache from imageContexts
-func (ic *imageContexts) getCache(id, path string) (interface{}, bool) {
-	if ic.cache != nil {
+// TODO: remove getCache/setCache from imageSources
+func (m *imageSources) getCache(id, path string) (interface{}, bool) {
+	if m.cache != nil {
 		if id == "" {
 		if id == "" {
 			return nil, false
 			return nil, false
 		}
 		}
-		return ic.cache.Load(id + path)
+		return m.cache.Load(id + path)
 	}
 	}
 	return nil, false
 	return nil, false
 }
 }
 
 
-func (ic *imageContexts) setCache(id, path string, v interface{}) {
-	if ic.cache != nil {
-		ic.cache.Store(id+path, v)
+func (m *imageSources) setCache(id, path string, v interface{}) {
+	if m.cache != nil {
+		m.cache.Store(id+path, v)
 	}
 	}
 }
 }
 
 
 // imageMount is a reference to an image that can be used as a builder.Source
 // imageMount is a reference to an image that can be used as a builder.Source
 type imageMount struct {
 type imageMount struct {
-	id        string
-	source    builder.Source
-	runConfig *container.Config
-	layer     builder.ReleaseableLayer
+	image  builder.Image
+	source builder.Source
+	layer  builder.ReleaseableLayer
 }
 }
 
 
 func newImageMount(image builder.Image, layer builder.ReleaseableLayer) *imageMount {
 func newImageMount(image builder.Image, layer builder.ReleaseableLayer) *imageMount {
-	im := &imageMount{layer: layer}
-	if image != nil {
-		im.update(image.ImageID(), image.RunConfig())
-	}
+	im := &imageMount{image: image, layer: layer}
 	return im
 	return im
 }
 }
 
 
-func (im *imageMount) context() (builder.Source, error) {
+func (im *imageMount) Source() (builder.Source, error) {
 	if im.source == nil {
 	if im.source == nil {
-		if im.id == "" || im.layer == nil {
+		if im.layer == nil {
 			return nil, errors.Errorf("empty context")
 			return nil, errors.Errorf("empty context")
 		}
 		}
-		mountPath, err := im.layer.Mount(im.id)
+		mountPath, err := im.layer.Mount()
 		if err != nil {
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to mount %s", im.id)
+			return nil, errors.Wrapf(err, "failed to mount %s", im.image.ImageID())
 		}
 		}
 		source, err := remotecontext.NewLazyContext(mountPath)
 		source, err := remotecontext.NewLazyContext(mountPath)
 		if err != nil {
 		if err != nil {
@@ -140,20 +200,11 @@ func (im *imageMount) unmount() error {
 		return nil
 		return nil
 	}
 	}
 	if err := im.layer.Release(); err != nil {
 	if err := im.layer.Release(); err != nil {
-		return errors.Wrapf(err, "failed to unmount previous build image %s", im.id)
+		return errors.Wrapf(err, "failed to unmount previous build image %s", im.image.ImageID())
 	}
 	}
 	return nil
 	return nil
 }
 }
 
 
-func (im *imageMount) update(imageID string, runConfig *container.Config) {
-	im.id = imageID
-	im.runConfig = runConfig
-}
-
-func (im *imageMount) ImageID() string {
-	return im.id
-}
-
-func (im *imageMount) RunConfig() *container.Config {
-	return im.runConfig
+func (im *imageMount) Image() builder.Image {
+	return im.image
 }
 }

+ 5 - 5
builder/dockerfile/internals.go

@@ -78,7 +78,7 @@ func (b *Builder) commitContainer(dispatchState *dispatchState, id string, conta
 	}
 	}
 
 
 	dispatchState.imageID = imageID
 	dispatchState.imageID = imageID
-	b.imageContexts.update(imageID, dispatchState.runConfig)
+	b.buildStages.update(imageID, dispatchState.runConfig)
 	return nil
 	return nil
 }
 }
 
 
@@ -369,7 +369,7 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 	source := b.source
 	source := b.source
 	var err error
 	var err error
 	if imageSource != nil {
 	if imageSource != nil {
-		source, err = imageSource.context()
+		source, err = imageSource.Source()
 		if err != nil {
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to copy")
 			return nil, errors.Wrapf(err, "failed to copy")
 		}
 		}
@@ -427,7 +427,7 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 
 
 	if imageSource != nil {
 	if imageSource != nil {
 		// fast-cache based on imageID
 		// fast-cache based on imageID
-		if h, ok := b.imageContexts.getCache(imageSource.id, origPath); ok {
+		if h, ok := b.imageSources.getCache(imageSource.Image().ImageID(), origPath); ok {
 			copyInfos[0].hash = h.(string)
 			copyInfos[0].hash = h.(string)
 			return copyInfos, nil
 			return copyInfos, nil
 		}
 		}
@@ -473,7 +473,7 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 	hasher.Write([]byte(strings.Join(subfiles, ",")))
 	hasher.Write([]byte(strings.Join(subfiles, ",")))
 	copyInfos[0].hash = "dir:" + hex.EncodeToString(hasher.Sum(nil))
 	copyInfos[0].hash = "dir:" + hex.EncodeToString(hasher.Sum(nil))
 	if imageSource != nil {
 	if imageSource != nil {
-		b.imageContexts.setCache(imageSource.id, origPath, copyInfos[0].hash)
+		b.imageSources.setCache(imageSource.Image().ImageID(), origPath, copyInfos[0].hash)
 	}
 	}
 
 
 	return copyInfos, nil
 	return copyInfos, nil
@@ -501,7 +501,7 @@ func (b *Builder) probeCache(dispatchState *dispatchState, runConfig *container.
 	fmt.Fprint(b.Stdout, " ---> Using cache\n")
 	fmt.Fprint(b.Stdout, " ---> Using cache\n")
 	logrus.Debugf("[BUILDER] Use cached version: %s", runConfig.Cmd)
 	logrus.Debugf("[BUILDER] Use cached version: %s", runConfig.Cmd)
 	dispatchState.imageID = string(cache)
 	dispatchState.imageID = string(cache)
-	b.imageContexts.update(dispatchState.imageID, runConfig)
+	b.buildStages.update(dispatchState.imageID, runConfig)
 
 
 	return true, nil
 	return true, nil
 }
 }

+ 2 - 10
builder/dockerfile/mockbackend_test.go

@@ -66,15 +66,7 @@ func (m *MockBackend) CopyOnBuild(containerID string, destPath string, srcRoot s
 	return nil
 	return nil
 }
 }
 
 
-func (m *MockBackend) HasExperimental() bool {
-	return false
-}
-
-func (m *MockBackend) SquashImage(from string, to string) (string, error) {
-	return "", nil
-}
-
-func (m *MockBackend) GetImageAndLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) {
+func (m *MockBackend) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) {
 	if m.getImageFunc != nil {
 	if m.getImageFunc != nil {
 		return m.getImageFunc(refOrID)
 		return m.getImageFunc(refOrID)
 	}
 	}
@@ -112,6 +104,6 @@ func (l *mockLayer) Release() error {
 	return nil
 	return nil
 }
 }
 
 
-func (l *mockLayer) Mount(_ string) (string, error) {
+func (l *mockLayer) Mount() (string, error) {
 	return "mountPath", nil
 	return "mountPath", nil
 }
 }

+ 54 - 38
daemon/build.go

@@ -1,6 +1,7 @@
 package daemon
 package daemon
 
 
 import (
 import (
+	"github.com/Sirupsen/logrus"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/backend"
@@ -15,54 +16,62 @@ import (
 )
 )
 
 
 type releaseableLayer struct {
 type releaseableLayer struct {
-	rwLayer layer.RWLayer
-	release func(layer.RWLayer) error
-	mount   func(string) (layer.RWLayer, error)
+	layerStore layer.Store
+	roLayer    layer.Layer
+	rwLayer    layer.RWLayer
 }
 }
 
 
-func (rl *releaseableLayer) Release() error {
-	if rl.rwLayer == nil {
-		return nil
+func (rl *releaseableLayer) Mount() (string, error) {
+	if rl.roLayer == nil {
+		return "", errors.New("can not mount an image with no root FS")
 	}
 	}
-	rl.rwLayer.Unmount()
-	return rl.release(rl.rwLayer)
-}
-
-func (rl *releaseableLayer) Mount(imageID string) (string, error) {
 	var err error
 	var err error
-	rl.rwLayer, err = rl.mount(imageID)
+	mountID := stringid.GenerateRandomID()
+	rl.rwLayer, err = rl.layerStore.CreateRWLayer(mountID, rl.roLayer.ChainID(), nil)
 	if err != nil {
 	if err != nil {
 		return "", errors.Wrap(err, "failed to create rwlayer")
 		return "", errors.Wrap(err, "failed to create rwlayer")
 	}
 	}
 
 
-	mountPath, err := rl.rwLayer.Mount("")
+	return rl.rwLayer.Mount("")
+}
+
+func (rl *releaseableLayer) Release() error {
+	rl.releaseRWLayer()
+	return rl.releaseROLayer()
+}
+
+func (rl *releaseableLayer) releaseRWLayer() error {
+	if rl.rwLayer == nil {
+		return nil
+	}
+	metadata, err := rl.layerStore.ReleaseRWLayer(rl.rwLayer)
+	layer.LogReleaseMetadata(metadata)
 	if err != nil {
 	if err != nil {
-		releaseErr := rl.release(rl.rwLayer)
-		if releaseErr != nil {
-			err = errors.Wrapf(err, "failed to release rwlayer: %s", releaseErr.Error())
-		}
-		return "", errors.Wrap(err, "failed to mount rwlayer")
+		logrus.Errorf("Failed to release RWLayer: %s", err)
 	}
 	}
-	return mountPath, err
+	return err
 }
 }
 
 
-func (daemon *Daemon) getReleasableLayerForImage() *releaseableLayer {
-	mountFunc := func(imageID string) (layer.RWLayer, error) {
-		img, err := daemon.GetImage(imageID)
-		if err != nil {
-			return nil, err
-		}
-		mountID := stringid.GenerateRandomID()
-		return daemon.layerStore.CreateRWLayer(mountID, img.RootFS.ChainID(), nil)
+func (rl *releaseableLayer) releaseROLayer() error {
+	if rl.roLayer == nil {
+		return nil
 	}
 	}
+	metadata, err := rl.layerStore.Release(rl.roLayer)
+	layer.LogReleaseMetadata(metadata)
+	return err
+}
 
 
-	releaseFunc := func(rwLayer layer.RWLayer) error {
-		metadata, err := daemon.layerStore.ReleaseRWLayer(rwLayer)
-		layer.LogReleaseMetadata(metadata)
-		return err
+func newReleasableLayerForImage(img *image.Image, layerStore layer.Store) (builder.ReleaseableLayer, error) {
+	if img.RootFS.ChainID() == "" {
+		return nil, nil
 	}
 	}
-
-	return &releaseableLayer{mount: mountFunc, release: releaseFunc}
+	// Hold a reference to the image layer so that it can't be removed before
+	// it is released
+	roLayer, err := layerStore.Get(img.RootFS.ChainID())
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to get layer for image %s", img.ImageID())
+	}
+	return &releaseableLayer{layerStore: layerStore, roLayer: roLayer}, nil
 }
 }
 
 
 // TODO: could this use the regular daemon PullImage ?
 // TODO: could this use the regular daemon PullImage ?
@@ -75,7 +84,7 @@ func (daemon *Daemon) pullForBuilder(ctx context.Context, name string, authConfi
 
 
 	pullRegistryAuth := &types.AuthConfig{}
 	pullRegistryAuth := &types.AuthConfig{}
 	if len(authConfigs) > 0 {
 	if len(authConfigs) > 0 {
-		// The request came with a full auth config file, we prefer to use that
+		// The request came with a full auth config, use it
 		repoInfo, err := daemon.RegistryService.ResolveRepository(ref)
 		repoInfo, err := daemon.RegistryService.ResolveRepository(ref)
 		if err != nil {
 		if err != nil {
 			return nil, err
 			return nil, err
@@ -91,16 +100,23 @@ func (daemon *Daemon) pullForBuilder(ctx context.Context, name string, authConfi
 	return daemon.GetImage(name)
 	return daemon.GetImage(name)
 }
 }
 
 
-// GetImageAndLayer returns an image and releaseable layer for a reference or ID
-func (daemon *Daemon) GetImageAndLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) {
+// GetImageAndReleasableLayer returns an image and releaseable layer for a reference or ID.
+// Every call to GetImageAndReleasableLayer MUST call releasableLayer.Release() to prevent
+// leaking of layers.
+func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) {
 	if !opts.ForcePull {
 	if !opts.ForcePull {
 		image, _ := daemon.GetImage(refOrID)
 		image, _ := daemon.GetImage(refOrID)
 		// TODO: shouldn't we error out if error is different from "not found" ?
 		// TODO: shouldn't we error out if error is different from "not found" ?
 		if image != nil {
 		if image != nil {
-			return image, daemon.getReleasableLayerForImage(), nil
+			layer, err := newReleasableLayerForImage(image, daemon.layerStore)
+			return image, layer, err
 		}
 		}
 	}
 	}
 
 
 	image, err := daemon.pullForBuilder(ctx, refOrID, opts.AuthConfig, opts.Output)
 	image, err := daemon.pullForBuilder(ctx, refOrID, opts.AuthConfig, opts.Output)
-	return image, daemon.getReleasableLayerForImage(), err
+	if err != nil {
+		return nil, nil, err
+	}
+	layer, err := newReleasableLayerForImage(image, daemon.layerStore)
+	return image, layer, err
 }
 }