From 172e73a1dfb894c54fdc93fa6ef338eb1374d06c Mon Sep 17 00:00:00 2001 From: Simon Ferquel Date: Mon, 9 Oct 2017 15:17:24 +0200 Subject: [PATCH] Test & Fix build with rm/force-rm matrix Signed-off-by: Simon Ferquel --- builder/dockerfile/builder.go | 4 - builder/dockerfile/evaluator.go | 16 ++-- integration/build/build_test.go | 130 ++++++++++++++++++++++++++++++++ integration/build/main_test.go | 28 +++++++ 4 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 integration/build/build_test.go create mode 100644 integration/build/main_test.go diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index a0610437c8..2b7b43827c 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -322,7 +322,6 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions. if err := dispatch(dispatchRequest, cmd); err != nil { return nil, err } - dispatchRequest.state.updateRunConfig() fmt.Fprintf(b.Stdout, " ---> %s\n", stringid.TruncateID(dispatchRequest.state.imageID)) @@ -335,9 +334,6 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions. return nil, err } } - if b.options.Remove { - b.containerManager.RemoveAll(b.Stdout) - } buildArgs.WarnOnUnusedBuildArgs(b.Stdout) return dispatchRequest.state, nil } diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index 0841bc7924..da97a7ff6e 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -32,7 +32,7 @@ import ( "github.com/pkg/errors" ) -func dispatch(d dispatchRequest, cmd instructions.Command) error { +func dispatch(d dispatchRequest, cmd instructions.Command) (err error) { if c, ok := cmd.(instructions.PlatformSpecific); ok { optionsOS := system.ParsePlatform(d.builder.options.Platform).OS err := c.CheckPlatform(optionsOS) @@ -52,10 +52,16 @@ func dispatch(d dispatchRequest, cmd instructions.Command) error { } } - if d.builder.options.ForceRemove { - defer d.builder.containerManager.RemoveAll(d.builder.Stdout) - } - + defer func() { + if d.builder.options.ForceRemove { + d.builder.containerManager.RemoveAll(d.builder.Stdout) + return + } + if d.builder.options.Remove && err == nil { + d.builder.containerManager.RemoveAll(d.builder.Stdout) + return + } + }() switch c := cmd.(type) { case *instructions.EnvCommand: return dispatchEnv(d, c) diff --git a/integration/build/build_test.go b/integration/build/build_test.go new file mode 100644 index 0000000000..5a23371bdc --- /dev/null +++ b/integration/build/build_test.go @@ -0,0 +1,130 @@ +package build + +import ( + "archive/tar" + "bytes" + "context" + "encoding/json" + "io" + "strings" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/integration/util/request" + "github.com/docker/docker/pkg/jsonmessage" + "github.com/stretchr/testify/require" +) + +func TestBuildWithRemoveAndForceRemove(t *testing.T) { + t.Parallel() + cases := []struct { + name string + dockerfile string + numberOfIntermediateContainers int + rm bool + forceRm bool + }{ + { + name: "successful build with no removal", + dockerfile: `FROM busybox + RUN exit 0 + RUN exit 0`, + numberOfIntermediateContainers: 2, + rm: false, + forceRm: false, + }, + { + name: "successful build with remove", + dockerfile: `FROM busybox + RUN exit 0 + RUN exit 0`, + numberOfIntermediateContainers: 0, + rm: true, + forceRm: false, + }, + { + name: "successful build with remove and force remove", + dockerfile: `FROM busybox + RUN exit 0 + RUN exit 0`, + numberOfIntermediateContainers: 0, + rm: true, + forceRm: true, + }, + { + name: "failed build with no removal", + dockerfile: `FROM busybox + RUN exit 0 + RUN exit 1`, + numberOfIntermediateContainers: 2, + rm: false, + forceRm: false, + }, + { + name: "failed build with remove", + dockerfile: `FROM busybox + RUN exit 0 + RUN exit 1`, + numberOfIntermediateContainers: 1, + rm: true, + forceRm: false, + }, + { + name: "failed build with remove and force remove", + dockerfile: `FROM busybox + RUN exit 0 + RUN exit 1`, + numberOfIntermediateContainers: 0, + rm: true, + forceRm: true, + }, + } + + client := request.NewAPIClient(t) + ctx := context.Background() + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + dockerfile := []byte(c.dockerfile) + + buff := bytes.NewBuffer(nil) + tw := tar.NewWriter(buff) + require.NoError(t, tw.WriteHeader(&tar.Header{ + Name: "Dockerfile", + Size: int64(len(dockerfile)), + })) + _, err := tw.Write(dockerfile) + require.NoError(t, err) + require.NoError(t, tw.Close()) + resp, err := client.ImageBuild(ctx, buff, types.ImageBuildOptions{Remove: c.rm, ForceRemove: c.forceRm, NoCache: true}) + require.NoError(t, err) + defer resp.Body.Close() + filter, err := buildContainerIdsFilter(resp.Body) + require.NoError(t, err) + remainingContainers, err := client.ContainerList(ctx, types.ContainerListOptions{Filters: filter, All: true}) + require.NoError(t, err) + require.Equal(t, c.numberOfIntermediateContainers, len(remainingContainers), "Expected %v remaining intermediate containers, got %v", c.numberOfIntermediateContainers, len(remainingContainers)) + }) + } +} + +func buildContainerIdsFilter(buildOutput io.Reader) (filters.Args, error) { + const intermediateContainerPrefix = " ---> Running in " + filter := filters.NewArgs() + + dec := json.NewDecoder(buildOutput) + for { + m := jsonmessage.JSONMessage{} + err := dec.Decode(&m) + if err == io.EOF { + return filter, nil + } + if err != nil { + return filter, err + } + if ix := strings.Index(m.Stream, intermediateContainerPrefix); ix != -1 { + filter.Add("id", strings.TrimSpace(m.Stream[ix+len(intermediateContainerPrefix):])) + } + } +} diff --git a/integration/build/main_test.go b/integration/build/main_test.go new file mode 100644 index 0000000000..007f2d98e5 --- /dev/null +++ b/integration/build/main_test.go @@ -0,0 +1,28 @@ +package build + +import ( + "fmt" + "os" + "testing" + + "github.com/docker/docker/internal/test/environment" +) + +var testEnv *environment.Execution + +func TestMain(m *testing.M) { + var err error + testEnv, err = environment.New() + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + testEnv.Print() + os.Exit(m.Run()) +} + +func setupTest(t *testing.T) func() { + environment.ProtectAll(t, testEnv) + return func() { testEnv.Clean(t) } +}