Merge pull request #32600 from dnephin/refactor-builder-dispatch-state

Refactor builder dispatch state

builder/dockerfile/bflag.go

@@ -37,6 +37,13 @@ func NewBFlags() *BFlags {
 	}
 }
 
+// NewBFlagsWithArgs returns the new BFlags struct with Args set to args
+func NewBFlagsWithArgs(args []string) *BFlags {
+	flags := NewBFlags()
+	flags.Args = args
+	return flags
+}
+
 // AddBool adds a bool flag to BFlags
 // Note, any error will be generated when Parse() is called (see Parse).
 func (bf *BFlags) AddBool(name string, def bool) *Flag {

builder/dockerfile/builder.go

@@ -52,20 +52,20 @@ type Builder struct {
 	clientCtx context.Context
 
 	runConfig     *container.Config // runconfig for cmd, run, entrypoint etc.
-	flags         *BFlags
 	tmpContainers map[string]struct{}
-	image         string         // imageID
 	imageContexts *imageContexts // helper for storing contexts from builds
-	noBaseImage   bool           // A flag to track the use of `scratch` as the base image
-	maintainer    string
-	cmdSet        bool
 	disableCommit bool
 	cacheBusted   bool
 	buildArgs     *buildArgs
-	escapeToken   rune
-
-	imageCache builder.ImageCache
-	from       builder.Image
+	imageCache    builder.ImageCache
+
+	// TODO: these move to DispatchState
+	escapeToken rune
+	maintainer  string
+	cmdSet      bool
+	noBaseImage bool   // A flag to track the use of `scratch` as the base image
+	image       string // imageID
+	from        builder.Image
 }
 
 // BuildManager implements builder.Backend and is shared across all Builder objects.
@@ -196,28 +196,28 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
 		return "", err
 	}
 
-	shortImageID, err := b.dispatchDockerfileWithCancellation(dockerfile)
+	imageID, err := b.dispatchDockerfileWithCancellation(dockerfile)
 	if err != nil {
 		return "", err
 	}
 
 	b.warnOnUnusedBuildArgs()
 
-	if b.image == "" {
+	if imageID == "" {
 		return "", errors.New("No image was generated. Is your Dockerfile empty?")
 	}
 
 	if b.options.Squash {
-		if err := b.squashBuild(); err != nil {
+		if imageID, err = b.squashBuild(imageID); err != nil {
 			return "", err
 		}
 	}
 
-	fmt.Fprintf(b.Stdout, "Successfully built %s\n", shortImageID)
-	if err := b.tagImages(repoAndTags); err != nil {
+	fmt.Fprintf(b.Stdout, "Successfully built %s\n", stringid.TruncateID(imageID))
+	if err := b.tagImages(imageID, repoAndTags); err != nil {
 		return "", err
 	}
-	return b.image, nil
+	return imageID, nil
 }
 
 func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result) (string, error) {
@@ -225,7 +225,7 @@ func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result)
 	b.escapeToken = dockerfile.EscapeToken
 
 	total := len(dockerfile.AST.Children)
-	var shortImgID string
+	var imageID string
 	for i, n := range dockerfile.AST.Children {
 		select {
 		case <-b.clientCtx.Done():
@@ -247,8 +247,10 @@ func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result)
 			return "", err
 		}
 
-		shortImgID = stringid.TruncateID(b.image)
-		fmt.Fprintf(b.Stdout, " ---> %s\n", shortImgID)
+		// TODO: get this from dispatch
+		imageID = b.image
+
+		fmt.Fprintf(b.Stdout, " ---> %s\n", stringid.TruncateID(imageID))
 		if b.options.Remove {
 			b.clearTmp()
 		}
@@ -258,20 +260,20 @@ func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result)
 		return "", errors.Errorf("failed to reach build target %s in Dockerfile", b.options.Target)
 	}
 
-	return shortImgID, nil
+	return imageID, nil
 }
 
