Merge pull request #5839 from unclejack/improve_build_rm

add --force-rm to clean up after a failed build

api/client/commands.go

@@ -110,6 +110,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 	suppressOutput := cmd.Bool([]string{"q", "-quiet"}, false, "Suppress the verbose output generated by the containers")
 	noCache := cmd.Bool([]string{"#no-cache", "-no-cache"}, false, "Do not use cache when building the image")
 	rm := cmd.Bool([]string{"#rm", "-rm"}, true, "Remove intermediate containers after a successful build")
+	forceRm := cmd.Bool([]string{"-force-rm"}, false, "Always remove intermediate containers, even after unsuccessful builds")
 	if err := cmd.Parse(args); err != nil {
 		return nil
 	}
@@ -197,6 +198,12 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 	}
 	if *rm {
 		v.Set("rm", "1")
+	} else {
+		v.Set("rm", "0")
+	}
+
+	if *forceRm {
+		v.Set("forcerm", "1")
 	}
 
 	cli.LoadConfigFile()

api/common.go

@@ -11,7 +11,7 @@ import (
 )
 
 const (
-	APIVERSION        version.Version = "1.11"
+	APIVERSION        version.Version = "1.12"
 	DEFAULTHTTPHOST                   = "127.0.0.1"
 	DEFAULTUNIXSOCKET                 = "/var/run/docker.sock"
 )

api/server/server.go

@@ -901,12 +901,20 @@ func postBuild(eng *engine.Engine, version version.Version, w http.ResponseWrite
 	} else {
 		job.Stdout.Add(utils.NewWriteFlusher(w))
 	}
+
+	if r.FormValue("forcerm") == "1" && version.GreaterThanOrEqualTo("1.12") {
+		job.Setenv("rm", "1")
+	} else if r.FormValue("rm") == "" && version.GreaterThanOrEqualTo("1.12") {
+		job.Setenv("rm", "1")
+	} else {
+		job.Setenv("rm", r.FormValue("rm"))
+	}
 	job.Stdin.Add(r.Body)
 	job.Setenv("remote", r.FormValue("remote"))
 	job.Setenv("t", r.FormValue("t"))
 	job.Setenv("q", r.FormValue("q"))
 	job.Setenv("nocache", r.FormValue("nocache"))
-	job.Setenv("rm", r.FormValue("rm"))
+	job.Setenv("forcerm", r.FormValue("forcerm"))
 	job.SetenvJson("authConfig", authConfig)
 	job.SetenvJson("configFile", configFile)
 

docs/sources/reference/api/docker_remote_api.md

@@ -20,13 +20,23 @@ page_keywords: API, Docker, rcli, REST, documentation
 
 
 
-The current version of the API is v1.11
+The current version of the API is v1.12
 
 Calling /images/<name>/insert is the same as calling
-/v1.11/images/<name>/insert
+/v1.12/images/<name>/insert
 
 You can still call an old version of the api using
-/v1.11/images/<name>/insert
+/v1.12/images/<name>/insert
+
+## v1.12
+
+### Full Documentation
+
+[*Docker Remote API v1.12*](/reference/api/docker_remote_api_v1.12/)
+
+### What's new
+
+docker build now has support for the `forcerm` parameter to always remove containers
 
 ## v1.11
 

docs/sources/reference/api/docker_remote_api_v1.10.md

@@ -1023,6 +1023,7 @@ Build an image from Dockerfile via stdin
         the resulting image in case of success
     -   **q** – suppress verbose build output
     -   **nocache** – do not use the cache when building the image
+    -   **rm** - remove intermediate containers after a successful build
 
     Request Headers:
 

docs/sources/reference/api/docker_remote_api_v1.11.md

@@ -1063,6 +1063,7 @@ Build an image from Dockerfile via stdin
         the resulting image in case of success
     -   **q** – suppress verbose build output
     -   **nocache** – do not use the cache when building the image
+    -   **rm** - remove intermediate containers after a successful build
 
     Request Headers:
 

docs/sources/reference/commandline/cli.md

@@ -192,6 +192,7 @@ To kill the container, use `docker kill`.
 
     Build a new container image from the source code at PATH
 
+      --force-rm=false     Always remove intermediate containers, even after unsuccessful builds
       --no-cache=false     Do not use cache when building the image
       -q, --quiet=false    Suppress the verbose output generated by the containers
       --rm=true            Remove intermediate containers after a successful build

integration-cli/build_tests/TestBuildForceRm/Dockerfile

@@ -0,0 +1,3 @@
+FROM busybox
+RUN true
+RUN thiswillfail

integration-cli/build_tests/TestBuildRm/Dockerfile

@@ -0,0 +1,4 @@
+FROM busybox
+ADD foo /
+ADD foo /
+

integration-cli/build_tests/TestBuildRm/foo

@@ -0,0 +1 @@
+bar

