From 48c6a8f86f8bc30951e2da899434186e28225a90 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sun, 24 Jul 2016 01:06:10 -0700 Subject: [PATCH] Add hint of progress to the output of `docker build` This fix tries to address the issue raised in 24912 where docker build only consists of the current step without overall total steps. This fix adds the overall total steps so that end user could follow the progress of the docker build. An additonal test has been added to cover the changes. This fix fixes 24912. Signed-off-by: Yong Tang Upstream-commit: 35418c145518c3f816ae5837beda7d853ce96dfc Component: engine --- components/engine/builder/dockerfile/builder.go | 6 ++++-- .../engine/builder/dockerfile/evaluator.go | 4 ++-- .../engine/builder/dockerfile/evaluator_test.go | 2 +- .../engine/builder/dockerfile/internals.go | 3 ++- .../integration-cli/docker_cli_build_test.go | 17 +++++++++++++++-- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/components/engine/builder/dockerfile/builder.go b/components/engine/builder/dockerfile/builder.go index a7f96c6f13..1b730249c0 100644 --- a/components/engine/builder/dockerfile/builder.go +++ b/components/engine/builder/dockerfile/builder.go @@ -226,6 +226,7 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri } var shortImgID string + total := len(b.dockerfile.Children) for i, n := range b.dockerfile.Children { select { case <-b.clientCtx.Done(): @@ -235,7 +236,7 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri default: // Not cancelled yet, keep going... } - if err := b.dispatch(i, n); err != nil { + if err := b.dispatch(i, total, n); err != nil { if b.options.ForceRemove { b.clearTmp() } @@ -312,8 +313,9 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con b.Stderr = ioutil.Discard b.disableCommit = true + total := len(ast.Children) for i, n := range ast.Children { - if err := b.dispatch(i, n); err != nil { + if err := b.dispatch(i, total, n); err != nil { return nil, err } } diff --git a/components/engine/builder/dockerfile/evaluator.go b/components/engine/builder/dockerfile/evaluator.go index 4c9d425f9b..cdecf05a3b 100644 --- a/components/engine/builder/dockerfile/evaluator.go +++ b/components/engine/builder/dockerfile/evaluator.go @@ -93,7 +93,7 @@ func init() { // such as `RUN` in ONBUILD RUN foo. There is special case logic in here to // deal with that, at least until it becomes more of a general concern with new // features. -func (b *Builder) dispatch(stepN int, ast *parser.Node) error { +func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error { cmd := ast.Value upperCasedCmd := strings.ToUpper(cmd) @@ -107,7 +107,7 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { original := ast.Original flags := ast.Flags strList := []string{} - msg := fmt.Sprintf("Step %d : %s", stepN+1, upperCasedCmd) + msg := fmt.Sprintf("Step %d/%d : %s", stepN+1, stepTotal, upperCasedCmd) if len(ast.Flags) > 0 { msg += " " + strings.Join(ast.Flags, " ") diff --git a/components/engine/builder/dockerfile/evaluator_test.go b/components/engine/builder/dockerfile/evaluator_test.go index 5b9f7ec8ec..55cf4078cc 100644 --- a/components/engine/builder/dockerfile/evaluator_test.go +++ b/components/engine/builder/dockerfile/evaluator_test.go @@ -182,7 +182,7 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) { b := &Builder{runConfig: config, options: options, Stdout: ioutil.Discard, context: context} - err = b.dispatch(0, n.Children[0]) + err = b.dispatch(0, len(n.Children), n.Children[0]) if err == nil { t.Fatalf("No error when executing test %s", testCase.name) diff --git a/components/engine/builder/dockerfile/internals.go b/components/engine/builder/dockerfile/internals.go index 50755f2642..441f38b63e 100644 --- a/components/engine/builder/dockerfile/internals.go +++ b/components/engine/builder/dockerfile/internals.go @@ -432,6 +432,7 @@ func (b *Builder) processImageFrom(img builder.Image) error { return err } + total := len(ast.Children) for i, n := range ast.Children { switch strings.ToUpper(n.Value) { case "ONBUILD": @@ -440,7 +441,7 @@ func (b *Builder) processImageFrom(img builder.Image) error { return fmt.Errorf("%s isn't allowed as an ONBUILD trigger", n.Value) } - if err := b.dispatch(i, n); err != nil { + if err := b.dispatch(i, total, n); err != nil { return err } } diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index 07b0ea5a31..7820545e4d 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -5402,7 +5402,7 @@ func (s *DockerSuite) TestBuildNoDupOutput(c *check.C) { c.Fatalf("Build should have worked: %q", err) } - exp := "\nStep 2 : RUN env\n" + exp := "\nStep 2/2 : RUN env\n" if !strings.Contains(out, exp) { c.Fatalf("Bad output\nGot:%s\n\nExpected to contain:%s\n", out, exp) } @@ -5419,7 +5419,7 @@ func (s *DockerSuite) TestBuildStartsFromOne(c *check.C) { c.Fatalf("Build should have worked: %q", err) } - exp := "\nStep 1 : FROM busybox\n" + exp := "\nStep 1/1 : FROM busybox\n" if !strings.Contains(out, exp) { c.Fatalf("Bad output\nGot:%s\n\nExpected to contain:%s\n", out, exp) } @@ -6967,3 +6967,16 @@ func (s *DockerSuite) TestBuildCmdShellArgsEscaped(c *check.C) { c.Fatalf("CMD was not escaped Config.Cmd: got %v", res) } } + +// Test case for #24912. +func (s *DockerSuite) TestBuildStepsWithProgress(c *check.C) { + name := "testbuildstepswithprogress" + + totalRun := 5 + _, out, err := buildImageWithOut(name, "FROM busybox\n"+strings.Repeat("RUN echo foo\n", totalRun), true) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Contains, fmt.Sprintf("Step 1/%d : FROM busybox", 1+totalRun)) + for i := 2; i <= 1+totalRun; i++ { + c.Assert(out, checker.Contains, fmt.Sprintf("Step %d/%d : RUN echo foo", i, 1+totalRun)) + } +}