-func (b *Builder) squashBuild() error {
+func (b *Builder) squashBuild(imageID string) (string, error) {
 	var fromID string
 	var err error
 	if b.from != nil {
 		fromID = b.from.ImageID()
 	}
-	b.image, err = b.docker.SquashImage(b.image, fromID)
+	imageID, err = b.docker.SquashImage(imageID, fromID)
 	if err != nil {
-		return errors.Wrap(err, "error squashing image")
+		return "", errors.Wrap(err, "error squashing image")
 	}
-	return nil
+	return imageID, nil
 }
 
 func addNodesForLabelOption(dockerfile *parser.Node, labels map[string]string) {
@@ -292,8 +294,8 @@ func (b *Builder) warnOnUnusedBuildArgs() {
 	}
 }
 
-func (b *Builder) tagImages(repoAndTags []reference.Named) error {
-	imageID := image.ID(b.image)
+func (b *Builder) tagImages(id string, repoAndTags []reference.Named) error {
+	imageID := image.ID(id)
 	for _, rt := range repoAndTags {
 		if err := b.docker.TagImageWithReference(imageID, rt); err != nil {
 			return err
@@ -304,6 +306,7 @@ func (b *Builder) tagImages(repoAndTags []reference.Named) error {
 }
 
 // hasFromImage returns true if the builder has processed a `FROM <image>` line
+// TODO: move to DispatchState
 func (b *Builder) hasFromImage() bool {
 	return b.image != "" || b.noBaseImage
 }

builder/dockerfile/dispatchers.go

@@ -33,103 +33,104 @@ import (
 // Sets the environment variable foo to bar, also makes interpolation
 // in the dockerfile available from the next statement on via ${foo}.
 //
-func env(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) == 0 {
+func env(req dispatchRequest) error {
+	if len(req.args) == 0 {
 		return errAtLeastOneArgument("ENV")
 	}
 
-	if len(args)%2 != 0 {
+	if len(req.args)%2 != 0 {
 		// should never get here, but just in case
 		return errTooManyArguments("ENV")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
 	commitMessage := bytes.NewBufferString("ENV")
 
-	for j := 0; j < len(args); j += 2 {
-		if len(args[j]) == 0 {
+	for j := 0; j < len(req.args); j += 2 {
+		if len(req.args[j]) == 0 {
 			return errBlankCommandNames("ENV")
 		}
-		name := args[j]
-		value := args[j+1]
+		name := req.args[j]
+		value := req.args[j+1]
 		newVar := name + "=" + value
 		commitMessage.WriteString(" " + newVar)
 
 		gotOne := false
-		for i, envVar := range b.runConfig.Env {
+		for i, envVar := range req.runConfig.Env {
 			envParts := strings.SplitN(envVar, "=", 2)
 			compareFrom := envParts[0]
 			if equalEnvKeys(compareFrom, name) {
-				b.runConfig.Env[i] = newVar
+				req.runConfig.Env[i] = newVar
 				gotOne = true
 				break
 			}
 		}
 		if !gotOne {
-			b.runConfig.Env = append(b.runConfig.Env, newVar)
+			req.runConfig.Env = append(req.runConfig.Env, newVar)
 		}
 	}
 
-	return b.commit("", b.runConfig.Cmd, commitMessage.String())
+	return req.builder.commit("", req.runConfig.Cmd, commitMessage.String())
 }
 
 // MAINTAINER some text <maybe@an.email.address>
 //
 // Sets the maintainer metadata.
-func maintainer(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) != 1 {
+func maintainer(req dispatchRequest) error {
+	if len(req.args) != 1 {
 		return errExactlyOneArgument("MAINTAINER")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	b.maintainer = args[0]
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("MAINTAINER %s", b.maintainer))
+	maintainer := req.args[0]
+	req.builder.maintainer = maintainer
+	return req.builder.commit("", req.runConfig.Cmd, "MAINTAINER "+maintainer)
 }
 
 // LABEL some json data describing the image
 //
 // Sets the Label variable foo to bar,
 //
-func label(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) == 0 {
+func label(req dispatchRequest) error {
+	if len(req.args) == 0 {
 		return errAtLeastOneArgument("LABEL")
 	}
-	if len(args)%2 != 0 {
+	if len(req.args)%2 != 0 {
 		// should never get here, but just in case
 		return errTooManyArguments("LABEL")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
 	commitStr := "LABEL"
 
-	if b.runConfig.Labels == nil {
-		b.runConfig.Labels = map[string]string{}
+	if req.runConfig.Labels == nil {
+		req.runConfig.Labels = map[string]string{}
 	}
 
-	for j := 0; j < len(args); j++ {
-		// name  ==> args[j]
-		// value ==> args[j+1]
+	for j := 0; j < len(req.args); j++ {
+		// name  ==> req.args[j]
+		// value ==> req.args[j+1]
 
-		if len(args[j]) == 0 {
+		if len(req.args[j]) == 0 {
 			return errBlankCommandNames("LABEL")
 		}
 
-		newVar := args[j] + "=" + args[j+1] + ""
+		newVar := req.args[j] + "=" + req.args[j+1] + ""
 		commitStr += " " + newVar
 
-		b.runConfig.Labels[args[j]] = args[j+1]
+		req.runConfig.Labels[req.args[j]] = req.args[j+1]
 		j++
 	}
-	return b.commit("", b.runConfig.Cmd, commitStr)
+	return req.builder.commit("", req.runConfig.Cmd, commitStr)
 }
 
 // ADD foo /path
@@ -137,73 +138,73 @@ func label(b *Builder, args []string, attributes map[string]bool, original strin
 // Add the file 'foo' to '/path'. Tarball and Remote URL (git, http) handling
 // exist here. If you do not wish to have this automatic handling, use COPY.
 //
-func add(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) < 2 {
+func add(req dispatchRequest) error {
+	if len(req.args) < 2 {
 		return errAtLeastTwoArguments("ADD")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	return b.runContextCommand(args, true, true, "ADD", nil)
+	return req.builder.runContextCommand(req.args, true, true, "ADD", nil)
 }
 
 // COPY foo /path
 //
 // Same as 'ADD' but without the tar and remote url handling.
 //
-func dispatchCopy(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) < 2 {
+func dispatchCopy(req dispatchRequest) error {
+	if len(req.args) < 2 {
 		return errAtLeastTwoArguments("COPY")
 	}
 
-	flFrom := b.flags.AddString("from", "")
+	flFrom := req.flags.AddString("from", "")
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
 	var im *imageMount
 	if flFrom.IsUsed() {
 		var err error
-		im, err = b.imageContexts.get(flFrom.Value)
+		im, err = req.builder.imageContexts.get(flFrom.Value)
 		if err != nil {
 			return err
 		}
 	}
 
-	return b.runContextCommand(args, false, false, "COPY", im)
+	return req.builder.runContextCommand(req.args, false, false, "COPY", im)
 }
 
 // FROM imagename[:tag | @digest] [AS build-stage-name]
 //
-// from sets the base image
-func from(b *Builder, args []string, attributes map[string]bool, original string) error {
-	ctxName, err := parseBuildStageName(args)
+func from(req dispatchRequest) error {
+	ctxName, err := parseBuildStageName(req.args)
 	if err != nil {
 		return err
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
-	b.resetImageCache()
-	if _, err := b.imageContexts.add(ctxName); err != nil {
+
+	req.builder.resetImageCache()
+	if _, err := req.builder.imageContexts.add(ctxName); err != nil {
 		return err
 	}
 
-	image, err := b.getFromImage(args[0])
+	image, err := req.builder.getFromImage(req.args[0])
 	if err != nil {
 		return err
 	}
 	if image != nil {
-		b.imageContexts.update(image.ImageID(), image.RunConfig())
+		req.builder.imageContexts.update(image.ImageID(), image.RunConfig())
 	}
-	b.from = image
+	req.builder.from = image
 
-	b.buildArgs.ResetAllowed()
-	return b.processImageFrom(image)
+	req.builder.buildArgs.ResetAllowed()
+	return req.builder.processImageFrom(image)
 }
 
 func parseBuildStageName(args []string) (string, error) {
@@ -261,16 +262,16 @@ func (b *Builder) getFromImage(name string) (builder.Image, error) {
 // special cases. search for 'OnBuild' in internals.go for additional special
 // cases.
 //
-func onbuild(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) == 0 {
+func onbuild(req dispatchRequest) error {
+	if len(req.args) == 0 {
 		return errAtLeastOneArgument("ONBUILD")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	triggerInstruction := strings.ToUpper(strings.TrimSpace(args[0]))
+	triggerInstruction := strings.ToUpper(strings.TrimSpace(req.args[0]))
 	switch triggerInstruction {
 	case "ONBUILD":
 		return errors.New("Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed")
@@ -278,29 +279,28 @@ func onbuild(b *Builder, args []string, attributes map[string]bool, original str
 		return fmt.Errorf("%s isn't allowed as an ONBUILD trigger", triggerInstruction)
 	}
 
-	original = regexp.MustCompile(`(?i)^\s*ONBUILD\s*`).ReplaceAllString(original, "")
-
-	b.runConfig.OnBuild = append(b.runConfig.OnBuild, original)
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("ONBUILD %s", original))
+	original := regexp.MustCompile(`(?i)^\s*ONBUILD\s*`).ReplaceAllString(req.original, "")
+	req.runConfig.OnBuild = append(req.runConfig.OnBuild, original)
+	return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("ONBUILD %s", original))
 }
 
 // WORKDIR /tmp
 //
 // Set the working directory for future RUN/CMD/etc statements.
 //
-func workdir(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) != 1 {
+func workdir(req dispatchRequest) error {
+	if len(req.args) != 1 {
 		return errExactlyOneArgument("WORKDIR")
 	}
 
-	err := b.flags.Parse()
+	err := req.flags.Parse()
 	if err != nil {
 		return err
 	}
 
 	// This is from the Dockerfile and will not necessarily be in platform
 	// specific semantics, hence ensure it is converted.
-	b.runConfig.WorkingDir, err = normaliseWorkdir(b.runConfig.WorkingDir, args[0])
+	req.runConfig.WorkingDir, err = normaliseWorkdir(req.runConfig.WorkingDir, req.args[0])
 	if err != nil {
 		return err
 	}
@@ -309,39 +309,39 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
 	// This avoids having an unnecessary expensive mount/unmount calls
 	// (on Windows in particular) during each container create.
 	// Prior to 1.13, the mkdir was deferred and not executed at this step.
-	if b.disableCommit {
+	if req.builder.disableCommit {
 		// Don't call back into the daemon if we're going through docker commit --change "WORKDIR /foo".
 		// We've already updated the runConfig and that's enough.
 		return nil
 	}
-	b.runConfig.Image = b.image
 
-	cmd := b.runConfig.Cmd
-	comment := "WORKDIR " + b.runConfig.WorkingDir
+	cmd := req.runConfig.Cmd
+	comment := "WORKDIR " + req.runConfig.WorkingDir
 	// reset the command for cache detection
-	b.runConfig.Cmd = strslice.StrSlice(append(getShell(b.runConfig), "#(nop) "+comment))
-	defer func(cmd strslice.StrSlice) { b.runConfig.Cmd = cmd }(cmd)
+	req.runConfig.Cmd = strslice.StrSlice(append(getShell(req.runConfig), "#(nop) "+comment))
+	defer func(cmd strslice.StrSlice) { req.runConfig.Cmd = cmd }(cmd)
 
-	if hit, err := b.probeCache(); err != nil {
+	if hit, err := req.builder.probeCache(); err != nil {
 		return err
 	} else if hit {
 		return nil
 	}
 
-	container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{
-		Config: b.runConfig,
+	req.runConfig.Image = req.builder.image
+	container, err := req.builder.docker.ContainerCreate(types.ContainerCreateConfig{
+		Config: req.runConfig,
 		// Set a log config to override any default value set on the daemon
 		HostConfig: &container.HostConfig{LogConfig: defaultLogConfig},
 	})
 	if err != nil {
 		return err
 	}
-	b.tmpContainers[container.ID] = struct{}{}
-	if err := b.docker.ContainerCreateWorkdir(container.ID); err != nil {
+	req.builder.tmpContainers[container.ID] = struct{}{}
+	if err := req.builder.docker.ContainerCreateWorkdir(container.ID); err != nil {
 		return err
 	}
 
-	return b.commit(container.ID, cmd, comment)
+	return req.builder.commit(container.ID, cmd, comment)
 }
 
 // RUN some command yo
@@ -354,38 +354,38 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
 // RUN echo hi          # cmd /S /C echo hi   (Windows)
 // RUN [ "echo", "hi" ] # echo hi
 //
-func run(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if !b.hasFromImage() {
+func run(req dispatchRequest) error {
+	if !req.builder.hasFromImage() {
 		return errors.New("Please provide a source image with `from` prior to run")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	args = handleJSONArgs(args, attributes)
+	args := handleJSONArgs(req.args, req.attributes)
 
-	if !attributes["json"] {
-		args = append(getShell(b.runConfig), args...)
+	if !req.attributes["json"] {
+		args = append(getShell(req.runConfig), args...)
 	}
 	config := &container.Config{
 		Cmd:   strslice.StrSlice(args),
-		Image: b.image,
+		Image: req.builder.image,
 	}
 
 	// stash the cmd
-	cmd := b.runConfig.Cmd
-	if len(b.runConfig.Entrypoint) == 0 && len(b.runConfig.Cmd) == 0 {
-		b.runConfig.Cmd = config.Cmd
+	cmd := req.runConfig.Cmd
+	if len(req.runConfig.Entrypoint) == 0 && len(req.runConfig.Cmd) == 0 {
+		req.runConfig.Cmd = config.Cmd
 	}
 
 	// stash the config environment
-	env := b.runConfig.Env
+	env := req.runConfig.Env
 
-	defer func(cmd strslice.StrSlice) { b.runConfig.Cmd = cmd }(cmd)
-	defer func(env []string) { b.runConfig.Env = env }(env)
+	defer func(cmd strslice.StrSlice) { req.runConfig.Cmd = cmd }(cmd)
+	defer func(env []string) { req.runConfig.Env = env }(env)
 
-	cmdBuildEnv := b.buildArgsWithoutConfigEnv()
+	cmdBuildEnv := req.builder.buildArgsWithoutConfigEnv()
 
 	// derive the command to use for probeCache() and to commit in this container.
 	// Note that we only do this if there are any build-time env vars.  Also, we
@@ -401,8 +401,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 		saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...))
 	}
 
-	b.runConfig.Cmd = saveCmd
-	hit, err := b.probeCache()
+	req.runConfig.Cmd = saveCmd
+	hit, err := req.builder.probeCache()
 	if err != nil {
 		return err
 	}
@@ -411,20 +411,20 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 	}
 
 	// set Cmd manually, this is special case only for Dockerfiles
-	b.runConfig.Cmd = config.Cmd
+	req.runConfig.Cmd = config.Cmd
 	// set build-time environment for 'run'.
-	b.runConfig.Env = append(b.runConfig.Env, cmdBuildEnv...)
+	req.runConfig.Env = append(req.runConfig.Env, cmdBuildEnv...)
 	// set config as already being escaped, this prevents double escaping on windows
-	b.runConfig.ArgsEscaped = true
+	req.runConfig.ArgsEscaped = true
 
-	logrus.Debugf("[BUILDER] Command to be executed: %v", b.runConfig.Cmd)
+	logrus.Debugf("[BUILDER] Command to be executed: %v", req.runConfig.Cmd)
 
-	cID, err := b.create()
+	cID, err := req.builder.create()
 	if err != nil {
 		return err
 	}
 
-	if err := b.run(cID); err != nil {
+	if err := req.builder.run(cID); err != nil {
 		return err
 	}
 
@@ -432,7 +432,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 	// revert to original config environment and set the command string to
 	// have the build-time env vars in it (if any) so that future cache look-ups
 	// properly match it.
-	b.runConfig.Env = env
+	req.runConfig.Env = env
 
 	// remove builtinAllowedBuildArgs (see: builder.go)  from the saveCmd
 	// these args are transparent so resulting image should be the same regardless of the value
@@ -442,7 +442,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 		copy(tmpBuildEnv, cmdBuildEnv)
 		for i, env := range tmpBuildEnv {
 			key := strings.SplitN(env, "=", 2)[0]
-			if b.buildArgs.IsUnreferencedBuiltin(key) {
+			if req.builder.buildArgs.IsUnreferencedBuiltin(key) {
 				tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
 			}
 		}
@@ -450,8 +450,8 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 		tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...)
 		saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...))
 	}
-	b.runConfig.Cmd = saveCmd
-	return b.commit(cID, cmd, "run")
+	req.runConfig.Cmd = saveCmd
+	return req.builder.commit(cID, cmd, "run")
 }
 
 // CMD foo
@@ -459,27 +459,27 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 // Set the default command to run in the container (which may be empty).
 // Argument handling is the same as RUN.
 //
-func cmd(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if err := b.flags.Parse(); err != nil {
+func cmd(req dispatchRequest) error {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	cmdSlice := handleJSONArgs(args, attributes)
+	cmdSlice := handleJSONArgs(req.args, req.attributes)
 
-	if !attributes["json"] {
-		cmdSlice = append(getShell(b.runConfig), cmdSlice...)
+	if !req.attributes["json"] {
+		cmdSlice = append(getShell(req.runConfig), cmdSlice...)
 	}
 
-	b.runConfig.Cmd = strslice.StrSlice(cmdSlice)
+	req.runConfig.Cmd = strslice.StrSlice(cmdSlice)
 	// set config as already being escaped, this prevents double escaping on windows
-	b.runConfig.ArgsEscaped = true
+	req.runConfig.ArgsEscaped = true
 
-	if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("CMD %q", cmdSlice)); err != nil {
+	if err := req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("CMD %q", cmdSlice)); err != nil {
 		return err
 	}
 
-	if len(args) != 0 {
-		b.cmdSet = true
+	if len(req.args) != 0 {
+		req.builder.cmdSet = true
 	}
 
 	return nil
@@ -507,47 +507,47 @@ func parseOptInterval(f *Flag) (time.Duration, error) {
 // Set the default healthcheck command to run in the container (which may be empty).
 // Argument handling is the same as RUN.
 //
-func healthcheck(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) == 0 {
+func healthcheck(req dispatchRequest) error {
+	if len(req.args) == 0 {
 		return errAtLeastOneArgument("HEALTHCHECK")
 	}
-	typ := strings.ToUpper(args[0])
-	args = args[1:]
+	typ := strings.ToUpper(req.args[0])
+	args := req.args[1:]
 	if typ == "NONE" {
 		if len(args) != 0 {
 			return errors.New("HEALTHCHECK NONE takes no arguments")
 		}
 		test := strslice.StrSlice{typ}
-		b.runConfig.Healthcheck = &container.HealthConfig{
+		req.runConfig.Healthcheck = &container.HealthConfig{
 			Test: test,
 		}
 	} else {
-		if b.runConfig.Healthcheck != nil {
-			oldCmd := b.runConfig.Healthcheck.Test
+		if req.runConfig.Healthcheck != nil {
+			oldCmd := req.runConfig.Healthcheck.Test
 			if len(oldCmd) > 0 && oldCmd[0] != "NONE" {
-				fmt.Fprintf(b.Stdout, "Note: overriding previous HEALTHCHECK: %v\n", oldCmd)
+				fmt.Fprintf(req.builder.Stdout, "Note: overriding previous HEALTHCHECK: %v\n", oldCmd)
 			}
 		}
 
 		healthcheck := container.HealthConfig{}
 
-		flInterval := b.flags.AddString("interval", "")
-		flTimeout := b.flags.AddString("timeout", "")
-		flStartPeriod := b.flags.AddString("start-period", "")
-		flRetries := b.flags.AddString("retries", "")
+		flInterval := req.flags.AddString("interval", "")
+		flTimeout := req.flags.AddString("timeout", "")
+		flStartPeriod := req.flags.AddString("start-period", "")
+		flRetries := req.flags.AddString("retries", "")
 
-		if err := b.flags.Parse(); err != nil {
+		if err := req.flags.Parse(); err != nil {
 			return err
 		}
 
 		switch typ {
 		case "CMD":
-			cmdSlice := handleJSONArgs(args, attributes)
+			cmdSlice := handleJSONArgs(args, req.attributes)
 			if len(cmdSlice) == 0 {
 				return errors.New("Missing command after HEALTHCHECK CMD")
 			}
 
-			if !attributes["json"] {
+			if !req.attributes["json"] {
 				typ = "CMD-SHELL"
 			}
 
@@ -587,10 +587,10 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, original
 			healthcheck.Retries = 0
 		}
 
-		b.runConfig.Healthcheck = &healthcheck
+		req.runConfig.Healthcheck = &healthcheck
 	}
 
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", b.runConfig.Healthcheck))
+	return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", req.runConfig.Healthcheck))
 }
 
 // ENTRYPOINT /usr/sbin/nginx
@@ -598,35 +598,35 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, original
 // Set the entrypoint to /usr/sbin/nginx. Will accept the CMD as the arguments
 // to /usr/sbin/nginx. Uses the default shell if not in JSON format.
 //
-// Handles command processing similar to CMD and RUN, only b.runConfig.Entrypoint
+// Handles command processing similar to CMD and RUN, only req.runConfig.Entrypoint
 // is initialized at NewBuilder time instead of through argument parsing.
 //
-func entrypoint(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if err := b.flags.Parse(); err != nil {
+func entrypoint(req dispatchRequest) error {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	parsed := handleJSONArgs(args, attributes)
+	parsed := handleJSONArgs(req.args, req.attributes)
 
 	switch {
-	case attributes["json"]:
+	case req.attributes["json"]:
 		// ENTRYPOINT ["echo", "hi"]
-		b.runConfig.Entrypoint = strslice.StrSlice(parsed)
+		req.runConfig.Entrypoint = strslice.StrSlice(parsed)
 	case len(parsed) == 0:
 		// ENTRYPOINT []
-		b.runConfig.Entrypoint = nil
+		req.runConfig.Entrypoint = nil
 	default:
 		// ENTRYPOINT echo hi
-		b.runConfig.Entrypoint = strslice.StrSlice(append(getShell(b.runConfig), parsed[0]))
+		req.runConfig.Entrypoint = strslice.StrSlice(append(getShell(req.runConfig), parsed[0]))
 	}
 
 	// when setting the entrypoint if a CMD was not explicitly set then
 	// set the command to nil
-	if !b.cmdSet {
-		b.runConfig.Cmd = nil
+	if !req.builder.cmdSet {
+		req.runConfig.Cmd = nil
 	}
 
-	if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("ENTRYPOINT %q", b.runConfig.Entrypoint)); err != nil {
+	if err := req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("ENTRYPOINT %q", req.runConfig.Entrypoint)); err != nil {
 		return err
 	}
 
@@ -636,21 +636,21 @@ func entrypoint(b *Builder, args []string, attributes map[string]bool, original
 // EXPOSE 6667/tcp 7000/tcp
 //
 // Expose ports for links and port mappings. This all ends up in
-// b.runConfig.ExposedPorts for runconfig.
+// req.runConfig.ExposedPorts for runconfig.
 //
-func expose(b *Builder, args []string, attributes map[string]bool, original string) error {
-	portsTab := args
+func expose(req dispatchRequest) error {
+	portsTab := req.args
 
-	if len(args) == 0 {
+	if len(req.args) == 0 {
 		return errAtLeastOneArgument("EXPOSE")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	if b.runConfig.ExposedPorts == nil {
-		b.runConfig.ExposedPorts = make(nat.PortSet)
+	if req.runConfig.ExposedPorts == nil {
+		req.runConfig.ExposedPorts = make(nat.PortSet)
 	}
 
 	ports, _, err := nat.ParsePortSpecs(portsTab)
@@ -664,14 +664,14 @@ func expose(b *Builder, args []string, attributes map[string]bool, original stri
 	portList := make([]string, len(ports))
 	var i int
 	for port := range ports {
-		if _, exists := b.runConfig.ExposedPorts[port]; !exists {
-			b.runConfig.ExposedPorts[port] = struct{}{}
+		if _, exists := req.runConfig.ExposedPorts[port]; !exists {
+			req.runConfig.ExposedPorts[port] = struct{}{}
 		}
 		portList[i] = string(port)
 		i++
 	}
 	sort.Strings(portList)
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("EXPOSE %s", strings.Join(portList, " ")))
+	return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("EXPOSE %s", strings.Join(portList, " ")))
 }
 
 // USER foo
@@ -679,43 +679,43 @@ func expose(b *Builder, args []string, attributes map[string]bool, original stri
 // Set the user to 'foo' for future commands and when running the
 // ENTRYPOINT/CMD at container run time.
 //
-func user(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) != 1 {
+func user(req dispatchRequest) error {
+	if len(req.args) != 1 {
 		return errExactlyOneArgument("USER")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	b.runConfig.User = args[0]
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("USER %v", args))
+	req.runConfig.User = req.args[0]
+	return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("USER %v", req.args))
 }
 
 // VOLUME /foo
 //
 // Expose the volume /foo for use. Will also accept the JSON array form.
 //
-func volume(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) == 0 {
+func volume(req dispatchRequest) error {
+	if len(req.args) == 0 {
 		return errAtLeastOneArgument("VOLUME")
 	}
 
-	if err := b.flags.Parse(); err != nil {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
 
-	if b.runConfig.Volumes == nil {
-		b.runConfig.Volumes = map[string]struct{}{}
+	if req.runConfig.Volumes == nil {
+		req.runConfig.Volumes = map[string]struct{}{}
 	}
-	for _, v := range args {
+	for _, v := range req.args {
 		v = strings.TrimSpace(v)
 		if v == "" {
 			return errors.New("VOLUME specified can not be an empty string")
 		}
-		b.runConfig.Volumes[v] = struct{}{}
+		req.runConfig.Volumes[v] = struct{}{}
 	}
-	if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("VOLUME %v", args)); err != nil {
+	if err := req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("VOLUME %v", req.args)); err != nil {
 		return err
 	}
 	return nil
@@ -724,19 +724,19 @@ func volume(b *Builder, args []string, attributes map[string]bool, original stri
 // STOPSIGNAL signal
 //
 // Set the signal that will be used to kill the container.
-func stopSignal(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) != 1 {
+func stopSignal(req dispatchRequest) error {
+	if len(req.args) != 1 {
 		return errExactlyOneArgument("STOPSIGNAL")
 	}
 
-	sig := args[0]
+	sig := req.args[0]
 	_, err := signal.ParseSignal(sig)
 	if err != nil {
 		return err
 	}
 
-	b.runConfig.StopSignal = sig
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("STOPSIGNAL %v", args))
+	req.runConfig.StopSignal = sig
+	return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("STOPSIGNAL %v", req.args))
 }
 
 // ARG name[=value]
@@ -744,8 +744,8 @@ func stopSignal(b *Builder, args []string, attributes map[string]bool, original
 // Adds the variable foo to the trusted list of variables that can be passed
 // to builder using the --build-arg flag for expansion/substitution or passing to 'run'.
 // Dockerfile author may optionally set a default value of this variable.
-func arg(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) != 1 {
+func arg(req dispatchRequest) error {
+	if len(req.args) != 1 {
 		return errExactlyOneArgument("ARG")
 	}
 
@@ -755,7 +755,7 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string)
 		hasDefault bool
 	)
 
-	arg := args[0]
+	arg := req.args[0]
 	// 'arg' can just be a name or name-value pair. Note that this is different
 	// from 'env' that handles the split of name and value at the parser level.
 	// The reason for doing it differently for 'arg' is that we support just
@@ -779,36 +779,36 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string)
 	if hasDefault {
 		value = &newValue
 	}
-	b.buildArgs.AddArg(name, value)
+	req.builder.buildArgs.AddArg(name, value)
 
 	// Arg before FROM doesn't add a layer
-	if !b.hasFromImage() {
-		b.buildArgs.AddMetaArg(name, value)
+	if !req.builder.hasFromImage() {
+		req.builder.buildArgs.AddMetaArg(name, value)
 		return nil
 	}
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("ARG %s", arg))
+	return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("ARG %s", arg))
 }
 
 // SHELL powershell -command
 //
 // Set the non-default shell to use.
-func shell(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if err := b.flags.Parse(); err != nil {
+func shell(req dispatchRequest) error {
+	if err := req.flags.Parse(); err != nil {
 		return err
 	}
-	shellSlice := handleJSONArgs(args, attributes)
+	shellSlice := handleJSONArgs(req.args, req.attributes)
 	switch {
 	case len(shellSlice) == 0:
 		// SHELL []
 		return errAtLeastOneArgument("SHELL")
-	case attributes["json"]:
+	case req.attributes["json"]:
 		// SHELL ["powershell", "-command"]
-		b.runConfig.Shell = strslice.StrSlice(shellSlice)
+		req.runConfig.Shell = strslice.StrSlice(shellSlice)
 	default:
 		// SHELL powershell -command - not JSON
-		return errNotJSON("SHELL", original)
+		return errNotJSON("SHELL", req.original)
 	}
-	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("SHELL %v", shellSlice))
+	return req.builder.commit("", req.runConfig.Cmd, fmt.Sprintf("SHELL %v", shellSlice))
 }
 
 func errAtLeastOneArgument(command string) error {

builder/dockerfile/dispatchers_test.go

@@ -3,15 +3,16 @@ package dockerfile
 import (
 	"fmt"
 	"runtime"
-	"strings"
 	"testing"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/builder"
+	"github.com/docker/docker/pkg/testutil"
 	"github.com/docker/go-connections/nat"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type commandWithFunction struct {
@@ -19,161 +20,142 @@ type commandWithFunction struct {
 	function func(args []string) error
 }
 
+func withArgs(f dispatcher) func([]string) error {
+	return func(args []string) error {
+		return f(dispatchRequest{args: args, runConfig: &container.Config{}})
+	}
+}
+
+func withBuilderAndArgs(builder *Builder, f dispatcher) func([]string) error {
+	return func(args []string) error {
+		return f(defaultDispatchReq(builder, args...))
+	}
+}
+
+func defaultDispatchReq(builder *Builder, args ...string) dispatchRequest {
+	return dispatchRequest{
+		builder:   builder,
+		args:      args,
+		flags:     NewBFlags(),
+		runConfig: &container.Config{},
+	}
+}
+
+func newBuilderWithMockBackend() *Builder {
+	b := &Builder{
+		runConfig:     &container.Config{},
+		options:       &types.ImageBuildOptions{},
+		docker:        &MockBackend{},
+		buildArgs:     newBuildArgs(make(map[string]*string)),
+		disableCommit: true,
+	}
+	b.imageContexts = &imageContexts{b: b}
+	return b
+}
+
 func TestCommandsExactlyOneArgument(t *testing.T) {
 	commands := []commandWithFunction{
-		{"MAINTAINER", func(args []string) error { return maintainer(nil, args, nil, "") }},
-		{"WORKDIR", func(args []string) error { return workdir(nil, args, nil, "") }},
-		{"USER", func(args []string) error { return user(nil, args, nil, "") }},
-		{"STOPSIGNAL", func(args []string) error { return stopSignal(nil, args, nil, "") }}}
+		{"MAINTAINER", withArgs(maintainer)},
+		{"WORKDIR", withArgs(workdir)},
+		{"USER", withArgs(user)},
+		{"STOPSIGNAL", withArgs(stopSignal)},
+	}
 
 	for _, command := range commands {
 		err := command.function([]string{})
-
-		if err == nil {
-			t.Fatalf("Error should be present for %s command", command.name)
-		}
-
-		expectedError := errExactlyOneArgument(command.name)
-
-		if err.Error() != expectedError.Error() {
-			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
-		}
+		assert.EqualError(t, err, errExactlyOneArgument(command.name).Error())
 	}
 }
 
 func TestCommandsAtLeastOneArgument(t *testing.T) {
 	commands := []commandWithFunction{
-		{"ENV", func(args []string) error { return env(nil, args, nil, "") }},
-		{"LABEL", func(args []string) error { return label(nil, args, nil, "") }},
-		{"ONBUILD", func(args []string) error { return onbuild(nil, args, nil, "") }},
-		{"HEALTHCHECK", func(args []string) error { return healthcheck(nil, args, nil, "") }},
-		{"EXPOSE", func(args []string) error { return expose(nil, args, nil, "") }},
-		{"VOLUME", func(args []string) error { return volume(nil, args, nil, "") }}}
+		{"ENV", withArgs(env)},
+		{"LABEL", withArgs(label)},
+		{"ONBUILD", withArgs(onbuild)},
+		{"HEALTHCHECK", withArgs(healthcheck)},
+		{"EXPOSE", withArgs(expose)},
+		{"VOLUME", withArgs(volume)},
+	}
 
 	for _, command := range commands {
 		err := command.function([]string{})
-
-		if err == nil {
-			t.Fatalf("Error should be present for %s command", command.name)
-		}
-
-		expectedError := errAtLeastOneArgument(command.name)
-
-		if err.Error() != expectedError.Error() {
-			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
-		}
+		assert.EqualError(t, err, errAtLeastOneArgument(command.name).Error())
 	}
 }
 
 func TestCommandsAtLeastTwoArguments(t *testing.T) {
 	commands := []commandWithFunction{
-		{"ADD", func(args []string) error { return add(nil, args, nil, "") }},
-		{"COPY", func(args []string) error { return dispatchCopy(nil, args, nil, "") }}}
+		{"ADD", withArgs(add)},
+		{"COPY", withArgs(dispatchCopy)}}
 
 	for _, command := range commands {
 		err := command.function([]string{"arg1"})
-
-		if err == nil {
-			t.Fatalf("Error should be present for %s command", command.name)
-		}
-
-		expectedError := errAtLeastTwoArguments(command.name)
-
-		if err.Error() != expectedError.Error() {
-			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
-		}
+		assert.EqualError(t, err, errAtLeastTwoArguments(command.name).Error())
 	}
 }
 
 func TestCommandsTooManyArguments(t *testing.T) {
 	commands := []commandWithFunction{
-		{"ENV", func(args []string) error { return env(nil, args, nil, "") }},
-		{"LABEL", func(args []string) error { return label(nil, args, nil, "") }}}
+		{"ENV", withArgs(env)},
+		{"LABEL", withArgs(label)}}
 
 	for _, command := range commands {
 		err := command.function([]string{"arg1", "arg2", "arg3"})
-
-		if err == nil {
-			t.Fatalf("Error should be present for %s command", command.name)
-		}
-
-		expectedError := errTooManyArguments(command.name)
-
-		if err.Error() != expectedError.Error() {
-			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
-		}
+		assert.EqualError(t, err, errTooManyArguments(command.name).Error())
 	}
 }
 
-func TestCommandseBlankNames(t *testing.T) {
-	bflags := &BFlags{}
-	config := &container.Config{}
-
-	b := &Builder{flags: bflags, runConfig: config, disableCommit: true}
-
+func TestCommandsBlankNames(t *testing.T) {
+	builder := newBuilderWithMockBackend()
 	commands := []commandWithFunction{
-		{"ENV", func(args []string) error { return env(b, args, nil, "") }},
-		{"LABEL", func(args []string) error { return label(b, args, nil, "") }},
+		{"ENV", withBuilderAndArgs(builder, env)},
+		{"LABEL", withBuilderAndArgs(builder, label)},
 	}
 
 	for _, command := range commands {
 		err := command.function([]string{"", ""})
-
-		if err == nil {
-			t.Fatalf("Error should be present for %s command", command.name)
-		}
-
-		expectedError := errBlankCommandNames(command.name)
-
-		if err.Error() != expectedError.Error() {
-			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
-		}
+		assert.EqualError(t, err, errBlankCommandNames(command.name).Error())
 	}
 }
 
 func TestEnv2Variables(t *testing.T) {
 	b := newBuilderWithMockBackend()
-	b.disableCommit = true
 
 	args := []string{"var1", "val1", "var2", "val2"}
-	err := env(b, args, nil, "")
-	assert.NoError(t, err)
+	req := defaultDispatchReq(b, args...)
+	err := env(req)
+	require.NoError(t, err)
 
 	expected := []string{
 		fmt.Sprintf("%s=%s", args[0], args[1]),
 		fmt.Sprintf("%s=%s", args[2], args[3]),
 	}
-	assert.Equal(t, expected, b.runConfig.Env)
+	assert.Equal(t, expected, req.runConfig.Env)
 }
 
 func TestEnvValueWithExistingRunConfigEnv(t *testing.T) {
 	b := newBuilderWithMockBackend()
-	b.disableCommit = true
-	b.runConfig.Env = []string{"var1=old", "var2=fromenv"}
 
 	args := []string{"var1", "val1"}
-	err := env(b, args, nil, "")
-	assert.NoError(t, err)
+	req := defaultDispatchReq(b, args...)
+	req.runConfig.Env = []string{"var1=old", "var2=fromenv"}
+	err := env(req)
+	require.NoError(t, err)
 
 	expected := []string{
 		fmt.Sprintf("%s=%s", args[0], args[1]),
 		"var2=fromenv",
 	}
-	assert.Equal(t, expected, b.runConfig.Env)
+	assert.Equal(t, expected, req.runConfig.Env)
 }
 
 func TestMaintainer(t *testing.T) {
 	maintainerEntry := "Some Maintainer <maintainer@example.com>"
 
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
-	if err := maintainer(b, []string{maintainerEntry}, nil, ""); err != nil {
-		t.Fatalf("Error when executing maintainer: %s", err.Error())
-	}
-
-	if b.maintainer != maintainerEntry {
-		t.Fatalf("Maintainer in builder should be set to %s. Got: %s", maintainerEntry, b.maintainer)
-	}
+	b := newBuilderWithMockBackend()
+	err := maintainer(defaultDispatchReq(b, maintainerEntry))
+	require.NoError(t, err)
+	assert.Equal(t, maintainerEntry, b.maintainer)
 }
 
 func TestLabel(t *testing.T) {
@@ -181,45 +163,25 @@ func TestLabel(t *testing.T) {
 	labelValue := "value"
 
 	labelEntry := []string{labelName, labelValue}
+	b := newBuilderWithMockBackend()
+	req := defaultDispatchReq(b, labelEntry...)
+	err := label(req)
+	require.NoError(t, err)
 
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
-	if err := label(b, labelEntry, nil, ""); err != nil {
-		t.Fatalf("Error when executing label: %s", err.Error())
-	}
-
-	if val, ok := b.runConfig.Labels[labelName]; ok {
-		if val != labelValue {
-			t.Fatalf("Label %s should have value %s, had %s instead", labelName, labelValue, val)
-		}
-	} else {
-		t.Fatalf("Label %s should be present but it is not", labelName)
-	}
-}
-
-func newBuilderWithMockBackend() *Builder {
-	b := &Builder{
-		flags:     &BFlags{},
-		runConfig: &container.Config{},
-		options:   &types.ImageBuildOptions{},
-		docker:    &MockBackend{},
-		buildArgs: newBuildArgs(make(map[string]*string)),
-	}
-	b.imageContexts = &imageContexts{b: b}
-	return b
+	require.Contains(t, req.runConfig.Labels, labelName)
+	assert.Equal(t, req.runConfig.Labels[labelName], labelValue)
 }
 
 func TestFromScratch(t *testing.T) {
 	b := newBuilderWithMockBackend()
-
-	err := from(b, []string{"scratch"}, nil, "")
+	err := from(defaultDispatchReq(b, "scratch"))
 
 	if runtime.GOOS == "windows" {
 		assert.EqualError(t, err, "Windows does not support FROM scratch")
 		return
 	}
 
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "", b.image)
 	assert.Equal(t, true, b.noBaseImage)
 }
@@ -234,10 +196,10 @@ func TestFromWithArg(t *testing.T) {
 	b := newBuilderWithMockBackend()
 	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
 
-	assert.NoError(t, arg(b, []string{"THETAG=" + tag}, nil, ""))
-	err := from(b, []string{"alpine${THETAG}"}, nil, "")
+	require.NoError(t, arg(defaultDispatchReq(b, "THETAG="+tag)))
+	err := from(defaultDispatchReq(b, "alpine${THETAG}"))
 
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, expected, b.image)
 	assert.Equal(t, expected, b.from.ImageID())
 	assert.Len(t, b.buildArgs.GetAllAllowed(), 0)
@@ -255,8 +217,8 @@ func TestFromWithUndefinedArg(t *testing.T) {
 	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
 	b.options.BuildArgs = map[string]*string{"THETAG": &tag}
 
-	err := from(b, []string{"alpine${THETAG}"}, nil, "")
-	assert.NoError(t, err)
+	err := from(defaultDispatchReq(b, "alpine${THETAG}"))
+	require.NoError(t, err)
 	assert.Equal(t, expected, b.image)
 }
 
@@ -267,237 +229,147 @@ func TestOnbuildIllegalTriggers(t *testing.T) {
 		{"FROM", "FROM isn't allowed as an ONBUILD trigger"}}
 
 	for _, trigger := range triggers {
-		b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
-		err := onbuild(b, []string{trigger.command}, nil, "")
-
-		if err == nil {
-			t.Fatal("Error should not be nil")
-		}
+		b := newBuilderWithMockBackend()
 
-		if !strings.Contains(err.Error(), trigger.expectedError) {
-			t.Fatalf("Error message not correct. Should be: %s, got: %s", trigger.expectedError, err.Error())
-		}
+		err := onbuild(defaultDispatchReq(b, trigger.command))
+		testutil.ErrorContains(t, err, trigger.expectedError)
 	}
 }
 
 func TestOnbuild(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
-	err := onbuild(b, []string{"ADD", ".", "/app/src"}, nil, "ONBUILD ADD . /app/src")
-
-	if err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
+	b := newBuilderWithMockBackend()
 
-	expectedOnbuild := "ADD . /app/src"
+	req := defaultDispatchReq(b, "ADD", ".", "/app/src")
+	req.original = "ONBUILD ADD . /app/src"
+	req.runConfig = &container.Config{}
 
-	if b.runConfig.OnBuild[0] != expectedOnbuild {
-		t.Fatalf("Wrong ONBUILD command. Expected: %s, got: %s", expectedOnbuild, b.runConfig.OnBuild[0])
-	}
+	err := onbuild(req)
+	require.NoError(t, err)
+	assert.Equal(t, "ADD . /app/src", req.runConfig.OnBuild[0])
 }
 
 func TestWorkdir(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
+	b := newBuilderWithMockBackend()
 	workingDir := "/app"
-
 	if runtime.GOOS == "windows" {
 		workingDir = "C:\app"
 	}
 
-	err := workdir(b, []string{workingDir}, nil, "")
-
-	if err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.WorkingDir != workingDir {
-		t.Fatalf("WorkingDir should be set to %s, got %s", workingDir, b.runConfig.WorkingDir)
-	}
-
+	req := defaultDispatchReq(b, workingDir)
+	err := workdir(req)
+	require.NoError(t, err)
+	assert.Equal(t, workingDir, req.runConfig.WorkingDir)
 }
 
 func TestCmd(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
+	b := newBuilderWithMockBackend()
 	command := "./executable"
 
-	err := cmd(b, []string{command}, nil, "")
-
-	if err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
+	req := defaultDispatchReq(b, command)
+	err := cmd(req)
+	require.NoError(t, err)
 
 	var expectedCommand strslice.StrSlice
-
 	if runtime.GOOS == "windows" {
 		expectedCommand = strslice.StrSlice(append([]string{"cmd"}, "/S", "/C", command))
 	} else {
 		expectedCommand = strslice.StrSlice(append([]string{"/bin/sh"}, "-c", command))
 	}
 
-	if !compareStrSlice(b.runConfig.Cmd, expectedCommand) {
-		t.Fatalf("Command should be set to %s, got %s", command, b.runConfig.Cmd)
-	}
-
-	if !b.cmdSet {
-		t.Fatal("Command should be marked as set")
-	}
-}
-
-func compareStrSlice(slice1, slice2 strslice.StrSlice) bool {
-	if len(slice1) != len(slice2) {
-		return false
-	}
-
-	for i := range slice1 {
-		if slice1[i] != slice2[i] {
-			return false
-		}
-	}
-
-	return true
+	assert.Equal(t, expectedCommand, req.runConfig.Cmd)
+	assert.True(t, b.cmdSet)
 }
 
 func TestHealthcheckNone(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
-	if err := healthcheck(b, []string{"NONE"}, nil, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.Healthcheck == nil {
-		t.Fatal("Healthcheck should be set, got nil")
-	}
+	b := newBuilderWithMockBackend()
 
-	expectedTest := strslice.StrSlice(append([]string{"NONE"}))
+	req := defaultDispatchReq(b, "NONE")
+	err := healthcheck(req)
+	require.NoError(t, err)
 
-	if !compareStrSlice(expectedTest, b.runConfig.Healthcheck.Test) {
-		t.Fatalf("Command should be set to %s, got %s", expectedTest, b.runConfig.Healthcheck.Test)
-	}
+	require.NotNil(t, req.runConfig.Healthcheck)
+	assert.Equal(t, []string{"NONE"}, req.runConfig.Healthcheck.Test)
 }
 
 func TestHealthcheckCmd(t *testing.T) {
-	b := &Builder{flags: &BFlags{flags: make(map[string]*Flag)}, runConfig: &container.Config{}, disableCommit: true}
-
-	if err := healthcheck(b, []string{"CMD", "curl", "-f", "http://localhost/", "||", "exit", "1"}, nil, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.Healthcheck == nil {
-		t.Fatal("Healthcheck should be set, got nil")
-	}
+	b := newBuilderWithMockBackend()
 
-	expectedTest := strslice.StrSlice(append([]string{"CMD-SHELL"}, "curl -f http://localhost/ || exit 1"))
+	args := []string{"CMD", "curl", "-f", "http://localhost/", "||", "exit", "1"}
+	req := defaultDispatchReq(b, args...)
+	err := healthcheck(req)
+	require.NoError(t, err)
 
-	if !compareStrSlice(expectedTest, b.runConfig.Healthcheck.Test) {
-		t.Fatalf("Command should be set to %s, got %s", expectedTest, b.runConfig.Healthcheck.Test)
-	}
+	require.NotNil(t, req.runConfig.Healthcheck)
+	expectedTest := []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"}
+	assert.Equal(t, expectedTest, req.runConfig.Healthcheck.Test)
 }
 
 func TestEntrypoint(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
+	b := newBuilderWithMockBackend()
 	entrypointCmd := "/usr/sbin/nginx"
 
-	if err := entrypoint(b, []string{entrypointCmd}, nil, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.Entrypoint == nil {
-		t.Fatal("Entrypoint should be set")
-	}
+	req := defaultDispatchReq(b, entrypointCmd)
+	err := entrypoint(req)
+	require.NoError(t, err)
+	require.NotNil(t, req.runConfig.Entrypoint)
 
 	var expectedEntrypoint strslice.StrSlice
-
 	if runtime.GOOS == "windows" {
 		expectedEntrypoint = strslice.StrSlice(append([]string{"cmd"}, "/S", "/C", entrypointCmd))
 	} else {
 		expectedEntrypoint = strslice.StrSlice(append([]string{"/bin/sh"}, "-c", entrypointCmd))
 	}
-
-	if !compareStrSlice(expectedEntrypoint, b.runConfig.Entrypoint) {
-		t.Fatalf("Entrypoint command should be set to %s, got %s", expectedEntrypoint, b.runConfig.Entrypoint)
-	}
+	assert.Equal(t, expectedEntrypoint, req.runConfig.Entrypoint)
 }
 
 func TestExpose(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
+	b := newBuilderWithMockBackend()
 
 	exposedPort := "80"
+	req := defaultDispatchReq(b, exposedPort)
+	err := expose(req)
+	require.NoError(t, err)
 
-	if err := expose(b, []string{exposedPort}, nil, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.ExposedPorts == nil {
-		t.Fatal("ExposedPorts should be set")
-	}
-
-	if len(b.runConfig.ExposedPorts) != 1 {
-		t.Fatalf("ExposedPorts should contain only 1 element. Got %s", b.runConfig.ExposedPorts)
-	}
+	require.NotNil(t, req.runConfig.ExposedPorts)
+	require.Len(t, req.runConfig.ExposedPorts, 1)
 
 	portsMapping, err := nat.ParsePortSpec(exposedPort)
-
-	if err != nil {
-		t.Fatalf("Error when parsing port spec: %s", err.Error())
-	}
-
-	if _, ok := b.runConfig.ExposedPorts[portsMapping[0].Port]; !ok {
-		t.Fatalf("Port %s should be present. Got %s", exposedPort, b.runConfig.ExposedPorts)
-	}
+	require.NoError(t, err)
+	assert.Contains(t, req.runConfig.ExposedPorts, portsMapping[0].Port)
 }
 
 func TestUser(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
+	b := newBuilderWithMockBackend()
 	userCommand := "foo"
 
-	if err := user(b, []string{userCommand}, nil, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.User != userCommand {
-		t.Fatalf("User should be set to %s, got %s", userCommand, b.runConfig.User)
-	}
+	req := defaultDispatchReq(b, userCommand)
+	err := user(req)
+	require.NoError(t, err)
+	assert.Equal(t, userCommand, req.runConfig.User)
 }
 
 func TestVolume(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
+	b := newBuilderWithMockBackend()
 
 	exposedVolume := "/foo"
 
-	if err := volume(b, []string{exposedVolume}, nil, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.Volumes == nil {
-		t.Fatal("Volumes should be set")
-	}
+	req := defaultDispatchReq(b, exposedVolume)
+	err := volume(req)
+	require.NoError(t, err)
 
-	if len(b.runConfig.Volumes) != 1 {
-		t.Fatalf("Volumes should contain only 1 element. Got %s", b.runConfig.Volumes)
-	}
-
-	if _, ok := b.runConfig.Volumes[exposedVolume]; !ok {
-		t.Fatalf("Volume %s should be present. Got %s", exposedVolume, b.runConfig.Volumes)
-	}
+	require.NotNil(t, req.runConfig.Volumes)
+	assert.Len(t, req.runConfig.Volumes, 1)
+	assert.Contains(t, req.runConfig.Volumes, exposedVolume)
 }
 
 func TestStopSignal(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
-
+	b := newBuilderWithMockBackend()
 	signal := "SIGKILL"
 
-	if err := stopSignal(b, []string{signal}, nil, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.StopSignal != signal {
-		t.Fatalf("StopSignal should be set to %s, got %s", signal, b.runConfig.StopSignal)
-	}
+	req := defaultDispatchReq(b, signal)
+	err := stopSignal(req)
+	require.NoError(t, err)
+	assert.Equal(t, signal, req.runConfig.StopSignal)
 }
 
 func TestArg(t *testing.T) {
@@ -507,33 +379,23 @@ func TestArg(t *testing.T) {
 	argVal := "bar"
 	argDef := fmt.Sprintf("%s=%s", argName, argVal)
 
-	err := arg(b, []string{argDef}, nil, "")
-	assert.NoError(t, err)
+	err := arg(defaultDispatchReq(b, argDef))
+	require.NoError(t, err)
 
 	expected := map[string]string{argName: argVal}
-	allowed := b.buildArgs.GetAllAllowed()
-	assert.Equal(t, expected, allowed)
+	assert.Equal(t, expected, b.buildArgs.GetAllAllowed())
 }
 
 func TestShell(t *testing.T) {
-	b := &Builder{flags: &BFlags{}, runConfig: &container.Config{}, disableCommit: true}
+	b := newBuilderWithMockBackend()
 
 	shellCmd := "powershell"
+	req := defaultDispatchReq(b, shellCmd)
+	req.attributes = map[string]bool{"json": true}
 
-	attrs := make(map[string]bool)
-	attrs["json"] = true
-
-	if err := shell(b, []string{shellCmd}, attrs, ""); err != nil {
-		t.Fatalf("Error should be empty, got: %s", err.Error())
-	}
-
-	if b.runConfig.Shell == nil {
-		t.Fatal("Shell should be set")
-	}
+	err := shell(req)
+	require.NoError(t, err)
 
 	expectedShell := strslice.StrSlice([]string{shellCmd})
-
-	if !compareStrSlice(expectedShell, b.runConfig.Shell) {
-		t.Fatalf("Shell should be set to %s, got %s", expectedShell, b.runConfig.Shell)
-	}
+	assert.Equal(t, expectedShell, req.runConfig.Shell)
 }

builder/dockerfile/evaluator.go

@@ -20,9 +20,11 @@
 package dockerfile
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 
+	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/dockerfile/command"
 	"github.com/docker/docker/builder/dockerfile/parser"
 	"github.com/docker/docker/runconfig/opts"
@@ -56,10 +58,32 @@ var allowWordExpansion = map[string]bool{
 	command.Expose: true,
 }
 
-var evaluateTable map[string]func(*Builder, []string, map[string]bool, string) error
+type dispatchRequest struct {
+	builder    *Builder // TODO: replace this with a smaller interface
+	args       []string
+	attributes map[string]bool
+	flags      *BFlags
+	original   string
+	runConfig  *container.Config
+}
+
+func newDispatchRequestFromNode(node *parser.Node, builder *Builder, args []string) dispatchRequest {
+	return dispatchRequest{
+		builder:    builder,
+		args:       args,
+		attributes: node.Attributes,
+		original:   node.Original,
+		flags:      NewBFlagsWithArgs(node.Flags),
+		runConfig:  builder.runConfig,
+	}
+}
+
+type dispatcher func(dispatchRequest) error
+
+var evaluateTable map[string]dispatcher
 
 func init() {
-	evaluateTable = map[string]func(*Builder, []string, map[string]bool, string) error{
+	evaluateTable = map[string]dispatcher{
 		command.Add:         add,
 		command.Arg:         arg,
 		command.Cmd:         cmd,
@@ -95,8 +119,8 @@ 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, stepTotal int, ast *parser.Node) error {
-	cmd := ast.Value
+func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node) error {
+	cmd := node.Value
 	upperCasedCmd := strings.ToUpper(cmd)
 
 	// To ensure the user is given a decent error message if the platform
@@ -105,28 +129,25 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error {
 		return err
 	}
 
-	attrs := ast.Attributes
-	original := ast.Original
-	flags := ast.Flags
 	strList := []string{}
-	msg := fmt.Sprintf("Step %d/%d : %s", stepN+1, stepTotal, upperCasedCmd)
+	msg := bytes.NewBufferString(fmt.Sprintf("Step %d/%d : %s", stepN+1, stepTotal, upperCasedCmd))
 
-	if len(ast.Flags) > 0 {
-		msg += " " + strings.Join(ast.Flags, " ")
+	if len(node.Flags) > 0 {
+		msg.WriteString(strings.Join(node.Flags, " "))
 	}
 
+	ast := node
 	if cmd == "onbuild" {
 		if ast.Next == nil {
 			return errors.New("ONBUILD requires at least one argument")
 		}
 		ast = ast.Next.Children[0]
 		strList = append(strList, ast.Value)
-		msg += " " + ast.Value
+		msg.WriteString(" " + ast.Value)
 
 		if len(ast.Flags) > 0 {
-			msg += " " + strings.Join(ast.Flags, " ")
+			msg.WriteString(" " + strings.Join(ast.Flags, " "))
 		}
-
 	}
 
 	msgList := initMsgList(ast)
@@ -143,15 +164,13 @@ func (b *Builder) dispatch(stepN int, stepTotal int, ast *parser.Node) error {
 		msgList[i] = ast.Value
 	}
 
-	msg += " " + strings.Join(msgList, " ")
-	fmt.Fprintln(b.Stdout, msg)
+	msg.WriteString(" " + strings.Join(msgList, " "))
+	fmt.Fprintln(b.Stdout, msg.String())
 
 	// XXX yes, we skip any cmds that are not valid; the parser should have
 	// picked these out already.
 	if f, ok := evaluateTable[cmd]; ok {
-		b.flags = NewBFlags()
-		b.flags.Args = flags
-		return f(b, strList, attrs, original)
+		return f(newDispatchRequestFromNode(node, b, strList))
 	}
 
 	return fmt.Errorf("Unknown instruction: %s", upperCasedCmd)

integration-cli/docker_cli_build_test.go

@@ -505,7 +505,7 @@ func (s *DockerSuite) TestBuildAddSingleFileToWorkdir(c *check.C) {
 
 func (s *DockerSuite) TestBuildAddSingleFileToExistDir(c *check.C) {
 	testRequires(c, DaemonIsLinux) // Linux specific test
-	buildImageSuccessfully(c, "testaddsinglefiletoexistdir", build.WithBuildContext(c,
+	cli.BuildCmd(c, "testaddsinglefiletoexistdir", build.WithBuildContext(c,
 		build.WithFile("Dockerfile", `FROM busybox
 RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd
 RUN echo 'dockerio:x:1001:' >> /etc/group
@@ -561,13 +561,13 @@ func (s *DockerSuite) TestBuildUsernamespaceValidateRemappedRoot(c *check.C) {
 	}
 	name := "testbuildusernamespacevalidateremappedroot"
 	for _, tc := range testCases {
-		buildImageSuccessfully(c, name, build.WithBuildContext(c,
+		cli.BuildCmd(c, name, build.WithBuildContext(c,
 			build.WithFile("Dockerfile", fmt.Sprintf(`FROM busybox
 %s
 RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'root:root' ]`, tc)),
 			build.WithFile("test_dir/test_file", "test file")))
 
-		dockerCmd(c, "rmi", name)
+		cli.DockerCmd(c, "rmi", name)
 	}
 }
 
@@ -576,7 +576,7 @@ func (s *DockerSuite) TestBuildAddAndCopyFileWithWhitespace(c *check.C) {
 	name := "testaddfilewithwhitespace"
 
 	for _, command := range []string{"ADD", "COPY"} {
-		buildImageSuccessfully(c, name, build.WithBuildContext(c,
+		cli.BuildCmd(c, name, build.WithBuildContext(c,
 			build.WithFile("Dockerfile", fmt.Sprintf(`FROM busybox
 RUN mkdir "/test dir"
 RUN mkdir "/test_dir"
@@ -600,7 +600,7 @@ RUN [ $(cat "/test dir/test_file6") = 'test6' ]`, command, command, command, com
 			build.WithFile("test dir/test_file6", "test6"),
 		))
 
-		dockerCmd(c, "rmi", name)
+		cli.DockerCmd(c, "rmi", name)
 	}
 }
 
@@ -623,7 +623,7 @@ RUN find "test5" "C:/test dir/test_file5"
 RUN find "test6" "C:/test dir/test_file6"`
 
 	name := "testcopyfilewithwhitespace"
-	buildImageSuccessfully(c, name, build.WithBuildContext(c,
+	cli.BuildCmd(c, name, build.WithBuildContext(c,
 		build.WithFile("Dockerfile", dockerfile),
 		build.WithFile("test file1", "test1"),
 		build.WithFile("test_file2", "test2"),