kotovalexarian-likes-github
/
moby--moby
mirror of https://github.com/moby/moby.git
1
0
Fork 0
Code Releases Activity

Merge pull request #32532 from dnephin/refactor-some-builder-parts 

[builder] cleanup env dispatcher and Builder.build()
This commit is contained in:
Tõnis Tiigi 2017-04-12 11:11:06 -07:00 committed by GitHub
parent 480d1b841f 068f344e03
commit d2ab40e5b9
11 changed files with 159 additions and 172 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
api/types/backend/backend.go
View File

@ -100,8 +100,7 @@ type ContainerCommitConfig struct {
Changes []string
}
// ProgressWriter is an interface
// to transport progress streams.
// ProgressWriter is a data object to transport progress streams to the client
type ProgressWriter struct {
Output io.Writer
StdoutFormatter *streamformatter.StdoutFormatter

145
builder/dockerfile/builder.go
View File

@ -2,7 +2,6 @@ package dockerfile
import (
"bytes"
"errors"
"fmt"
"io"
"io/ioutil"
@ -20,7 +19,7 @@ import (
"github.com/docker/docker/builder/dockerfile/parser"
"github.com/docker/docker/image"
"github.com/docker/docker/pkg/stringid"
perrors "github.com/pkg/errors"
"github.com/pkg/errors"
"golang.org/x/net/context"
)
@ -51,7 +50,6 @@ type Builder struct {
docker builder.Backend
context builder.Context
clientCtx context.Context
cancel context.CancelFunc
runConfig *container.Config // runconfig for cmd, run, entrypoint etc.
flags *BFlags
@ -66,9 +64,6 @@ type Builder struct {
buildArgs *buildArgs
directive parser.Directive
// TODO: remove once docker.Commit can receive a tag
id string
imageCache builder.ImageCache
from builder.Image
}
@ -117,10 +112,8 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back
if config == nil {
config = new(types.ImageBuildOptions)
}
ctx, cancel := context.WithCancel(clientCtx)
b = &Builder{
clientCtx: ctx,
cancel: cancel,
clientCtx: clientCtx,
options: config,
Stdout: os.Stdout,
Stderr: os.Stderr,
@ -128,7 +121,6 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back
context: buildContext,
runConfig: new(container.Config),
tmpContainers: map[string]struct{}{},
id: stringid.GenerateNonCryptoID(),
buildArgs: newBuildArgs(config.BuildArgs),
directive: parser.Directive{
EscapeSeen: false,
@ -186,17 +178,6 @@ func sanitizeRepoAndTags(names []string) ([]reference.Named, error) {
// build runs the Dockerfile builder from a context and a docker object that allows to make calls
// to Docker.
//
// This will (barring errors):
//
// * read the dockerfile from context
// * parse the dockerfile if not already parsed
// * walk the AST and execute it by dispatching to handlers. If Remove
// or ForceRemove is set, additional cleanup around containers happens after
// processing.
// * Tag image, if applicable.
// * Print a happy message and return the image ID.
//
func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) {
defer b.imageContexts.unmount()
@ -204,7 +185,7 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
b.Stderr = stderr
b.Output = out
dockerfile, err := b.readDockerfile()
dockerfile, err := b.readAndParseDockerfile()
if err != nil {
return "", err
}
@ -216,14 +197,37 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
addNodesForLabelOption(dockerfile, b.options.Labels)
var shortImgID string
total := len(dockerfile.Children)
for _, n := range dockerfile.Children {
if err := b.checkDispatch(n, false); err != nil {
return "", perrors.Wrapf(err, "Dockerfile parse error line %d", n.StartLine)
if err := checkDispatchDockerfile(dockerfile); err != nil {
return "", err
}
shortImageID, err := b.dispatchDockerfileWithCancellation(dockerfile)
if err != nil {
return "", err
}
b.warnOnUnusedBuildArgs()
if b.image == "" {
return "", errors.New("No image was generated. Is your Dockerfile empty?")
}
if b.options.Squash {
if err := b.squashBuild(); err != nil {
return "", err
}
}
fmt.Fprintf(b.Stdout, "Successfully built %s\n", shortImageID)
if err := b.tagImages(repoAndTags); err != nil {
return "", err
}
return b.image, nil
}
func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Node) (string, error) {
total := len(dockerfile.Children)
var shortImgID string
for i, n := range dockerfile.Children {
select {
case <-b.clientCtx.Done():
@ -253,37 +257,23 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri
}
if b.options.Target != "" && !b.imageContexts.isCurrentTarget(b.options.Target) {
return "", perrors.Errorf("failed to reach build target %s in Dockerfile", b.options.Target)
return "", errors.Errorf("failed to reach build target %s in Dockerfile", b.options.Target)
}
b.warnOnUnusedBuildArgs()
return shortImgID, nil
}
if b.image == "" {
return "", errors.New("No image was generated. Is your Dockerfile empty?")
func (b *Builder) squashBuild() error {
var fromID string
var err error
if b.from != nil {
fromID = b.from.ImageID()
}
if b.options.Squash {
var fromID string
if b.from != nil {
fromID = b.from.ImageID()
}
b.image, err = b.docker.SquashImage(b.image, fromID)
if err != nil {
return "", perrors.Wrap(err, "error squashing image")
}
b.image, err = b.docker.SquashImage(b.image, fromID)
if err != nil {
return errors.Wrap(err, "error squashing image")
}
fmt.Fprintf(b.Stdout, "Successfully built %s\n", shortImgID)
imageID := image.ID(b.image)
for _, rt := range repoAndTags {
if err := b.docker.TagImageWithReference(imageID, rt); err != nil {
return "", err
}
fmt.Fprintf(b.Stdout, "Successfully tagged %s\n", reference.FamiliarString(rt))
}
return b.image, nil
return nil
}
func addNodesForLabelOption(dockerfile *parser.Node, labels map[string]string) {
@ -304,16 +294,22 @@ func (b *Builder) warnOnUnusedBuildArgs() {
}
}
func (b *Builder) tagImages(repoAndTags []reference.Named) error {
imageID := image.ID(b.image)
for _, rt := range repoAndTags {
if err := b.docker.TagImageWithReference(imageID, rt); err != nil {
return err
}
fmt.Fprintf(b.Stdout, "Successfully tagged %s\n", reference.FamiliarString(rt))
}
return nil
}
// hasFromImage returns true if the builder has processed a `FROM <image>` line
func (b *Builder) hasFromImage() bool {
return b.image != "" || b.noBaseImage
}
// Cancel cancels an ongoing Dockerfile build.
func (b *Builder) Cancel() {
b.cancel()
}
// BuildFromConfig builds directly from `changes`, treating it as if it were the contents of a Dockerfile
// It will:
// - Call parse.Parse() to get an AST root for the concatenated Dockerfile entries.
@ -346,18 +342,31 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con
b.Stderr = ioutil.Discard
b.disableCommit = true
total := len(ast.Children)
for _, n := range ast.Children {
if err := b.checkDispatch(n, false); err != nil {
return nil, err
}
if err := checkDispatchDockerfile(ast); err != nil {
return nil, err
}
for i, n := range ast.Children {
if err := b.dispatch(i, total, n); err != nil {
return nil, err
}
if err := dispatchFromDockerfile(b, ast); err != nil {
return nil, err
}
return b.runConfig, nil
}
func checkDispatchDockerfile(dockerfile *parser.Node) error {
for _, n := range dockerfile.Children {
if err := checkDispatch(n); err != nil {
return errors.Wrapf(err, "Dockerfile parse error line %d", n.StartLine)
}
}
return nil
}
func dispatchFromDockerfile(b *Builder, ast *parser.Node) error {
total := len(ast.Children)
for i, n := range ast.Children {
if err := b.dispatch(i, total, n); err != nil {
return err
}
}
return nil
}

42
builder/dockerfile/dispatchers.go
View File

@ -16,6 +16,7 @@ import (
"strings"
"time"
"bytes"
"github.com/Sirupsen/logrus"
"github.com/docker/docker/api"
"github.com/docker/docker/api/types"
@ -46,45 +47,22 @@ func env(b *Builder, args []string, attributes map[string]bool, original string)
return err
}
// TODO/FIXME/NOT USED
// Just here to show how to use the builder flags stuff within the
// context of a builder command. Will remove once we actually add
// a builder command to something!
/*
flBool1 := b.flags.AddBool("bool1", false)
flStr1 := b.flags.AddString("str1", "HI")
if err := b.flags.Parse(); err != nil {
return err
}
fmt.Printf("Bool1:%v\n", flBool1)
fmt.Printf("Str1:%v\n", flStr1)
*/
commitStr := "ENV"
for j := 0; j < len(args); j++ {
// name ==> args[j]
// value ==> args[j+1]
commitMessage := bytes.NewBufferString("ENV")
for j := 0; j < len(args); j += 2 {
if len(args[j]) == 0 {
return errBlankCommandNames("ENV")
}
newVar := args[j] + "=" + args[j+1] + ""
commitStr += " " + newVar
name := args[j]
value := args[j+1]
newVar := name + "=" + value
commitMessage.WriteString(" " + newVar)
gotOne := false
for i, envVar := range b.runConfig.Env {
envParts := strings.SplitN(envVar, "=", 2)
compareFrom := envParts[0]
compareTo := args[j]
if runtime.GOOS == "windows" {
// Case insensitive environment variables on Windows
compareFrom = strings.ToUpper(compareFrom)
compareTo = strings.ToUpper(compareTo)
}
if compareFrom == compareTo {
if equalEnvKeys(compareFrom, name) {
b.runConfig.Env[i] = newVar
gotOne = true
break
@ -93,10 +71,9 @@ func env(b *Builder, args []string, attributes map[string]bool, original string)
if !gotOne {
b.runConfig.Env = append(b.runConfig.Env, newVar)
}
j++
}
return b.commit("", b.runConfig.Cmd, commitStr)
return b.commit("", b.runConfig.Cmd, commitMessage.String())
}
// MAINTAINER some text <maybe@an.email.address>
@ -440,6 +417,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
return err
}
// FIXME: this is duplicated with the defer above in this function (i think?)
// 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.

37
builder/dockerfile/dispatchers_test.go
View File

@ -132,27 +132,34 @@ func TestCommandseBlankNames(t *testing.T) {
}
func TestEnv2Variables(t *testing.T) {
variables := []string{"var1", "val1", "var2", "val2"}
b := newBuilderWithMockBackend()
b.disableCommit = true
bflags := &BFlags{}
config := &container.Config{}
args := []string{"var1", "val1", "var2", "val2"}
err := env(b, args, nil, "")
assert.NilError(t, err)
b := &Builder{flags: bflags, runConfig: config, disableCommit: true}
if err := env(b, variables, nil, ""); err != nil {
t.Fatalf("Error when executing env: %s", err.Error())
expected := []string{
fmt.Sprintf("%s=%s", args[0], args[1]),
fmt.Sprintf("%s=%s", args[2], args[3]),
}
assert.DeepEqual(t, b.runConfig.Env, expected)
}
expectedVar1 := fmt.Sprintf("%s=%s", variables[0], variables[1])
expectedVar2 := fmt.Sprintf("%s=%s", variables[2], variables[3])
func TestEnvValueWithExistingRunConfigEnv(t *testing.T) {
b := newBuilderWithMockBackend()
b.disableCommit = true
b.runConfig.Env = []string{"var1=old", "var2=fromenv"}
if b.runConfig.Env[0] != expectedVar1 {
t.Fatalf("Wrong env output for first variable. Got: %s. Should be: %s", b.runConfig.Env[0], expectedVar1)
}
if b.runConfig.Env[1] != expectedVar2 {
t.Fatalf("Wrong env output for second variable. Got: %s, Should be: %s", b.runConfig.Env[1], expectedVar2)
args := []string{"var1", "val1"}
err := env(b, args, nil, "")
assert.NilError(t, err)
expected := []string{
fmt.Sprintf("%s=%s", args[0], args[1]),
"var2=fromenv",
}
assert.DeepEqual(t, b.runConfig.Env, expected)
}
func TestMaintainer(t *testing.T) {

6
builder/dockerfile/dispatchers_unix.go
View File

@ -26,3 +26,9 @@ func normaliseWorkdir(current string, requested string) (string, error) {
func errNotJSON(command, _ string) error {
return fmt.Errorf("%s requires the arguments to be in JSON form", command)
}
// equalEnvKeys compare two strings and returns true if they are equal. On
// Windows this comparison is case insensitive.
func equalEnvKeys(from, to string) bool {
return from == to
}

6
builder/dockerfile/dispatchers_windows.go
View File

@ -85,3 +85,9 @@ func errNotJSON(command, original string) error {
}
return fmt.Errorf("%s requires the arguments to be in JSON form%s", command, extra)
}
// equalEnvKeys compare two strings and returns true if they are equal. On
// Windows this comparison is case insensitive.
func equalEnvKeys(from, to string) bool {
return strings.ToUpper(from) == strings.ToUpper(to)
}

25
builder/dockerfile/evaluator.go
View File

@ -20,13 +20,13 @@
package dockerfile
import (
"errors"
"fmt"
"strings"
"github.com/docker/docker/builder/dockerfile/command"
"github.com/docker/docker/builder/dockerfile/parser"
runconfigopts "github.com/docker/docker/runconfig/opts"
"github.com/docker/docker/runconfig/opts"
"github.com/pkg/errors"
)
// Environment variable interpolation will happen on these statements only.
@ -187,7 +187,7 @@ func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string,
// args that are not overriden by runConfig environment variables.
func (b *Builder) buildArgsWithoutConfigEnv() []string {
envs := []string{}
configEnv := runconfigopts.ConvertKVStringsToMap(b.runConfig.Env)
configEnv := b.runConfigEnvMapping()
for key, val := range b.buildArgs.GetAllAllowed() {
if _, ok := configEnv[key]; !ok {
@ -197,13 +197,16 @@ func (b *Builder) buildArgsWithoutConfigEnv() []string {
return envs
}
func (b *Builder) runConfigEnvMapping() map[string]string {
return opts.ConvertKVStringsToMap(b.runConfig.Env)
}
// checkDispatch does a simple check for syntax errors of the Dockerfile.
// Because some of the instructions can only be validated through runtime,
// arg, env, etc., this syntax check will not be complete and could not replace
// the runtime check. Instead, this function is only a helper that allows
// user to find out the obvious error in Dockerfile earlier on.
// onbuild bool: indicate if instruction XXX is part of `ONBUILD XXX` trigger
func (b *Builder) checkDispatch(ast *parser.Node, onbuild bool) error {
func checkDispatch(ast *parser.Node) error {
cmd := ast.Value
upperCasedCmd := strings.ToUpper(cmd)
@ -221,19 +224,9 @@ func (b *Builder) checkDispatch(ast *parser.Node, onbuild bool) error {
}
}
// The instruction is part of ONBUILD trigger (not the instruction itself)
if onbuild {
switch upperCasedCmd {
case "ONBUILD":
return errors.New("Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed")
case "MAINTAINER", "FROM":
return fmt.Errorf("%s isn't allowed as an ONBUILD trigger", upperCasedCmd)
}
}
if _, ok := evaluateTable[cmd]; ok {
return nil
}
return fmt.Errorf("Unknown instruction: %s", upperCasedCmd)
return errors.Errorf("unknown instruction: %s", upperCasedCmd)
}

30
builder/dockerfile/internals.go
View File

@ -35,7 +35,6 @@ import (
"github.com/docker/docker/pkg/system"
"github.com/docker/docker/pkg/tarsum"
"github.com/docker/docker/pkg/urlutil"
"github.com/docker/docker/runconfig/opts"
"github.com/pkg/errors"
)
@ -433,11 +432,7 @@ func (b *Builder) processImageFrom(img builder.Image) error {
// Check to see if we have a default PATH, note that windows won't
// have one as it's set by HCS
if system.DefaultPathEnv != "" {
// Convert the slice of strings that represent the current list
// of env vars into a map so we can see if PATH is already set.
// If it's not set then go ahead and give it our default value
configEnv := opts.ConvertKVStringsToMap(b.runConfig.Env)
if _, ok := configEnv["PATH"]; !ok {
if _, ok := b.runConfigEnvMapping()["PATH"]; !ok {
b.runConfig.Env = append(b.runConfig.Env,
"PATH="+system.DefaultPathEnv)
}
@ -472,19 +467,24 @@ func (b *Builder) processImageFrom(img builder.Image) error {
return err
}
total := len(ast.Children)
for _, n := range ast.Children {
if err := b.checkDispatch(n, true); err != nil {
if err := checkDispatch(n); err != nil {
return err
}
upperCasedCmd := strings.ToUpper(n.Value)
switch upperCasedCmd {
case "ONBUILD":
return errors.New("Chaining ONBUILD via `ONBUILD ONBUILD` isn't allowed")
case "MAINTAINER", "FROM":
return errors.Errorf("%s isn't allowed as an ONBUILD trigger", upperCasedCmd)
}
}
for i, n := range ast.Children {
if err := b.dispatch(i, total, n); err != nil {
return err
}
if err := dispatchFromDockerfile(b, ast); err != nil {
return err
}
}
return nil
}
@ -649,8 +649,8 @@ func (b *Builder) clearTmp() {
}
}
// readDockerfile reads a Dockerfile from the current context.
func (b *Builder) readDockerfile() (*parser.Node, error) {
// readAndParseDockerfile reads a Dockerfile from the current context.
func (b *Builder) readAndParseDockerfile() (*parser.Node, error) {
// If no -f was specified then look for 'Dockerfile'. If we can't find
// that then look for 'dockerfile'. If neither are found then default
// back to 'Dockerfile' and use that in the error message.

2
builder/dockerfile/internals_test.go
View File

@ -77,6 +77,6 @@ func readAndCheckDockerfile(t *testing.T, testName, contextDir, dockerfilePath,
b := &Builder{options: options, context: context}
_, err = b.readDockerfile()
_, err = b.readAndParseDockerfile()
assert.Error(t, err, expectedError)
}

15
builder/dockerfile/shell_parser.go
View File

@ -8,7 +8,6 @@ package dockerfile
import (
"fmt"
"runtime"
"strings"
"text/scanner"
"unicode"
@ -296,17 +295,10 @@ func (sw *shellWord) processName() string {
}
func (sw *shellWord) getEnv(name string) string {
if runtime.GOOS == "windows" {
// Case-insensitive environment variables on Windows
name = strings.ToUpper(name)
}
for _, env := range sw.envs {
i := strings.Index(env, "=")
if i < 0 {
if runtime.GOOS == "windows" {
env = strings.ToUpper(env)
}
if name == env {
if equalEnvKeys(name, env) {
// Should probably never get here, but just in case treat
// it like "var" and "var=" are the same
return ""
@ -314,10 +306,7 @@ func (sw *shellWord) getEnv(name string) string {
continue
}
compareName := env[:i]
if runtime.GOOS == "windows" {
compareName = strings.ToUpper(compareName)
}
if name != compareName {
if !equalEnvKeys(name, compareName) {
continue
}
return env[i+1:]

20
integration-cli/docker_cli_build_test.go
View File

@ -6133,7 +6133,7 @@ func (s *DockerSuite) TestBuildCopyFromPreviousFromWindows(c *check.C) {
func (s *DockerSuite) TestBuildCopyFromForbidWindowsSystemPaths(c *check.C) {
testRequires(c, DaemonIsWindows)
dockerfile := `
FROM ` + testEnv.MinimalBaseImage() + `
FROM ` + testEnv.MinimalBaseImage() + `
FROM ` + testEnv.MinimalBaseImage() + `
COPY --from=0 %s c:\\oscopy
`
@ -6150,7 +6150,7 @@ func (s *DockerSuite) TestBuildCopyFromForbidWindowsSystemPaths(c *check.C) {
func (s *DockerSuite) TestBuildCopyFromForbidWindowsRelativePaths(c *check.C) {
testRequires(c, DaemonIsWindows)
dockerfile := `
FROM ` + testEnv.MinimalBaseImage() + `
FROM ` + testEnv.MinimalBaseImage() + `
FROM ` + testEnv.MinimalBaseImage() + `
COPY --from=0 %s c:\\oscopy
`
@ -6169,7 +6169,7 @@ func (s *DockerSuite) TestBuildCopyFromWindowsIsCaseInsensitive(c *check.C) {
testRequires(c, DaemonIsWindows)
dockerfile := `
FROM ` + testEnv.MinimalBaseImage() + `
COPY foo /
COPY foo /
FROM ` + testEnv.MinimalBaseImage() + `
COPY --from=0 c:\\fOo c:\\copied
RUN type c:\\copied
@ -6322,23 +6322,23 @@ func (s *DockerSuite) TestBuildLineErrorOnBuild(c *check.C) {
}
// FIXME(vdemeester) should be a unit test
func (s *DockerSuite) TestBuildLineErrorUknownInstruction(c *check.C) {
func (s *DockerSuite) TestBuildLineErrorUnknownInstruction(c *check.C) {
name := "test_build_line_error_unknown_instruction"
buildImage(name, build.WithDockerfile(`FROM busybox
cli.Docker(cli.Build(name), build.WithDockerfile(`FROM busybox
RUN echo hello world
NOINSTRUCTION echo ba
RUN echo hello
ERROR
`)).Assert(c, icmd.Expected{
ExitCode: 1,
Err: "Dockerfile parse error line 3: Unknown instruction: NOINSTRUCTION",
Err: "Dockerfile parse error line 3: unknown instruction: NOINSTRUCTION",
})
}
// FIXME(vdemeester) should be a unit test
func (s *DockerSuite) TestBuildLineErrorWithEmptyLines(c *check.C) {
name := "test_build_line_error_with_empty_lines"
buildImage(name, build.WithDockerfile(`
cli.Docker(cli.Build(name), build.WithDockerfile(`
FROM busybox
RUN echo hello world
@ -6348,21 +6348,21 @@ func (s *DockerSuite) TestBuildLineErrorWithEmptyLines(c *check.C) {
CMD ["/bin/init"]
`)).Assert(c, icmd.Expected{
ExitCode: 1,
Err: "Dockerfile parse error line 6: Unknown instruction: NOINSTRUCTION",
Err: "Dockerfile parse error line 6: unknown instruction: NOINSTRUCTION",
})
}
// FIXME(vdemeester) should be a unit test
func (s *DockerSuite) TestBuildLineErrorWithComments(c *check.C) {
name := "test_build_line_error_with_comments"
buildImage(name, build.WithDockerfile(`FROM busybox
cli.Docker(cli.Build(name), build.WithDockerfile(`FROM busybox
# This will print hello world
# and then ba
RUN echo hello world
NOINSTRUCTION echo ba
`)).Assert(c, icmd.Expected{
ExitCode: 1,
Err: "Dockerfile parse error line 5: Unknown instruction: NOINSTRUCTION",
Err: "Dockerfile parse error line 5: unknown instruction: NOINSTRUCTION",
})
}