integration-cli/docker_cli_build_test.go

@@ -264,6 +264,118 @@ func TestBuildWithInaccessibleFilesInContext(t *testing.T) {
 	logDone("build - ADD from context with accessible links must work")
 }
 
+func TestBuildForceRm(t *testing.T) {
+	containerCountBefore, err := getContainerCount()
+	if err != nil {
+		t.Fatalf("failed to get the container count: %s", err)
+	}
+
+	buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestBuildForceRm")
+	buildCmd := exec.Command(dockerBinary, "build", "--force-rm", ".")
+	buildCmd.Dir = buildDirectory
+	_, exitCode, err := runCommandWithOutput(buildCmd)
+
+	if err == nil || exitCode == 0 {
+		t.Fatal("failed to build the image")
+	}
+
+	containerCountAfter, err := getContainerCount()
+	if err != nil {
+		t.Fatalf("failed to get the container count: %s", err)
+	}
+
+	if containerCountBefore != containerCountAfter {
+		t.Fatalf("--force-rm shouldn't have left containers behind")
+	}
+
+	logDone("build - ensure --force-rm doesn't leave containers behind")
+}
+
+func TestBuildRm(t *testing.T) {
+	{
+		containerCountBefore, err := getContainerCount()
+		if err != nil {
+			t.Fatalf("failed to get the container count: %s", err)
+		}
+
+		buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestBuildRm")
+		buildCmd := exec.Command(dockerBinary, "build", "--rm", "-t", "testbuildrm", ".")
+		buildCmd.Dir = buildDirectory
+		_, exitCode, err := runCommandWithOutput(buildCmd)
+
+		if err != nil || exitCode != 0 {
+			t.Fatal("failed to build the image")
+		}
+
+		containerCountAfter, err := getContainerCount()
+		if err != nil {
+			t.Fatalf("failed to get the container count: %s", err)
+		}
+
+		if containerCountBefore != containerCountAfter {
+			t.Fatalf("-rm shouldn't have left containers behind")
+		}
+		deleteImages("testbuildrm")
+	}
+
+	{
+		containerCountBefore, err := getContainerCount()
+		if err != nil {
+			t.Fatalf("failed to get the container count: %s", err)
+		}
+
+		buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestBuildRm")
+		buildCmd := exec.Command(dockerBinary, "build", "-t", "testbuildrm", ".")
+		buildCmd.Dir = buildDirectory
+		_, exitCode, err := runCommandWithOutput(buildCmd)
+
+		if err != nil || exitCode != 0 {
+			t.Fatal("failed to build the image")
+		}
+
+		containerCountAfter, err := getContainerCount()
+		if err != nil {
+			t.Fatalf("failed to get the container count: %s", err)
+		}
+
+		if containerCountBefore != containerCountAfter {
+			t.Fatalf("--rm shouldn't have left containers behind")
+		}
+		deleteImages("testbuildrm")
+	}
+
+	{
+		containerCountBefore, err := getContainerCount()
+		if err != nil {
+			t.Fatalf("failed to get the container count: %s", err)
+		}
+
+		buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestBuildRm")
+		buildCmd := exec.Command(dockerBinary, "build", "--rm=false", "-t", "testbuildrm", ".")
+		buildCmd.Dir = buildDirectory
+		_, exitCode, err := runCommandWithOutput(buildCmd)
+
+		if err != nil || exitCode != 0 {
+			t.Fatal("failed to build the image")
+		}
+
+		containerCountAfter, err := getContainerCount()
+		if err != nil {
+			t.Fatalf("failed to get the container count: %s", err)
+		}
+
+		if containerCountBefore == containerCountAfter {
+			t.Fatalf("--rm=false should have left containers behind")
+		}
+		deleteAllContainers()
+		deleteImages("testbuildrm")
+
+	}
+
+	logDone("build - ensure --rm doesn't leave containers behind and that --rm=true is the default")
+	logDone("build - ensure --rm=false overrides the default")
+}
+
 // TODO: TestCaching
 
 // TODO: TestADDCacheInvalidation

integration-cli/docker_utils.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"os/exec"
+	"strconv"
 	"strings"
 	"testing"
 )
@@ -71,3 +72,28 @@ func findContainerIp(t *testing.T, id string) string {
 
 	return strings.Trim(out, " \r\n'")
 }
+
+func getContainerCount() (int, error) {
+	const containers = "Containers:"
+
+	cmd := exec.Command(dockerBinary, "info")
+	out, _, err := runCommandWithOutput(cmd)
+	if err != nil {
+		return 0, err
+	}
+
+	lines := strings.Split(out, "\n")
+	for _, line := range lines {
+		if strings.Contains(line, containers) {
+			output := stripTrailingCharacters(line)
+			output = strings.TrimLeft(output, containers)
+			output = strings.Trim(output, " ")
+			containerCount, err := strconv.Atoi(output)
+			if err != nil {
+				return 0, err
+			}
+			return containerCount, nil
+		}
+	}
+	return 0, fmt.Errorf("couldn't find the Container count in the output")
+}

