Extract imageProber and ContainerBackend from Builder

Extract a common function for builder.createContainer
Extract imageCache for doing cache probes
Removes the cacheBuested field from Builder
Create a new containerManager class which reduces the interface between the
builder and managing containers to 3 functions (from 6)

Signed-off-by: Daniel Nephin <dnephin@docker.com>

builder/builder.go

@@ -7,12 +7,11 @@ package builder
 import (
 	"io"
 
-	"golang.org/x/net/context"
-
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
 	containerpkg "github.com/docker/docker/container"
+	"golang.org/x/net/context"
 )
 
 const (
@@ -36,21 +35,10 @@ type Source interface {
 // Backend abstracts calls to a Docker Daemon.
 type Backend interface {
 	ImageBackend
+	ExecBackend
 
-	// ContainerAttachRaw attaches to container.
-	ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool, attached chan struct{}) error
-	// ContainerCreate creates a new Docker container and returns potential warnings
-	ContainerCreate(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error)
-	// ContainerRm removes a container specified by `id`.
-	ContainerRm(name string, config *types.ContainerRmConfig) error
 	// Commit creates a new Docker image from an existing Docker container.
 	Commit(string, *backend.ContainerCommitConfig) (string, error)
-	// ContainerKill stops the container execution abruptly.
-	ContainerKill(containerID string, sig uint64) error
-	// ContainerStart starts a new container
-	ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error
-	// ContainerWait stops processing until the given container is stopped.
-	ContainerWait(ctx context.Context, name string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error)
 	// ContainerCreateWorkdir creates the workdir
 	ContainerCreateWorkdir(containerID string) error
 
@@ -59,6 +47,8 @@ type Backend interface {
 	// TODO: extract in the builder instead of passing `decompress`
 	// TODO: use containerd/fs.changestream instead as a source
 	CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error
+
+	ImageCacheBuilder
 }
 
 // ImageBackend are the interface methods required from an image component
@@ -66,6 +56,22 @@ type ImageBackend interface {
 	GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (Image, ReleaseableLayer, error)
 }
 
+// ExecBackend contains the interface methods required for executing containers
+type ExecBackend interface {
+	// ContainerAttachRaw attaches to container.
+	ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool, attached chan struct{}) error
+	// ContainerCreate creates a new Docker container and returns potential warnings
+	ContainerCreate(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error)
+	// ContainerRm removes a container specified by `id`.
+	ContainerRm(name string, config *types.ContainerRmConfig) error
+	// ContainerKill stops the container execution abruptly.
+	ContainerKill(containerID string, sig uint64) error
+	// ContainerStart starts a new container
+	ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error
+	// ContainerWait stops processing until the given container is stopped.
+	ContainerWait(ctx context.Context, name string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error)
+}
+
 // Result is the output produced by a Builder
 type Result struct {
 	ImageID   string

builder/dockerfile/buildargs.go

@@ -2,8 +2,9 @@ package dockerfile
 
 import (
 	"fmt"
-	"github.com/docker/docker/runconfig/opts"
 	"io"
+
+	"github.com/docker/docker/runconfig/opts"
 )
 
 // builtinAllowedBuildArgs is list of built-in allowed build args

builder/dockerfile/buildargs_test.go

@@ -1,9 +1,9 @@
 package dockerfile
 
 import (
+	"bytes"
 	"testing"
 
-	"bytes"
 	"github.com/stretchr/testify/assert"
 )
 

builder/dockerfile/builder.go

@@ -35,8 +35,6 @@ var validCommitCommands = map[string]bool{
 	"workdir":     true,
 }
 
-var defaultLogConfig = container.LogConfig{Type: "none"}
-
 // BuildManager is shared across all Builder objects
 type BuildManager struct {
 	backend   builder.Backend
@@ -100,14 +98,13 @@ type Builder struct {
 	docker    builder.Backend
 	clientCtx context.Context
 
-	tmpContainers map[string]struct{}
-	buildStages   *buildStages
-	disableCommit bool
-	cacheBusted   bool
-	buildArgs     *buildArgs
-	imageCache    builder.ImageCache
-	imageSources  *imageSources
-	pathCache     pathCache
+	buildStages      *buildStages
+	disableCommit    bool
+	buildArgs        *buildArgs
+	imageSources     *imageSources
+	pathCache        pathCache
+	containerManager *containerManager
+	imageProber      ImageProber
 }
 
 // newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options.
@@ -117,29 +114,23 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
 		config = new(types.ImageBuildOptions)
 	}
 	b := &Builder{
-		clientCtx:     clientCtx,
-		options:       config,
-		Stdout:        options.ProgressWriter.StdoutFormatter,
-		Stderr:        options.ProgressWriter.StderrFormatter,
-		Aux:           options.ProgressWriter.AuxFormatter,
-		Output:        options.ProgressWriter.Output,
-		docker:        options.Backend,
-		tmpContainers: map[string]struct{}{},
-		buildArgs:     newBuildArgs(config.BuildArgs),
-		buildStages:   newBuildStages(),
-		imageSources:  newImageSources(clientCtx, options),
-		pathCache:     options.PathCache,
+		clientCtx:        clientCtx,
+		options:          config,
+		Stdout:           options.ProgressWriter.StdoutFormatter,
+		Stderr:           options.ProgressWriter.StderrFormatter,
+		Aux:              options.ProgressWriter.AuxFormatter,
+		Output:           options.ProgressWriter.Output,
+		docker:           options.Backend,
+		buildArgs:        newBuildArgs(config.BuildArgs),
+		buildStages:      newBuildStages(),
+		imageSources:     newImageSources(clientCtx, options),
+		pathCache:        options.PathCache,
+		imageProber:      newImageProber(options.Backend, config.CacheFrom, config.NoCache),
+		containerManager: newContainerManager(options.Backend),
 	}
 	return b
 }
 
-func (b *Builder) resetImageCache() {
-	if icb, ok := b.docker.(builder.ImageCacheBuilder); ok {
-		b.imageCache = icb.MakeImageCache(b.options.CacheFrom)
-	}
-	b.cacheBusted = false
-}
-
 // Build runs the Dockerfile builder by parsing the Dockerfile and executing
 // the instructions from the file.
 func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*builder.Result, error) {
@@ -216,14 +207,14 @@ func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result,
 		}
 		if state, err = b.dispatch(opts); err != nil {
 			if b.options.ForceRemove {
-				b.clearTmp()
+				b.containerManager.RemoveAll(b.Stdout)
 			}
 			return nil, err
 		}
 
 		fmt.Fprintf(b.Stdout, " ---> %s\n", stringid.TruncateID(state.imageID))
 		if b.options.Remove {
-			b.clearTmp()
+			b.containerManager.RemoveAll(b.Stdout)
 		}
 	}
 
