diff --git a/api/client/build.go b/api/client/build.go index 5d7b9f5da3..b700884835 100644 --- a/api/client/build.go +++ b/api/client/build.go @@ -3,6 +3,7 @@ package client import ( "archive/tar" "bufio" + "bytes" "fmt" "io" "io/ioutil" @@ -42,7 +43,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { cmd := Cli.Subcmd("build", []string{"PATH | URL | -"}, Cli.DockerCommands["build"].Description, true) flTags := opts.NewListOpts(validateTag) cmd.Var(&flTags, []string{"t", "-tag"}, "Name and optionally a tag in the 'name:tag' format") - suppressOutput := cmd.Bool([]string{"q", "-quiet"}, false, "Suppress the verbose output generated by the containers") + suppressOutput := cmd.Bool([]string{"q", "-quiet"}, false, "Suppress the build output and print image ID on success") noCache := cmd.Bool([]string{"-no-cache"}, false, "Do not use cache when building the image") rm := cmd.Bool([]string{"-rm"}, true, "Remove intermediate containers after a successful build") forceRm := cmd.Bool([]string{"-force-rm"}, false, "Always remove intermediate containers") @@ -87,20 +88,32 @@ func (cli *DockerCli) CmdBuild(args ...string) error { contextDir string tempDir string relDockerfile string + progBuff io.Writer + buildBuff io.Writer ) + progBuff = cli.out + buildBuff = cli.out + if *suppressOutput { + progBuff = bytes.NewBuffer(nil) + buildBuff = bytes.NewBuffer(nil) + } + switch { case specifiedContext == "-": tempDir, relDockerfile, err = getContextFromReader(cli.in, *dockerfileName) case urlutil.IsGitURL(specifiedContext) && hasGit: tempDir, relDockerfile, err = getContextFromGitURL(specifiedContext, *dockerfileName) case urlutil.IsURL(specifiedContext): - tempDir, relDockerfile, err = getContextFromURL(cli.out, specifiedContext, *dockerfileName) + tempDir, relDockerfile, err = getContextFromURL(progBuff, specifiedContext, *dockerfileName) default: contextDir, relDockerfile, err = getContextFromLocalDir(specifiedContext, *dockerfileName) } if err != nil { + if *suppressOutput && urlutil.IsURL(specifiedContext) { + fmt.Fprintln(cli.err, progBuff) + } return fmt.Errorf("unable to prepare context: %s", err) } @@ -169,7 +182,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { context = replaceDockerfileTarWrapper(context, newDockerfile, relDockerfile) // Setup an upload progress bar - progressOutput := streamformatter.NewStreamFormatter().NewProgressOutput(cli.out, true) + progressOutput := streamformatter.NewStreamFormatter().NewProgressOutput(progBuff, true) var body io.Reader = progress.NewProgressReader(context, progressOutput, 0, "", "Sending build context to Docker daemon") @@ -230,13 +243,16 @@ func (cli *DockerCli) CmdBuild(args ...string) error { return err } - err = jsonmessage.DisplayJSONMessagesStream(response.Body, cli.out, cli.outFd, cli.isTerminalOut) + err = jsonmessage.DisplayJSONMessagesStream(response.Body, buildBuff, cli.outFd, cli.isTerminalOut) if err != nil { if jerr, ok := err.(*jsonmessage.JSONError); ok { // If no error code is set, default to 1 if jerr.Code == 0 { jerr.Code = 1 } + if *suppressOutput { + fmt.Fprintf(cli.err, "%s%s", progBuff, buildBuff) + } return Cli.StatusError{Status: jerr.Message, StatusCode: jerr.Code} } } @@ -246,6 +262,11 @@ func (cli *DockerCli) CmdBuild(args ...string) error { fmt.Fprintln(cli.err, `SECURITY WARNING: You are building a Docker image from Windows against a non-Windows Docker host. All files and directories added to build context will have '-rwxr-xr-x' permissions. It is recommended to double check and reset permissions for sensitive files and directories.`) } + // Everything worked so if -q was provided the output from the daemon + // should be just the image ID and we'll print that to stdout. + if *suppressOutput { + fmt.Fprintf(cli.out, "%s", buildBuff) + } // Since the build was successful, now we must tag any of the resolved // images from the above Dockerfile rewrite. for _, resolved := range resolvedTags { diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index c834b077a0..8b1ff8fa78 100644 --- a/api/server/router/build/build_routes.go +++ b/api/server/router/build/build_routes.go @@ -1,6 +1,7 @@ package build import ( + "bytes" "encoding/base64" "encoding/json" "errors" @@ -72,6 +73,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * authConfigs = map[string]types.AuthConfig{} authConfigsEncoded = r.Header.Get("X-Registry-Config") buildConfig = &dockerfile.Config{} + notVerboseBuffer = bytes.NewBuffer(nil) ) if authConfigsEncoded != "" { @@ -90,6 +92,9 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * defer output.Close() sf := streamformatter.NewJSONStreamFormatter() errf := func(err error) error { + if !buildConfig.Verbose && notVerboseBuffer.Len() > 0 { + output.Write(notVerboseBuffer.Bytes()) + } // Do not write the error in the http output if it's still empty. // This prevents from writing a 200(OK) when there is an internal error. if !output.Flushed() { @@ -170,6 +175,9 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * // Look at code in DetectContextFromRemoteURL for more information. createProgressReader := func(in io.ReadCloser) io.ReadCloser { progressOutput := sf.NewProgressOutput(output, true) + if !buildConfig.Verbose { + progressOutput = sf.NewProgressOutput(notVerboseBuffer, true) + } return progress.NewProgressReader(in, progressOutput, r.ContentLength, "Downloading context", remoteURL) } @@ -199,6 +207,9 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * AuthConfigs: authConfigs, Archiver: defaultArchiver, } + if !buildConfig.Verbose { + docker.OutOld = notVerboseBuffer + } b, err := dockerfile.NewBuilder(buildConfig, docker, builder.DockerIgnoreContext{ModifiableContext: context}, nil) if err != nil { @@ -206,6 +217,10 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * } b.Stdout = &streamformatter.StdoutFormatter{Writer: output, StreamFormatter: sf} b.Stderr = &streamformatter.StderrFormatter{Writer: output, StreamFormatter: sf} + if !buildConfig.Verbose { + b.Stdout = &streamformatter.StdoutFormatter{Writer: notVerboseBuffer, StreamFormatter: sf} + b.Stderr = &streamformatter.StderrFormatter{Writer: notVerboseBuffer, StreamFormatter: sf} + } if closeNotifier, ok := w.(http.CloseNotifier); ok { finished := make(chan struct{}) @@ -235,5 +250,12 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * } } + // Everything worked so if -q was provided the output from the daemon + // should be just the image ID and we'll print that to stdout. + if !buildConfig.Verbose { + stdout := &streamformatter.StdoutFormatter{Writer: output, StreamFormatter: sf} + fmt.Fprintf(stdout, "%s\n", string(imgID)) + } + return nil } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index e439516621..2af00bfe5d 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -524,11 +524,9 @@ func (b *Builder) create() (string, error) { func (b *Builder) run(cID string) (err error) { errCh := make(chan error) - if b.Verbose { - go func() { - errCh <- b.docker.ContainerAttach(cID, nil, b.Stdout, b.Stderr, true) - }() - } + go func() { + errCh <- b.docker.ContainerAttach(cID, nil, b.Stdout, b.Stderr, true) + }() finished := make(chan struct{}) defer close(finished) @@ -546,11 +544,9 @@ func (b *Builder) run(cID string) (err error) { return err } - if b.Verbose { - // Block on reading output from container, stop on err or chan closed - if err := <-errCh; err != nil { - return err - } + // Block on reading output from container, stop on err or chan closed + if err := <-errCh; err != nil { + return err } if ret, _ := b.docker.ContainerWait(cID, -1); ret != 0 { diff --git a/contrib/completion/fish/docker.fish b/contrib/completion/fish/docker.fish index 33abfd0d37..17af1c0171 100644 --- a/contrib/completion/fish/docker.fish +++ b/contrib/completion/fish/docker.fish @@ -95,7 +95,7 @@ complete -c docker -A -f -n '__fish_seen_subcommand_from build' -l force-rm -d ' complete -c docker -A -f -n '__fish_seen_subcommand_from build' -l help -d 'Print usage' complete -c docker -A -f -n '__fish_seen_subcommand_from build' -l no-cache -d 'Do not use cache when building the image' complete -c docker -A -f -n '__fish_seen_subcommand_from build' -l pull -d 'Always attempt to pull a newer version of the image' -complete -c docker -A -f -n '__fish_seen_subcommand_from build' -s q -l quiet -d 'Suppress the verbose output generated by the containers' +complete -c docker -A -f -n '__fish_seen_subcommand_from build' -s q -l quiet -d 'Suppress the build output and print image ID on success' complete -c docker -A -f -n '__fish_seen_subcommand_from build' -l rm -d 'Remove intermediate containers after a successful build' complete -c docker -A -f -n '__fish_seen_subcommand_from build' -s t -l tag -d 'Repository name (and optionally a tag) to be applied to the resulting image in case of success' diff --git a/docs/reference/commandline/build.md b/docs/reference/commandline/build.md index 88e57dce54..0219be0fbc 100644 --- a/docs/reference/commandline/build.md +++ b/docs/reference/commandline/build.md @@ -30,7 +30,7 @@ parent = "smn_cli" --memory-swap="" Total memory (memory + swap), `-1` to disable swap --no-cache=false Do not use cache when building the image --pull=false Always attempt to pull a newer version of the image - -q, --quiet=false Suppress the verbose output generated by the containers + -q, --quiet=false Suppress the build output and print image ID on success --rm=true Remove intermediate containers after a successful build --shm-size=[] Size of `/dev/shm`. The format is ``. `number` must be greater than `0`. Unit is optional and can be `b` (bytes), `k` (kilobytes), `m` (megabytes), or `g` (gigabytes). If you omit the unit, the system uses bytes. If you omit the size entirely, the system uses `64m`. -t, --tag=[] Name and optionally a tag in the 'name:tag' format diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 13001068f9..e9b557382f 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -4941,6 +4941,110 @@ func (s *DockerSuite) TestBuildLabelsCache(c *check.C) { } +func (s *DockerSuite) TestBuildNotVerboseSuccess(c *check.C) { + testRequires(c, DaemonIsLinux) + // This test makes sure that -q works correctly when build is successful: + // stdout has only the image ID (long image ID) and stderr is empty. + var stdout, stderr string + var err error + outRegexp := regexp.MustCompile("^(sha256:|)[a-z0-9]{64}\\n$") + + tt := []struct { + Name string + BuildFunc func(string) + }{ + { + Name: "quiet_build_stdin_success", + BuildFunc: func(name string) { + _, stdout, stderr, err = buildImageWithStdoutStderr(name, "FROM busybox", true, "-q", "--force-rm", "--rm") + }, + }, + { + Name: "quiet_build_ctx_success", + BuildFunc: func(name string) { + ctx, err := fakeContext("FROM busybox", map[string]string{ + "quiet_build_success_fctx": "test", + }) + if err != nil { + c.Fatalf("Failed to create context: %s", err.Error()) + } + defer ctx.Close() + _, stdout, stderr, err = buildImageFromContextWithStdoutStderr(name, ctx, true, "-q", "--force-rm", "--rm") + }, + }, + { + Name: "quiet_build_git_success", + BuildFunc: func(name string) { + git, err := newFakeGit("repo", map[string]string{ + "Dockerfile": "FROM busybox", + }, true) + if err != nil { + c.Fatalf("Failed to create the git repo: %s", err.Error()) + } + defer git.Close() + _, stdout, stderr, err = buildImageFromGitWithStdoutStderr(name, git, true, "-q", "--force-rm", "--rm") + + }, + }, + } + + for _, te := range tt { + te.BuildFunc(te.Name) + if err != nil { + c.Fatalf("Test %s shouldn't fail, but got the following error: %s", te.Name, err.Error()) + } + if outRegexp.Find([]byte(stdout)) == nil { + c.Fatalf("Test %s expected stdout to match the [%v] regexp, but it is [%v]", te.Name, outRegexp, stdout) + } + if stderr != "" { + c.Fatalf("Test %s expected stderr to be empty, but it is [%#v]", te.Name, stderr) + } + } + +} + +func (s *DockerSuite) TestBuildNotVerboseFailure(c *check.C) { + testRequires(c, DaemonIsLinux) + // This test makes sure that -q works correctly when build fails by + // comparing between the stderr output in quiet mode and in stdout + // and stderr output in verbose mode + tt := []struct { + TestName string + BuildCmds string + }{ + {"quiet_build_no_from_at_the_beginning", "RUN whoami"}, + {"quiet_build_unknown_instr", "FROMD busybox"}, + {"quiet_build_not_exists_image", "FROM busybox11"}, + } + + for _, te := range tt { + _, _, qstderr, qerr := buildImageWithStdoutStderr(te.TestName, te.BuildCmds, false, "-q", "--force-rm", "--rm") + _, vstdout, vstderr, verr := buildImageWithStdoutStderr(te.TestName, te.BuildCmds, false, "--force-rm", "--rm") + if verr == nil || qerr == nil { + c.Fatal(fmt.Errorf("Test [%s] expected to fail but didn't", te.TestName)) + } + if qstderr != vstdout+vstderr { + c.Fatal(fmt.Errorf("Test[%s] expected that quiet stderr and verbose stdout are equal; quiet [%v], verbose [%v]", te.TestName, qstderr, vstdout)) + } + } +} + +func (s *DockerSuite) TestBuildNotVerboseFailureRemote(c *check.C) { + testRequires(c, DaemonIsLinux) + // This test ensures that when given a wrong URL, stderr in quiet mode and + // stdout and stderr in verbose mode are identical. + URL := "http://bla.bla.com" + Name := "quiet_build_wrong_remote" + _, _, qstderr, qerr := buildImageWithStdoutStderr(Name, "", false, "-q", "--force-rm", "--rm", URL) + _, vstdout, vstderr, verr := buildImageWithStdoutStderr(Name, "", false, "--force-rm", "--rm", URL) + if qerr == nil || verr == nil { + c.Fatal(fmt.Errorf("Test [%s] expected to fail but didn't", Name)) + } + if qstderr != vstdout+vstderr { + c.Fatal(fmt.Errorf("Test[%s] expected that quiet stderr and verbose stdout are equal; quiet [%v], verbose [%v]", Name, qstderr, vstdout)) + } +} + func (s *DockerSuite) TestBuildStderr(c *check.C) { testRequires(c, DaemonIsLinux) // This test just makes sure that no non-error output goes @@ -5589,34 +5693,6 @@ func (s *DockerSuite) TestBuildDotDotFile(c *check.C) { } } -func (s *DockerSuite) TestBuildNotVerbose(c *check.C) { - testRequires(c, DaemonIsLinux) - ctx, err := fakeContext("FROM busybox\nENV abc=hi\nRUN echo $abc there", map[string]string{}) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - // First do it w/verbose - baseline - out, _, err := dockerCmdInDir(c, ctx.Dir, "build", "--no-cache", "-t", "verbose", ".") - if err != nil { - c.Fatalf("failed to build the image w/o -q: %s, %v", out, err) - } - if !strings.Contains(out, "hi there") { - c.Fatalf("missing output:%s\n", out) - } - - // Now do it w/o verbose - out, _, err = dockerCmdInDir(c, ctx.Dir, "build", "--no-cache", "-q", "-t", "verbose", ".") - if err != nil { - c.Fatalf("failed to build the image w/ -q: %s, %v", out, err) - } - if strings.Contains(out, "hi there") { - c.Fatalf("Bad output, should not contain 'hi there':%s", out) - } - -} - func (s *DockerSuite) TestBuildRUNoneJSON(c *check.C) { testRequires(c, DaemonIsLinux) name := "testbuildrunonejson" diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index 5580032641..c40a52931d 100644 --- a/integration-cli/docker_utils.go +++ b/integration-cli/docker_utils.go @@ -1308,6 +1308,47 @@ func buildImageFromContextWithOut(name string, ctx *FakeContext, useCache bool, return id, out, nil } +func buildImageFromContextWithStdoutStderr(name string, ctx *FakeContext, useCache bool, buildFlags ...string) (string, string, string, error) { + args := []string{"build", "-t", name} + if !useCache { + args = append(args, "--no-cache") + } + args = append(args, buildFlags...) + args = append(args, ".") + buildCmd := exec.Command(dockerBinary, args...) + buildCmd.Dir = ctx.Dir + + stdout, stderr, exitCode, err := runCommandWithStdoutStderr(buildCmd) + if err != nil || exitCode != 0 { + return "", stdout, stderr, fmt.Errorf("failed to build the image: %s", stdout) + } + id, err := getIDByName(name) + if err != nil { + return "", stdout, stderr, err + } + return id, stdout, stderr, nil +} + +func buildImageFromGitWithStdoutStderr(name string, ctx *fakeGit, useCache bool, buildFlags ...string) (string, string, string, error) { + args := []string{"build", "-t", name} + if !useCache { + args = append(args, "--no-cache") + } + args = append(args, buildFlags...) + args = append(args, ctx.RepoURL) + buildCmd := exec.Command(dockerBinary, args...) + + stdout, stderr, exitCode, err := runCommandWithStdoutStderr(buildCmd) + if err != nil || exitCode != 0 { + return "", stdout, stderr, fmt.Errorf("failed to build the image: %s", stdout) + } + id, err := getIDByName(name) + if err != nil { + return "", stdout, stderr, err + } + return id, stdout, stderr, nil +} + func buildImageFromPath(name, path string, useCache bool, buildFlags ...string) (string, error) { args := []string{"build", "-t", name} if !useCache { diff --git a/man/docker-build.1.md b/man/docker-build.1.md index 4a87c4d515..52451c1d75 100644 --- a/man/docker-build.1.md +++ b/man/docker-build.1.md @@ -81,7 +81,7 @@ set as the **URL**, the repository is cloned locally and then sent as the contex Always attempt to pull a newer version of the image. The default is *false*. **-q**, **--quiet**=*true*|*false* - Suppress the verbose output generated by the containers. The default is *false*. + Suppress the build output and print image ID on success. The default is *false*. **--rm**=*true*|*false* Remove intermediate containers after a successful build. The default is *true*.