Make docker build return exit code of build step

If a command during build fails, `docker build` now returns with
the exit code of that command.

This makes it necessary to change the build api endpoint to
return a json object stream.

api.go

@@ -960,9 +960,17 @@ func postBuild(srv *Server, version float64, w http.ResponseWriter, r *http.Requ
 		return err
 	}
 
-	b := NewBuildFile(srv, utils.NewWriteFlusher(w), !suppressOutput, !noCache, rm)
+	if version > 1.6 {
+		w.Header().Set("Content-Type", "application/json")
+	}
+	sf := utils.NewStreamFormatter(version > 1.6)
+	b := NewBuildFile(srv, utils.NewWriteFlusher(w), !suppressOutput, !noCache, rm, sf)
 	id, err := b.Build(context)
 	if err != nil {
+		if sf.Used() {
+			w.Write(sf.FormatError(err))
+			return nil
+		}
 		return fmt.Errorf("Error build: %s", err)
 	}
 	if repoName != "" {

buildfile.go

@@ -37,6 +37,7 @@ type buildFile struct {
 	tmpImages     map[string]struct{}
 
 	out io.Writer
+	sf  *utils.StreamFormatter
 }
 
 func (b *buildFile) clearTmp(containers map[string]struct{}) {
@@ -52,7 +53,7 @@ func (b *buildFile) CmdFrom(name string) error {
 	if err != nil {
 		if b.runtime.graph.IsNotExist(err) {
 			remote, tag := utils.ParseRepositoryTag(name)
-			if err := b.srv.ImagePull(remote, tag, b.out, utils.NewStreamFormatter(false), nil, nil, true); err != nil {
+			if err := b.srv.ImagePull(remote, tag, b.out, b.sf, nil, nil, true); err != nil {
 				return err
 			}
 			image, err = b.runtime.repositories.LookupImage(name)
@@ -100,7 +101,11 @@ func (b *buildFile) CmdRun(args string) error {
 		if cache, err := b.srv.ImageGetCached(b.image, b.config); err != nil {
 			return err
 		} else if cache != nil {
-			fmt.Fprintf(b.out, " ---> Using cache\n")
+			if b.sf.Used() {
+				b.out.Write(b.sf.FormatStatus("", " ---> Using cache"))
+			} else {
+				fmt.Fprintf(b.out, " ---> Using cache\n")
+			}
 			utils.Debugf("[BUILDER] Use cached version")
 			b.image = cache.ID
 			return nil
@@ -376,8 +381,11 @@ func (b *buildFile) run() (string, error) {
 		return "", err
 	}
 	b.tmpContainers[c.ID] = struct{}{}
-	fmt.Fprintf(b.out, " ---> Running in %s\n", utils.TruncateID(c.ID))
+	if b.sf.Used() {
-
+		b.out.Write(b.sf.FormatStatus("", " ---> Running in %s", utils.TruncateID(c.ID)))
+	} else {
+		fmt.Fprintf(b.out, " ---> Running in %s\n", utils.TruncateID(c.ID))
+	}
 	// override the entry point that may have been picked up from the base image
 	c.Path = b.config.Cmd[0]
 	c.Args = b.config.Cmd[1:]
@@ -403,7 +411,11 @@ func (b *buildFile) run() (string, error) {
 
 	// Wait for it to finish
 	if ret := c.Wait(); ret != 0 {
-		return "", fmt.Errorf("The command %v returned a non-zero code: %d", b.config.Cmd, ret)
+		err := &utils.JSONError{
+			Message: fmt.Sprintf("The command %v returned a non-zero code: %d", b.config.Cmd, ret),
+			Code:    ret,
+		}
+		return "", err
 	}
 
 	return c.ID, nil
@@ -424,7 +436,11 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error {
 			if cache, err := b.srv.ImageGetCached(b.image, b.config); err != nil {
 				return err
 			} else if cache != nil {
-				fmt.Fprintf(b.out, " ---> Using cache\n")
+				if b.sf.Used() {
+					b.out.Write(b.sf.FormatStatus("", " ---> Using cache"))
+				} else {
+					fmt.Fprintf(b.out, " ---> Using cache\n")
+				}
 				utils.Debugf("[BUILDER] Use cached version")
 				b.image = cache.ID
 				return nil
@@ -441,7 +457,11 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error {
 			fmt.Fprintf(b.out, " ---> [Warning] %s\n", warning)
 		}
 		b.tmpContainers[container.ID] = struct{}{}
-		fmt.Fprintf(b.out, " ---> Running in %s\n", utils.TruncateID(container.ID))
+		if b.sf.Used() {
+			b.out.Write(b.sf.FormatStatus("", " ---> Running in %s", utils.TruncateID(container.ID)))
+		} else {
+			fmt.Fprintf(b.out, " ---> Running in %s\n", utils.TruncateID(container.ID))
+		}
 		id = container.ID
 		if err := container.EnsureMounted(); err != nil {
 			return err
@@ -507,22 +527,22 @@ func (b *buildFile) Build(context io.Reader) (string, error) {
 
 		method, exists := reflect.TypeOf(b).MethodByName("Cmd" + strings.ToUpper(instruction[:1]) + strings.ToLower(instruction[1:]))
 		if !exists {
-			fmt.Fprintf(b.out, "# Skipping unknown instruction %s\n", strings.ToUpper(instruction))
+			b.out.Write(b.sf.FormatStatus("", "# Skipping unknown instruction %s", strings.ToUpper(instruction)))
 			continue
 		}
 
 		stepN += 1
-		fmt.Fprintf(b.out, "Step %d : %s %s\n", stepN, strings.ToUpper(instruction), arguments)
+		b.out.Write(b.sf.FormatStatus("", "Step %d : %s %s", stepN, strings.ToUpper(instruction), arguments))
 
 		ret := method.Func.Call([]reflect.Value{reflect.ValueOf(b), reflect.ValueOf(arguments)})[0].Interface()
 		if ret != nil {
 			return "", ret.(error)
 		}
 
-		fmt.Fprintf(b.out, " ---> %v\n", utils.TruncateID(b.image))
+		b.out.Write(b.sf.FormatStatus("", " ---> %s", utils.TruncateID(b.image)))
 	}
 	if b.image != "" {
-		fmt.Fprintf(b.out, "Successfully built %s\n", utils.TruncateID(b.image))
+		b.out.Write(b.sf.FormatStatus("", "Successfully built %s", utils.TruncateID(b.image)))
 		if b.rm {
 			b.clearTmp(b.tmpContainers)
 		}
@@ -531,7 +551,7 @@ func (b *buildFile) Build(context io.Reader) (string, error) {
 	return "", fmt.Errorf("An error occurred during the build\n")
 }
 
-func NewBuildFile(srv *Server, out io.Writer, verbose, utilizeCache, rm bool) BuildFile {
+func NewBuildFile(srv *Server, out io.Writer, verbose, utilizeCache, rm bool, sf *utils.StreamFormatter) BuildFile {
 	return &buildFile{
 		runtime:       srv.runtime,
 		srv:           srv,
@@ -542,5 +562,6 @@ func NewBuildFile(srv *Server, out io.Writer, verbose, utilizeCache, rm bool) Bu
 		verbose:       verbose,
 		utilizeCache:  utilizeCache,
 		rm:            rm,
+		sf:            sf,
 	}
 }

commands.go

@@ -220,42 +220,16 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 	if *rm {
 		v.Set("rm", "1")
 	}
-	req, err := http.NewRequest("POST", fmt.Sprintf("/v%g/build?%s", APIVERSION, v.Encode()), body)
+
-	if err != nil {
+	headers := http.Header(make(map[string][]string))
-		return err
-	}
 	if context != nil {
-		req.Header.Set("Content-Type", "application/tar")
+		headers.Set("Content-Type", "application/tar")
 	}
-	dial, err := net.Dial(cli.proto, cli.addr)
+	err = cli.stream("POST", fmt.Sprintf("/build?%s", v.Encode()), body, cli.out, headers)
-	if err != nil {
+	if jerr, ok := err.(*utils.JSONError); ok {
-		return err
+		return &utils.StatusError{Status: jerr.Message, StatusCode: jerr.Code}
 	}
-	clientconn := httputil.NewClientConn(dial, nil)
+	return err
-	resp, err := clientconn.Do(req)
-	defer clientconn.Close()
-	if err != nil {
-		return err
-	}
-	defer resp.Body.Close()
-	// Check for errors
-	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
-		body, err := ioutil.ReadAll(resp.Body)
-		if err != nil {
-			return err
-		}
-		if len(body) == 0 {
-			return fmt.Errorf("Error: %s", http.StatusText(resp.StatusCode))
-		}
-		return fmt.Errorf("Error: %s", body)
-	}
-
-	// Output the result
-	if _, err := io.Copy(cli.out, resp.Body); err != nil {
-		return err
-	}
-
-	return nil
 }
 
 // 'docker login': login / register a user to registry service.
@@ -699,7 +673,7 @@ func (cli *DockerCli) CmdInspect(args ...string) error {
 	}
 	fmt.Fprintf(cli.out, "]")
 	if status != 0 {
-		return &utils.StatusError{Status: status}
+		return &utils.StatusError{StatusCode: status}
 	}
 	return nil
 }
@@ -1584,7 +1558,7 @@ func (cli *DockerCli) CmdAttach(args ...string) error {
 		return err
 	}
 	if status != 0 {
-		return &utils.StatusError{Status: status}
+		return &utils.StatusError{StatusCode: status}
 	}
 
 	return nil
@@ -2167,7 +2141,7 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 		}
 	}
 	if status != 0 {
-		return &utils.StatusError{Status: status}
+		return &utils.StatusError{StatusCode: status}
 	}
 	return nil
 }

docker/docker.go

@@ -100,7 +100,10 @@ func main() {
 		protoAddrParts := strings.SplitN(flHosts[0], "://", 2)
 		if err := docker.ParseCommands(protoAddrParts[0], protoAddrParts[1], flag.Args()...); err != nil {
 			if sterr, ok := err.(*utils.StatusError); ok {
-				os.Exit(sterr.Status)
+				if sterr.Status != "" {
+					log.Println(sterr.Status)
+				}
+				os.Exit(sterr.StatusCode)
 			}
 			log.Fatal(err)
 		}

docs/sources/api/docker_remote_api.rst

@@ -138,6 +138,11 @@ What's new
    This URI no longer exists.  The ``images -viz`` output is now generated in
    the client, using the ``/images/json`` data.
 
+.. http:post:: /build
+
+   **New!** This endpoint now returns build status as json stream. In case
+   of a build error, it returns the exit status of the failed command.
+
 v1.6
 ****
 

docs/sources/api/docker_remote_api_v1.7.rst

@@ -990,9 +990,11 @@ Build an image from Dockerfile via stdin
    .. sourcecode:: http
 
       HTTP/1.1 200 OK
+      Content-Type: application/json
 
-      {{ STREAM }}
+      {"status":"Step 1..."}
-
+      {"status":"..."}
+      {"error":"Error...", "errorDetail":{"code": 123, "message": "Error..."}}
 
    The stream must be a tar archive compressed with one of the
    following algorithms: identity (no compression), gzip, bzip2,

integration/buildfile_test.go

@@ -5,6 +5,7 @@ import (
 	"github.com/dotcloud/docker"
 	"github.com/dotcloud/docker/archive"
 	"github.com/dotcloud/docker/engine"
+	"github.com/dotcloud/docker/utils"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -226,11 +227,14 @@ func mkTestingFileServer(files [][2]string) (*httptest.Server, error) {
 
 func TestBuild(t *testing.T) {
 	for _, ctx := range testContexts {
-		buildImage(ctx, t, nil, true)
+		_, err := buildImage(ctx, t, nil, true)
+		if err != nil {
+			t.Fatal(err)
+		}
 	}
 }
 
-func buildImage(context testContextTemplate, t *testing.T, eng *engine.Engine, useCache bool) *docker.Image {
+func buildImage(context testContextTemplate, t *testing.T, eng *engine.Engine, useCache bool) (*docker.Image, error) {
 	if eng == nil {
 		eng = NewTestEngine(t)
 		runtime := mkRuntimeFromEngine(eng, t)
@@ -262,25 +266,24 @@ func buildImage(context testContextTemplate, t *testing.T, eng *engine.Engine, u
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := docker.NewBuildFile(srv, ioutil.Discard, false, useCache, false)
+	buildfile := docker.NewBuildFile(srv, ioutil.Discard, false, useCache, false, utils.NewStreamFormatter(false))
 	id, err := buildfile.Build(mkTestContext(dockerfile, context.files, t))
 	if err != nil {
-		t.Fatal(err)
+		return nil, err
 	}
 
-	img, err := srv.ImageInspect(id)
+	return srv.ImageInspect(id)
-	if err != nil {
-		t.Fatal(err)
-	}
-	return img
 }
 
 func TestVolume(t *testing.T) {
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         volume /test
         cmd Hello world
     `, nil, nil}, t, nil, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if len(img.Config.Volumes) == 0 {
 		t.Fail()
@@ -293,10 +296,13 @@ func TestVolume(t *testing.T) {
 }
 
 func TestBuildMaintainer(t *testing.T) {
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         maintainer dockerio
     `, nil, nil}, t, nil, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if img.Author != "dockerio" {
 		t.Fail()
@@ -304,10 +310,13 @@ func TestBuildMaintainer(t *testing.T) {
 }
 
 func TestBuildUser(t *testing.T) {
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         user dockerio
     `, nil, nil}, t, nil, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if img.Config.User != "dockerio" {
 		t.Fail()
@@ -315,11 +324,15 @@ func TestBuildUser(t *testing.T) {
 }
 
 func TestBuildEnv(t *testing.T) {
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         env port 4243
         `,
 		nil, nil}, t, nil, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	hasEnv := false
 	for _, envVar := range img.Config.Env {
 		if envVar == "port=4243" {
@@ -333,11 +346,14 @@ func TestBuildEnv(t *testing.T) {
 }
 
 func TestBuildCmd(t *testing.T) {
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         cmd ["/bin/echo", "Hello World"]
         `,
 		nil, nil}, t, nil, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if img.Config.Cmd[0] != "/bin/echo" {
 		t.Log(img.Config.Cmd[0])
@@ -350,11 +366,14 @@ func TestBuildCmd(t *testing.T) {
 }
 
 func TestBuildExpose(t *testing.T) {
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         expose 4243
         `,
 		nil, nil}, t, nil, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if img.Config.PortSpecs[0] != "4243" {
 		t.Fail()
@@ -362,11 +381,14 @@ func TestBuildExpose(t *testing.T) {
 }
 
 func TestBuildEntrypoint(t *testing.T) {
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         entrypoint ["/bin/echo"]
         `,
 		nil, nil}, t, nil, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if img.Config.Entrypoint[0] != "/bin/echo" {
 	}
@@ -378,19 +400,25 @@ func TestBuildEntrypointRunCleanup(t *testing.T) {
 	eng := NewTestEngine(t)
 	defer nuke(mkRuntimeFromEngine(eng, t))
 
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
         from {IMAGE}
         run echo "hello"
         `,
 		nil, nil}, t, eng, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	img = buildImage(testContextTemplate{`
+	img, err = buildImage(testContextTemplate{`
         from {IMAGE}
         run echo "hello"
         add foo /foo
         entrypoint ["/bin/echo"]
         `,
 		[][2]string{{"foo", "HEYO"}}, nil}, t, eng, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if len(img.Config.Cmd) != 0 {
 		t.Fail()
@@ -407,11 +435,18 @@ func TestBuildImageWithCache(t *testing.T) {
         `,
 		nil, nil}
 
-	img := buildImage(template, t, eng, true)
+	img, err := buildImage(template, t, eng, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	imageId := img.ID
 
 	img = nil
-	img = buildImage(template, t, eng, true)
+	img, err = buildImage(template, t, eng, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if imageId != img.ID {
 		t.Logf("Image ids should match: %s != %s", imageId, img.ID)
@@ -429,11 +464,17 @@ func TestBuildImageWithoutCache(t *testing.T) {
         `,
 		nil, nil}
 
-	img := buildImage(template, t, eng, true)
+	img, err := buildImage(template, t, eng, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 	imageId := img.ID
 
 	img = nil
-	img = buildImage(template, t, eng, false)
+	img, err = buildImage(template, t, eng, false)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if imageId == img.ID {
 		t.Logf("Image ids should not match: %s == %s", imageId, img.ID)
@@ -475,7 +516,7 @@ func TestForbiddenContextPath(t *testing.T) {
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := docker.NewBuildFile(srv, ioutil.Discard, false, true, false)
+	buildfile := docker.NewBuildFile(srv, ioutil.Discard, false, true, false, utils.NewStreamFormatter(false))
 	_, err = buildfile.Build(mkTestContext(dockerfile, context.files, t))
 
 	if err == nil {
@@ -521,7 +562,7 @@ func TestBuildADDFileNotFound(t *testing.T) {
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := docker.NewBuildFile(mkServerFromEngine(eng, t), ioutil.Discard, false, true, false)
+	buildfile := docker.NewBuildFile(mkServerFromEngine(eng, t), ioutil.Discard, false, true, false, utils.NewStreamFormatter(false))
 	_, err = buildfile.Build(mkTestContext(dockerfile, context.files, t))
 
 	if err == nil {
@@ -539,18 +580,26 @@ func TestBuildInheritance(t *testing.T) {
 	eng := NewTestEngine(t)
 	defer nuke(mkRuntimeFromEngine(eng, t))
 
-	img := buildImage(testContextTemplate{`
+	img, err := buildImage(testContextTemplate{`
             from {IMAGE}
             expose 4243
             `,
 		nil, nil}, t, eng, true)
 
-	img2 := buildImage(testContextTemplate{fmt.Sprintf(`
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	img2, _ := buildImage(testContextTemplate{fmt.Sprintf(`
             from %s
             entrypoint ["/bin/echo"]
             `, img.ID),
 		nil, nil}, t, eng, true)
 
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	// from child
 	if img2.Config.Entrypoint[0] != "/bin/echo" {
 		t.Fail()
@@ -561,3 +610,23 @@ func TestBuildInheritance(t *testing.T) {
 		t.Fail()
 	}
 }
+
+func TestBuildFails(t *testing.T) {
+	_, err := buildImage(testContextTemplate{`
+        from {IMAGE}
+        run sh -c "exit 23"
+        `,
+		nil, nil}, t, nil, true)
+
+	if err == nil {
+		t.Fatal("Error should not be nil")
+	}
+
+	sterr, ok := err.(*utils.JSONError)
+	if !ok {
+		t.Fatalf("Error should be utils.JSONError")
+	}
+	if sterr.Code != 23 {
+		t.Fatalf("StatusCode %d unexpected, should be 23", sterr.Code)
+	}
+}

integration/commands_test.go

@@ -905,9 +905,12 @@ run    [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ]
 		nil,
 		nil,
 	}
-	image := buildImage(testBuilder, t, eng, true)
+	image, err := buildImage(testBuilder, t, eng, true)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	err := mkServerFromEngine(eng, t).ContainerTag(image.ID, "test", "latest", false)
+	err = mkServerFromEngine(eng, t).ContainerTag(image.ID, "test", "latest", false)
 	if err != nil {
 		t.Fatal(err)
 	}

utils/utils.go

@@ -1184,11 +1184,12 @@ func (graph *DependencyGraph) GenerateTraversalMap() ([][]string, error) {
 
 // An StatusError reports an unsuccessful exit by a command.
 type StatusError struct {
-	Status int
+	Status     string
+	StatusCode int
 }
 
 func (e *StatusError) Error() string {
-	return fmt.Sprintf("Status: %d", e.Status)
+	return fmt.Sprintf("Status: %s, Code: %d", e.Status, e.StatusCode)
 }
 
 func quote(word string, buf *bytes.Buffer) {