Browse Source

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 <yong.tang.github@outlook.com>
Yong Tang 9 years ago
parent
commit
35418c1455

+ 4 - 2
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
 	var shortImgID string
+	total := len(b.dockerfile.Children)
 	for i, n := range b.dockerfile.Children {
 	for i, n := range b.dockerfile.Children {
 		select {
 		select {
 		case <-b.clientCtx.Done():
 		case <-b.clientCtx.Done():
@@ -235,7 +236,7 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
 		default:
 		default:
 			// Not cancelled yet, keep going...
 			// 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 {
 			if b.options.ForceRemove {
 				b.clearTmp()
 				b.clearTmp()
 			}
 			}
@@ -312,8 +313,9 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con
 	b.Stderr = ioutil.Discard
 	b.Stderr = ioutil.Discard
 	b.disableCommit = true
 	b.disableCommit = true
 
 
+	total := len(ast.Children)
 	for i, n := range 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
 			return nil, err
 		}
 		}
 	}
 	}

+ 2 - 2
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
 // 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
 // deal with that, at least until it becomes more of a general concern with new
 // features.
 // 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
 	cmd := ast.Value
 	upperCasedCmd := strings.ToUpper(cmd)
 	upperCasedCmd := strings.ToUpper(cmd)
 
 
@@ -107,7 +107,7 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
 	original := ast.Original
 	original := ast.Original
 	flags := ast.Flags
 	flags := ast.Flags
 	strList := []string{}
 	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 {
 	if len(ast.Flags) > 0 {
 		msg += " " + strings.Join(ast.Flags, " ")
 		msg += " " + strings.Join(ast.Flags, " ")

+ 1 - 1
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}
 	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 {
 	if err == nil {
 		t.Fatalf("No error when executing test %s", testCase.name)
 		t.Fatalf("No error when executing test %s", testCase.name)

+ 2 - 1
builder/dockerfile/internals.go

@@ -432,6 +432,7 @@ func (b *Builder) processImageFrom(img builder.Image) error {
 			return err
 			return err
 		}
 		}
 
 
+		total := len(ast.Children)
 		for i, n := range ast.Children {
 		for i, n := range ast.Children {
 			switch strings.ToUpper(n.Value) {
 			switch strings.ToUpper(n.Value) {
 			case "ONBUILD":
 			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)
 				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
 				return err
 			}
 			}
 		}
 		}

+ 15 - 2
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)
 		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) {
 	if !strings.Contains(out, exp) {
 		c.Fatalf("Bad output\nGot:%s\n\nExpected to contain:%s\n", 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)
 		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) {
 	if !strings.Contains(out, exp) {
 		c.Fatalf("Bad output\nGot:%s\n\nExpected to contain:%s\n", 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)
 		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))
+	}
+}