integration/buildfile_test.go

@@ -397,7 +397,7 @@ func buildImage(context testContextTemplate, t *testing.T, eng *engine.Engine, u
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := server.NewBuildFile(srv, ioutil.Discard, ioutil.Discard, false, useCache, false, ioutil.Discard, utils.NewStreamFormatter(false), nil, nil)
+	buildfile := server.NewBuildFile(srv, ioutil.Discard, ioutil.Discard, false, useCache, false, false, ioutil.Discard, utils.NewStreamFormatter(false), nil, nil)
 	id, err := buildfile.Build(context.Archive(dockerfile, t))
 	if err != nil {
 		return nil, err
@@ -839,7 +839,7 @@ func TestForbiddenContextPath(t *testing.T) {
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := server.NewBuildFile(srv, ioutil.Discard, ioutil.Discard, false, true, false, ioutil.Discard, utils.NewStreamFormatter(false), nil, nil)
+	buildfile := server.NewBuildFile(srv, ioutil.Discard, ioutil.Discard, false, true, false, false, ioutil.Discard, utils.NewStreamFormatter(false), nil, nil)
 	_, err = buildfile.Build(context.Archive(dockerfile, t))
 
 	if err == nil {
@@ -885,7 +885,7 @@ func TestBuildADDFileNotFound(t *testing.T) {
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := server.NewBuildFile(mkServerFromEngine(eng, t), ioutil.Discard, ioutil.Discard, false, true, false, ioutil.Discard, utils.NewStreamFormatter(false), nil, nil)
+	buildfile := server.NewBuildFile(mkServerFromEngine(eng, t), ioutil.Discard, ioutil.Discard, false, true, false, false, ioutil.Discard, utils.NewStreamFormatter(false), nil, nil)
 	_, err = buildfile.Build(context.Archive(dockerfile, t))
 
 	if err == nil {

server/buildfile.go

@@ -52,6 +52,7 @@ type buildFile struct {
 	verbose      bool
 	utilizeCache bool
 	rm           bool
+	forceRm      bool
 
 	authConfig *registry.AuthConfig
 	configFile *registry.ConfigFile
@@ -817,6 +818,9 @@ func (b *buildFile) Build(context io.Reader) (string, error) {
 			continue
 		}
 		if err := b.BuildStep(fmt.Sprintf("%d", stepN), line); err != nil {
+			if b.forceRm {
+				b.clearTmp(b.tmpContainers)
+			}
 			return "", err
 		} else if b.rm {
 			b.clearTmp(b.tmpContainers)
@@ -869,7 +873,7 @@ func stripComments(raw []byte) string {
 	return strings.Join(out, "\n")
 }
 
-func NewBuildFile(srv *Server, outStream, errStream io.Writer, verbose, utilizeCache, rm bool, outOld io.Writer, sf *utils.StreamFormatter, auth *registry.AuthConfig, authConfigFile *registry.ConfigFile) BuildFile {
+func NewBuildFile(srv *Server, outStream, errStream io.Writer, verbose, utilizeCache, rm bool, forceRm bool, outOld io.Writer, sf *utils.StreamFormatter, auth *registry.AuthConfig, authConfigFile *registry.ConfigFile) BuildFile {
 	return &buildFile{
 		daemon:        srv.daemon,
 		srv:           srv,
@@ -881,6 +885,7 @@ func NewBuildFile(srv *Server, outStream, errStream io.Writer, verbose, utilizeC
 		verbose:       verbose,
 		utilizeCache:  utilizeCache,
 		rm:            rm,
+		forceRm:       forceRm,
 		sf:            sf,
 		authConfig:    auth,
 		configFile:    authConfigFile,

server/server.go

@@ -424,6 +424,7 @@ func (srv *Server) Build(job *engine.Job) engine.Status {
 		suppressOutput = job.GetenvBool("q")
 		noCache        = job.GetenvBool("nocache")
 		rm             = job.GetenvBool("rm")
+		forceRm        = job.GetenvBool("forcerm")
 		authConfig     = &registry.AuthConfig{}
 		configFile     = &registry.ConfigFile{}
 		tag            string
@@ -482,7 +483,7 @@ func (srv *Server) Build(job *engine.Job) engine.Status {
 			Writer:          job.Stdout,
 			StreamFormatter: sf,
 		},
-		!suppressOutput, !noCache, rm, job.Stdout, sf, authConfig, configFile)
+		!suppressOutput, !noCache, rm, forceRm, job.Stdout, sf, authConfig, configFile)
 	id, err := b.Build(context)
 	if err != nil {
 		return job.Error(err)