@@ -258,7 +249,9 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con
 		return config, nil
 	}
 
-	b := newBuilder(context.Background(), builderOptions{})
+	b := newBuilder(context.Background(), builderOptions{
+		Options: &types.ImageBuildOptions{NoCache: true},
+	})
 
 	dockerfile, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n")))
 	if err != nil {

builder/dockerfile/containerbackend.go

@@ -0,0 +1,143 @@
+package dockerfile
+
+import (
+	"fmt"
+	"io"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/builder"
+	containerpkg "github.com/docker/docker/container"
+	"github.com/docker/docker/pkg/stringid"
+	"github.com/pkg/errors"
+	"golang.org/x/net/context"
+)
+
+type containerManager struct {
+	tmpContainers map[string]struct{}
+	backend       builder.ExecBackend
+}
+
+// newContainerManager creates a new container backend
+func newContainerManager(docker builder.ExecBackend) *containerManager {
+	return &containerManager{
+		backend:       docker,
+		tmpContainers: make(map[string]struct{}),
+	}
+}
+
+// Create a container
+func (c *containerManager) Create(runConfig *container.Config, hostConfig *container.HostConfig) (container.ContainerCreateCreatedBody, error) {
+	container, err := c.backend.ContainerCreate(types.ContainerCreateConfig{
+		Config:     runConfig,
+		HostConfig: hostConfig,
+	})
+	if err != nil {
+		return container, err
+	}
+	c.tmpContainers[container.ID] = struct{}{}
+	return container, nil
+}
+
+var errCancelled = errors.New("build cancelled")
+
+// Run a container by ID
+func (c *containerManager) Run(ctx context.Context, cID string, stdout, stderr io.Writer) (err error) {
+	attached := make(chan struct{})
+	errCh := make(chan error)
+	go func() {
+		errCh <- c.backend.ContainerAttachRaw(cID, nil, stdout, stderr, true, attached)
+	}()
+	select {
+	case err := <-errCh:
+		return err
+	case <-attached:
+	}
+
+	finished := make(chan struct{})
+	cancelErrCh := make(chan error, 1)
+	go func() {
+		select {
+		case <-ctx.Done():
+			logrus.Debugln("Build cancelled, killing and removing container:", cID)
+			c.backend.ContainerKill(cID, 0)
+			c.removeContainer(cID, stdout)
+			cancelErrCh <- errCancelled
+		case <-finished:
+			cancelErrCh <- nil
+		}
+	}()
+
+	if err := c.backend.ContainerStart(cID, nil, "", ""); err != nil {
+		close(finished)
+		logCancellationError(cancelErrCh, "error from ContainerStart: "+err.Error())
+		return err
+	}
+
+	// Block on reading output from container, stop on err or chan closed
+	if err := <-errCh; err != nil {
+		close(finished)
+		logCancellationError(cancelErrCh, "error from errCh: "+err.Error())
+		return err
+	}
+
+	waitC, err := c.backend.ContainerWait(ctx, cID, containerpkg.WaitConditionNotRunning)
+	if err != nil {
+		close(finished)
+		logCancellationError(cancelErrCh, fmt.Sprintf("unable to begin ContainerWait: %s", err))
+		return err
+	}
+
+	if status := <-waitC; status.ExitCode() != 0 {
+		close(finished)
+		logCancellationError(cancelErrCh,
+			fmt.Sprintf("a non-zero code from ContainerWait: %d", status.ExitCode()))
+		return &statusCodeError{code: status.ExitCode(), err: err}
+	}
+
+	close(finished)
+	return <-cancelErrCh
+}
+
+func logCancellationError(cancelErrCh chan error, msg string) {
+	if cancelErr := <-cancelErrCh; cancelErr != nil {
+		logrus.Debugf("Build cancelled (%v): ", cancelErr, msg)
+	}
+}
+
+type statusCodeError struct {
+	code int
+	err  error
+}
+
+func (e *statusCodeError) Error() string {
+	return e.err.Error()
+}
+
+func (e *statusCodeError) StatusCode() int {
+	return e.code
+}
+
+func (c *containerManager) removeContainer(containerID string, stdout io.Writer) error {
+	rmConfig := &types.ContainerRmConfig{
+		ForceRemove:  true,
+		RemoveVolume: true,
+	}
+	if err := c.backend.ContainerRm(containerID, rmConfig); err != nil {
+		fmt.Fprintf(stdout, "Error removing intermediate container %s: %v\n", stringid.TruncateID(containerID), err)
+		return err
+	}
+	return nil
+}
+
+// RemoveAll containers managed by this container manager
+func (c *containerManager) RemoveAll(stdout io.Writer) {
+	for containerID := range c.tmpContainers {
+		if err := c.removeContainer(containerID, stdout); err != nil {
+			return
+		}
+		delete(c.tmpContainers, containerID)
+		fmt.Fprintf(stdout, "Removing intermediate container %s\n", stringid.TruncateID(containerID))
+	}
+}

builder/dockerfile/dispatchers.go

@@ -19,11 +19,11 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api"
-	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/dockerfile/parser"
+	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/go-connections/nat"
 	"github.com/pkg/errors"
@@ -218,7 +218,7 @@ func from(req dispatchRequest) error {
 		return err
 	}
 
-	req.builder.resetImageCache()
+	req.builder.imageProber.Reset()
 	image, err := req.builder.getFromImage(req.shlex, req.args[0])
 	if err != nil {
 		return err
@@ -398,24 +398,15 @@ func workdir(req dispatchRequest) error {
 
 	comment := "WORKDIR " + runConfig.WorkingDir
 	runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment))
-	if hit, err := req.builder.probeCache(req.state, runConfigWithCommentCmd); err != nil || hit {
+	containerID, err := req.builder.probeAndCreate(req.state, runConfigWithCommentCmd)
+	if err != nil || containerID == "" {
+		return err
+	}
+	if err := req.builder.docker.ContainerCreateWorkdir(containerID); err != nil {
 		return err
 	}
 
-	container, err := req.builder.docker.ContainerCreate(types.ContainerCreateConfig{
-		Config: runConfigWithCommentCmd,
-		// Set a log config to override any default value set on the daemon
-		HostConfig: &container.HostConfig{LogConfig: defaultLogConfig},
-	})
-	if err != nil {
-		return err
-	}
-	req.builder.tmpContainers[container.ID] = struct{}{}
-	if err := req.builder.docker.ContainerCreateWorkdir(container.ID); err != nil {
-		return err
-	}
-
-	return req.builder.commitContainer(req.state, container.ID, runConfigWithCommentCmd)
+	return req.builder.commitContainer(req.state, containerID, runConfigWithCommentCmd)
 }
 
 // RUN some command yo
@@ -471,7 +462,16 @@ func run(req dispatchRequest) error {
 	if err != nil {
 		return err
 	}
-	if err := req.builder.run(cID, runConfig.Cmd); err != nil {
+	if err := req.builder.containerManager.Run(req.builder.clientCtx, cID, req.builder.Stdout, req.builder.Stderr); err != nil {
+		if err, ok := err.(*statusCodeError); ok {
+			// TODO: change error type, because jsonmessage.JSONError assumes HTTP
+			return &jsonmessage.JSONError{
+				Message: fmt.Sprintf(
+					"The command '%s' returned a non-zero code: %d",
+					strings.Join(runConfig.Cmd, " "), err.StatusCode()),
+				Code: err.StatusCode(),
+			}
+		}
 		return err
 	}
 

builder/dockerfile/dispatchers_test.go

@@ -7,6 +7,7 @@ import (
 
 	"bytes"
 	"context"
+
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
@@ -54,7 +55,6 @@ func newBuilderWithMockBackend() *Builder {
 		options:       &types.ImageBuildOptions{},
 		docker:        mockBackend,
 		buildArgs:     newBuildArgs(make(map[string]*string)),
-		tmpContainers: make(map[string]struct{}),
 		Stdout:        new(bytes.Buffer),
 		clientCtx:     ctx,
 		disableCommit: true,
@@ -62,7 +62,9 @@ func newBuilderWithMockBackend() *Builder {
 			Options: &types.ImageBuildOptions{},
 			Backend: mockBackend,
 		}),
-		buildStages: newBuildStages(),
+		buildStages:      newBuildStages(),
+		imageProber:      newImageProber(mockBackend, nil, false),
+		containerManager: newContainerManager(mockBackend),
 	}
 	return b
 }
@@ -479,9 +481,12 @@ func TestRunWithBuildArgs(t *testing.T) {
 			return "", nil
 		},
 	}
-	b.imageCache = imageCache
 
 	mockBackend := b.docker.(*MockBackend)
+	mockBackend.makeImageCacheFunc = func(_ []string) builder.ImageCache {
+		return imageCache
+	}
+	b.imageProber = newImageProber(mockBackend, nil, false)
 	mockBackend.getImageFunc = func(_ string) (builder.Image, builder.ReleaseableLayer, error) {
 		return &mockImage{
 			id:     "abcdef",

builder/dockerfile/imageprobe.go

@@ -0,0 +1,63 @@
+package dockerfile
+
+import (
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/builder"
+)
+
+// ImageProber exposes an Image cache to the Builder. It supports resetting a
+// cache.
+type ImageProber interface {
+	Reset()
+	Probe(parentID string, runConfig *container.Config) (string, error)
+}
+
+type imageProber struct {
+	cache       builder.ImageCache
+	reset       func() builder.ImageCache
+	cacheBusted bool
+}
+
+func newImageProber(cacheBuilder builder.ImageCacheBuilder, cacheFrom []string, noCache bool) ImageProber {
+	if noCache {
+		return &nopProber{}
+	}
+
+	reset := func() builder.ImageCache {
+		return cacheBuilder.MakeImageCache(cacheFrom)
+	}
+	return &imageProber{cache: reset(), reset: reset}
+}
+
+func (c *imageProber) Reset() {
+	c.cache = c.reset()
+	c.cacheBusted = false
+}
+
+// Probe checks if cache match can be found for current build instruction.
+// It returns the cachedID if there is a hit, and the empty string on miss
+func (c *imageProber) Probe(parentID string, runConfig *container.Config) (string, error) {
+	if c.cacheBusted {
+		return "", nil
+	}
+	cacheID, err := c.cache.GetCache(parentID, runConfig)
+	if err != nil {
+		return "", err
+	}
+	if len(cacheID) == 0 {
+		logrus.Debugf("[BUILDER] Cache miss: %s", runConfig.Cmd)
+		c.cacheBusted = true
+		return "", nil
+	}
+	logrus.Debugf("[BUILDER] Use cached version: %s", runConfig.Cmd)
+	return cacheID, nil
+}
+
+type nopProber struct{}
+
+func (c *nopProber) Reset() {}
+
+func (c *nopProber) Probe(_ string, _ *container.Config) (string, error) {
+	return "", nil
+}

builder/dockerfile/internals.go

@@ -9,12 +9,9 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
-	containerpkg "github.com/docker/docker/container"
-	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/pkg/errors"
 )
@@ -74,20 +71,11 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
 	runConfigWithCommentCmd := copyRunConfig(
 		state.runConfig,
 		withCmdCommentString(fmt.Sprintf("%s %s in %s ", inst.cmdName, srcHash, inst.dest)))
-	if hit, err := b.probeCache(state, runConfigWithCommentCmd); err != nil || hit {
+	containerID, err := b.probeAndCreate(state, runConfigWithCommentCmd)
+	if err != nil || containerID == "" {
 		return err
 	}
 
-	container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{
-		Config: runConfigWithCommentCmd,
-		// Set a log config to override any default value set on the daemon
-		HostConfig: &container.HostConfig{LogConfig: defaultLogConfig},
-	})
-	if err != nil {
-		return err
-	}
-	b.tmpContainers[container.ID] = struct{}{}
-
 	// Twiddle the destination when it's a relative path - meaning, make it
 	// relative to the WORKINGDIR
 	dest, err := normaliseDest(inst.cmdName, state.runConfig.WorkingDir, inst.dest)
@@ -96,11 +84,11 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
 	}
 
 	for _, info := range inst.infos {
-		if err := b.docker.CopyOnBuild(container.ID, dest, info.root, info.path, inst.allowLocalDecompression); err != nil {
+		if err := b.docker.CopyOnBuild(containerID, dest, info.root, info.path, inst.allowLocalDecompression); err != nil {
 			return err
 		}
 	}
-	return b.commitContainer(state, container.ID, runConfigWithCommentCmd)
+	return b.commitContainer(state, containerID, runConfigWithCommentCmd)
 }
 
 // For backwards compat, if there's just one info then use it as the
@@ -186,166 +174,65 @@ func getShell(c *container.Config) []string {
 	return append([]string{}, c.Shell[:]...)
 }
 
-// probeCache checks if cache match can be found for current build instruction.
-// If an image is found, probeCache returns `(true, nil)`.
-// If no image is found, it returns `(false, nil)`.
-// If there is any error, it returns `(false, err)`.
 func (b *Builder) probeCache(dispatchState *dispatchState, runConfig *container.Config) (bool, error) {
-	c := b.imageCache
-	if c == nil || b.options.NoCache || b.cacheBusted {
-		return false, nil
-	}
-	cache, err := c.GetCache(dispatchState.imageID, runConfig)
-	if err != nil {
+	cachedID, err := b.imageProber.Probe(dispatchState.imageID, runConfig)
+	if cachedID == "" || err != nil {
 		return false, err
 	}
-	if len(cache) == 0 {
-		logrus.Debugf("[BUILDER] Cache miss: %s", runConfig.Cmd)
-		b.cacheBusted = true
-		return false, nil
-	}
-
 	fmt.Fprint(b.Stdout, " ---> Using cache\n")
-	logrus.Debugf("[BUILDER] Use cached version: %s", runConfig.Cmd)
-	dispatchState.imageID = string(cache)
-	b.buildStages.update(dispatchState.imageID, runConfig)
 
+	dispatchState.imageID = string(cachedID)
+	b.buildStages.update(dispatchState.imageID, runConfig)
 	return true, nil
 }
 
+var defaultLogConfig = container.LogConfig{Type: "none"}
+
+func (b *Builder) probeAndCreate(dispatchState *dispatchState, runConfig *container.Config) (string, error) {
+	if hit, err := b.probeCache(dispatchState, runConfig); err != nil || hit {
+		return "", err
+	}
+	// Set a log config to override any default value set on the daemon
+	hostConfig := &container.HostConfig{LogConfig: defaultLogConfig}
+	container, err := b.containerManager.Create(runConfig, hostConfig)
+	return container.ID, err
+}
+
 func (b *Builder) create(runConfig *container.Config) (string, error) {
-	resources := container.Resources{
-		CgroupParent: b.options.CgroupParent,
-		CPUShares:    b.options.CPUShares,
-		CPUPeriod:    b.options.CPUPeriod,
-		CPUQuota:     b.options.CPUQuota,
-		CpusetCpus:   b.options.CPUSetCPUs,
-		CpusetMems:   b.options.CPUSetMems,
-		Memory:       b.options.Memory,
-		MemorySwap:   b.options.MemorySwap,
-		Ulimits:      b.options.Ulimits,
-	}
-
-	// TODO: why not embed a hostconfig in builder?
-	hostConfig := &container.HostConfig{
-		SecurityOpt: b.options.SecurityOpt,
-		Isolation:   b.options.Isolation,
-		ShmSize:     b.options.ShmSize,
-		Resources:   resources,
-		NetworkMode: container.NetworkMode(b.options.NetworkMode),
-		// Set a log config to override any default value set on the daemon
-		LogConfig:  defaultLogConfig,
-		ExtraHosts: b.options.ExtraHosts,
-	}
-
-	// Create the container
-	c, err := b.docker.ContainerCreate(types.ContainerCreateConfig{
-		Config:     runConfig,
-		HostConfig: hostConfig,
-	})
+	hostConfig := hostConfigFromOptions(b.options)
+	container, err := b.containerManager.Create(runConfig, hostConfig)
 	if err != nil {
 		return "", err
 	}
-	for _, warning := range c.Warnings {
+	// TODO: could this be moved into containerManager.Create() ?
+	for _, warning := range container.Warnings {
 		fmt.Fprintf(b.Stdout, " ---> [Warning] %s\n", warning)
 	}
-
-	b.tmpContainers[c.ID] = struct{}{}
-	fmt.Fprintf(b.Stdout, " ---> Running in %s\n", stringid.TruncateID(c.ID))
-	return c.ID, nil
+	fmt.Fprintf(b.Stdout, " ---> Running in %s\n", stringid.TruncateID(container.ID))
+	return container.ID, nil
 }
 
-var errCancelled = errors.New("build cancelled")
-
-func (b *Builder) run(cID string, cmd []string) (err error) {
-	attached := make(chan struct{})
-	errCh := make(chan error)
-	go func() {
-		errCh <- b.docker.ContainerAttachRaw(cID, nil, b.Stdout, b.Stderr, true, attached)
-	}()
-
-	select {
-	case err := <-errCh:
-		return err
-	case <-attached:
+func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConfig {
+	resources := container.Resources{
+		CgroupParent: options.CgroupParent,
+		CPUShares:    options.CPUShares,
+		CPUPeriod:    options.CPUPeriod,
+		CPUQuota:     options.CPUQuota,
+		CpusetCpus:   options.CPUSetCPUs,
+		CpusetMems:   options.CPUSetMems,
+		Memory:       options.Memory,
+		MemorySwap:   options.MemorySwap,
+		Ulimits:      options.Ulimits,
 	}
 
-	finished := make(chan struct{})
-	cancelErrCh := make(chan error, 1)
-	go func() {
-		select {
-		case <-b.clientCtx.Done():
-			logrus.Debugln("Build cancelled, killing and removing container:", cID)
-			b.docker.ContainerKill(cID, 0)
-			b.removeContainer(cID)
-			cancelErrCh <- errCancelled
-		case <-finished:
-			cancelErrCh <- nil
-		}
-	}()
-
-	if err := b.docker.ContainerStart(cID, nil, "", ""); err != nil {
-		close(finished)
-		if cancelErr := <-cancelErrCh; cancelErr != nil {
-			logrus.Debugf("Build cancelled (%v) and got an error from ContainerStart: %v",
-				cancelErr, err)
-		}
-		return err
-	}
-
-	// Block on reading output from container, stop on err or chan closed
-	if err := <-errCh; err != nil {
-		close(finished)
-		if cancelErr := <-cancelErrCh; cancelErr != nil {
-			logrus.Debugf("Build cancelled (%v) and got an error from errCh: %v",
-				cancelErr, err)
-		}
-		return err
-	}
-
-	waitC, err := b.docker.ContainerWait(b.clientCtx, cID, containerpkg.WaitConditionNotRunning)
-	if err != nil {
-		// Unable to begin waiting for container.
-		close(finished)
-		if cancelErr := <-cancelErrCh; cancelErr != nil {
-			logrus.Debugf("Build cancelled (%v) and unable to begin ContainerWait: %d", cancelErr, err)
-		}
-		return err
-	}
-
-	if status := <-waitC; status.ExitCode() != 0 {
-		close(finished)
-		if cancelErr := <-cancelErrCh; cancelErr != nil {
-			logrus.Debugf("Build cancelled (%v) and got a non-zero code from ContainerWait: %d", cancelErr, status.ExitCode())
-		}
-		// TODO: change error type, because jsonmessage.JSONError assumes HTTP
-		return &jsonmessage.JSONError{
-			Message: fmt.Sprintf("The command '%s' returned a non-zero code: %d", strings.Join(cmd, " "), status.ExitCode()),
-			Code:    status.ExitCode(),
-		}
-	}
-	close(finished)
-	return <-cancelErrCh
-}
-
-func (b *Builder) removeContainer(c string) error {
-	rmConfig := &types.ContainerRmConfig{
-		ForceRemove:  true,
-		RemoveVolume: true,
-	}
-	if err := b.docker.ContainerRm(c, rmConfig); err != nil {
-		fmt.Fprintf(b.Stdout, "Error removing intermediate container %s: %v\n", stringid.TruncateID(c), err)
-		return err
-	}
-	return nil
-}
-
-func (b *Builder) clearTmp() {
-	for c := range b.tmpContainers {
-		if err := b.removeContainer(c); err != nil {
-			return
-		}
-		delete(b.tmpContainers, c)
-		fmt.Fprintf(b.Stdout, "Removing intermediate container %s\n", stringid.TruncateID(c))
+	return &container.HostConfig{
+		SecurityOpt: options.SecurityOpt,
+		Isolation:   options.Isolation,
+		ShmSize:     options.ShmSize,
+		Resources:   resources,
+		NetworkMode: container.NetworkMode(options.NetworkMode),
+		// Set a log config to override any default value set on the daemon
+		LogConfig:  defaultLogConfig,
+		ExtraHosts: options.ExtraHosts,
 	}
 }

builder/dockerfile/mockbackend_test.go

@@ -3,13 +3,11 @@ package dockerfile
 import (
 	"io"
 
-	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
 	containerpkg "github.com/docker/docker/container"
-	"github.com/docker/docker/image"
 	"golang.org/x/net/context"
 )
 
@@ -18,10 +16,7 @@ type MockBackend struct {
 	containerCreateFunc func(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error)
 	commitFunc          func(string, *backend.ContainerCommitConfig) (string, error)
 	getImageFunc        func(string) (builder.Image, builder.ReleaseableLayer, error)
-}
-
-func (m *MockBackend) TagImageWithReference(image.ID, reference.Named) error {
-	return nil
+	makeImageCacheFunc  func(cacheFrom []string) builder.ImageCache
 }
 
 func (m *MockBackend) ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool, attached chan struct{}) error {
@@ -74,6 +69,13 @@ func (m *MockBackend) GetImageAndReleasableLayer(ctx context.Context, refOrID st
 	return &mockImage{id: "theid"}, &mockLayer{}, nil
 }
 
+func (m *MockBackend) MakeImageCache(cacheFrom []string) builder.ImageCache {
+	if m.makeImageCacheFunc != nil {
+		return m.makeImageCacheFunc(cacheFrom)
+	}
+	return nil
+}
+
 type mockImage struct {
 	id     string
 	config *container.Config

integration-cli/docker_cli_images_test.go

@@ -100,7 +100,7 @@ func (s *DockerSuite) TestImagesFilterLabelMatch(c *check.C) {
 }
 
 // Regression : #15659
-func (s *DockerSuite) TestImagesFilterLabelWithCommit(c *check.C) {
+func (s *DockerSuite) TestCommitWithFilterLabel(c *check.C) {
 	// Create a container
 	dockerCmd(c, "run", "--name", "bar", "busybox", "/bin/sh")
 	// Commit with labels "using changes"