From b04c6466cdc89a107879af7a22b0917006b38730 Mon Sep 17 00:00:00 2001 From: Johannes 'fish' Ziemke Date: Sat, 12 Oct 2013 14:14:52 +0200 Subject: [PATCH] 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 | 10 +- buildfile.go | 45 ++++++-- commands.go | 46 ++------ docker/docker.go | 5 +- docs/sources/api/docker_remote_api.rst | 5 + docs/sources/api/docker_remote_api_v1.7.rst | 6 +- integration/buildfile_test.go | 121 +++++++++++++++----- integration/commands_test.go | 7 +- utils/utils.go | 5 +- 9 files changed, 168 insertions(+), 82 deletions(-) diff --git a/api.go b/api.go index 1708420e12..92ab727869 100644 --- a/api.go +++ b/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 != "" { diff --git a/buildfile.go b/buildfile.go index 3075812436..15c27ecd7c 100644 --- a/buildfile.go +++ b/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, } } diff --git a/commands.go b/commands.go index 5d2ea1806c..ef95e32c33 100644 --- a/commands.go +++ b/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 { - return err - } + + headers := http.Header(make(map[string][]string)) if context != nil { - req.Header.Set("Content-Type", "application/tar") + headers.Set("Content-Type", "application/tar") } - dial, err := net.Dial(cli.proto, cli.addr) - if err != nil { - return err + err = cli.stream("POST", fmt.Sprintf("/build?%s", v.Encode()), body, cli.out, headers) + if jerr, ok := err.(*utils.JSONError); ok { + return &utils.StatusError{Status: jerr.Message, StatusCode: jerr.Code} } - clientconn := httputil.NewClientConn(dial, nil) - 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 + return err } // '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 } diff --git a/docker/docker.go b/docker/docker.go index 83df5c2171..2404ab3173 100644 --- a/docker/docker.go +++ b/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) } diff --git a/docs/sources/api/docker_remote_api.rst b/docs/sources/api/docker_remote_api.rst index d5cb1a44a9..5ef031ec03 100644 --- a/docs/sources/api/docker_remote_api.rst +++ b/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 **** diff --git a/docs/sources/api/docker_remote_api_v1.7.rst b/docs/sources/api/docker_remote_api_v1.7.rst index 251b162250..45c24376fe 100644 --- a/docs/sources/api/docker_remote_api_v1.7.rst +++ b/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, diff --git a/integration/buildfile_test.go b/integration/buildfile_test.go index 20d0450a7c..8975aa7ec1 100644 --- a/integration/buildfile_test.go +++ b/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) - if err != nil { - t.Fatal(err) - } - return img + return srv.ImageInspect(id) } 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) + } +} diff --git a/integration/commands_test.go b/integration/commands_test.go index 50cd230ce9..129a1575f8 100644 --- a/integration/commands_test.go +++ b/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) } diff --git a/utils/utils.go b/utils/utils.go index f62aa12ff5..b6857ff4d4 100644 --- a/utils/utils.go +++ b/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) {