Browse code
Test & Fix build with rm/force-rm matrix
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
Simon Ferquel authored on 2017/10/09 22:17:24Showing 4 changed files
- builder/dockerfile/builder.go
- builder/dockerfile/evaluator.go
- integration/build/build_test.go
- integration/build/main_test.go
| ... | ... |
@@ -322,7 +322,6 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions. |
| 322 | 322 |
if err := dispatch(dispatchRequest, cmd); err != nil {
|
| 323 | 323 |
return nil, err |
| 324 | 324 |
} |
| 325 |
- |
|
| 326 | 325 |
dispatchRequest.state.updateRunConfig() |
| 327 | 326 |
fmt.Fprintf(b.Stdout, " ---> %s\n", stringid.TruncateID(dispatchRequest.state.imageID)) |
| 328 | 327 |
|
| ... | ... |
@@ -335,9 +334,6 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions. |
| 335 | 335 |
return nil, err |
| 336 | 336 |
} |
| 337 | 337 |
} |
| 338 |
- if b.options.Remove {
|
|
| 339 |
- b.containerManager.RemoveAll(b.Stdout) |
|
| 340 |
- } |
|
| 341 | 338 |
buildArgs.WarnOnUnusedBuildArgs(b.Stdout) |
| 342 | 339 |
return dispatchRequest.state, nil |
| 343 | 340 |
} |
| ... | ... |
@@ -32,7 +32,7 @@ import ( |
| 32 | 32 |
"github.com/pkg/errors" |
| 33 | 33 |
) |
| 34 | 34 |
|
| 35 |
-func dispatch(d dispatchRequest, cmd instructions.Command) error {
|
|
| 35 |
+func dispatch(d dispatchRequest, cmd instructions.Command) (err error) {
|
|
| 36 | 36 |
if c, ok := cmd.(instructions.PlatformSpecific); ok {
|
| 37 | 37 |
optionsOS := system.ParsePlatform(d.builder.options.Platform).OS |
| 38 | 38 |
err := c.CheckPlatform(optionsOS) |
| ... | ... |
@@ -52,10 +52,16 @@ func dispatch(d dispatchRequest, cmd instructions.Command) error {
|
| 52 | 52 |
} |
| 53 | 53 |
} |
| 54 | 54 |
|
| 55 |
- if d.builder.options.ForceRemove {
|
|
| 56 |
- defer d.builder.containerManager.RemoveAll(d.builder.Stdout) |
|
| 57 |
- } |
|
| 58 |
- |
|
| 55 |
+ defer func() {
|
|
| 56 |
+ if d.builder.options.ForceRemove {
|
|
| 57 |
+ d.builder.containerManager.RemoveAll(d.builder.Stdout) |
|
| 58 |
+ return |
|
| 59 |
+ } |
|
| 60 |
+ if d.builder.options.Remove && err == nil {
|
|
| 61 |
+ d.builder.containerManager.RemoveAll(d.builder.Stdout) |
|
| 62 |
+ return |
|
| 63 |
+ } |
|
| 64 |
+ }() |
|
| 59 | 65 |
switch c := cmd.(type) {
|
| 60 | 66 |
case *instructions.EnvCommand: |
| 61 | 67 |
return dispatchEnv(d, c) |
| 62 | 68 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,130 @@ |
| 0 |
+package build |
|
| 1 |
+ |
|
| 2 |
+import ( |
|
| 3 |
+ "archive/tar" |
|
| 4 |
+ "bytes" |
|
| 5 |
+ "context" |
|
| 6 |
+ "encoding/json" |
|
| 7 |
+ "io" |
|
| 8 |
+ "strings" |
|
| 9 |
+ "testing" |
|
| 10 |
+ |
|
| 11 |
+ "github.com/docker/docker/api/types" |
|
| 12 |
+ "github.com/docker/docker/api/types/filters" |
|
| 13 |
+ "github.com/docker/docker/integration/util/request" |
|
| 14 |
+ "github.com/docker/docker/pkg/jsonmessage" |
|
| 15 |
+ "github.com/stretchr/testify/require" |
|
| 16 |
+) |
|
| 17 |
+ |
|
| 18 |
+func TestBuildWithRemoveAndForceRemove(t *testing.T) {
|
|
| 19 |
+ t.Parallel() |
|
| 20 |
+ cases := []struct {
|
|
| 21 |
+ name string |
|
| 22 |
+ dockerfile string |
|
| 23 |
+ numberOfIntermediateContainers int |
|
| 24 |
+ rm bool |
|
| 25 |
+ forceRm bool |
|
| 26 |
+ }{
|
|
| 27 |
+ {
|
|
| 28 |
+ name: "successful build with no removal", |
|
| 29 |
+ dockerfile: `FROM busybox |
|
| 30 |
+ RUN exit 0 |
|
| 31 |
+ RUN exit 0`, |
|
| 32 |
+ numberOfIntermediateContainers: 2, |
|
| 33 |
+ rm: false, |
|
| 34 |
+ forceRm: false, |
|
| 35 |
+ }, |
|
| 36 |
+ {
|
|
| 37 |
+ name: "successful build with remove", |
|
| 38 |
+ dockerfile: `FROM busybox |
|
| 39 |
+ RUN exit 0 |
|
| 40 |
+ RUN exit 0`, |
|
| 41 |
+ numberOfIntermediateContainers: 0, |
|
| 42 |
+ rm: true, |
|
| 43 |
+ forceRm: false, |
|
| 44 |
+ }, |
|
| 45 |
+ {
|
|
| 46 |
+ name: "successful build with remove and force remove", |
|
| 47 |
+ dockerfile: `FROM busybox |
|
| 48 |
+ RUN exit 0 |
|
| 49 |
+ RUN exit 0`, |
|
| 50 |
+ numberOfIntermediateContainers: 0, |
|
| 51 |
+ rm: true, |
|
| 52 |
+ forceRm: true, |
|
| 53 |
+ }, |
|
| 54 |
+ {
|
|
| 55 |
+ name: "failed build with no removal", |
|
| 56 |
+ dockerfile: `FROM busybox |
|
| 57 |
+ RUN exit 0 |
|
| 58 |
+ RUN exit 1`, |
|
| 59 |
+ numberOfIntermediateContainers: 2, |
|
| 60 |
+ rm: false, |
|
| 61 |
+ forceRm: false, |
|
| 62 |
+ }, |
|
| 63 |
+ {
|
|
| 64 |
+ name: "failed build with remove", |
|
| 65 |
+ dockerfile: `FROM busybox |
|
| 66 |
+ RUN exit 0 |
|
| 67 |
+ RUN exit 1`, |
|
| 68 |
+ numberOfIntermediateContainers: 1, |
|
| 69 |
+ rm: true, |
|
| 70 |
+ forceRm: false, |
|
| 71 |
+ }, |
|
| 72 |
+ {
|
|
| 73 |
+ name: "failed build with remove and force remove", |
|
| 74 |
+ dockerfile: `FROM busybox |
|
| 75 |
+ RUN exit 0 |
|
| 76 |
+ RUN exit 1`, |
|
| 77 |
+ numberOfIntermediateContainers: 0, |
|
| 78 |
+ rm: true, |
|
| 79 |
+ forceRm: true, |
|
| 80 |
+ }, |
|
| 81 |
+ } |
|
| 82 |
+ |
|
| 83 |
+ client := request.NewAPIClient(t) |
|
| 84 |
+ ctx := context.Background() |
|
| 85 |
+ for _, c := range cases {
|
|
| 86 |
+ t.Run(c.name, func(t *testing.T) {
|
|
| 87 |
+ t.Parallel() |
|
| 88 |
+ dockerfile := []byte(c.dockerfile) |
|
| 89 |
+ |
|
| 90 |
+ buff := bytes.NewBuffer(nil) |
|
| 91 |
+ tw := tar.NewWriter(buff) |
|
| 92 |
+ require.NoError(t, tw.WriteHeader(&tar.Header{
|
|
| 93 |
+ Name: "Dockerfile", |
|
| 94 |
+ Size: int64(len(dockerfile)), |
|
| 95 |
+ })) |
|
| 96 |
+ _, err := tw.Write(dockerfile) |
|
| 97 |
+ require.NoError(t, err) |
|
| 98 |
+ require.NoError(t, tw.Close()) |
|
| 99 |
+ resp, err := client.ImageBuild(ctx, buff, types.ImageBuildOptions{Remove: c.rm, ForceRemove: c.forceRm, NoCache: true})
|
|
| 100 |
+ require.NoError(t, err) |
|
| 101 |
+ defer resp.Body.Close() |
|
| 102 |
+ filter, err := buildContainerIdsFilter(resp.Body) |
|
| 103 |
+ require.NoError(t, err) |
|
| 104 |
+ remainingContainers, err := client.ContainerList(ctx, types.ContainerListOptions{Filters: filter, All: true})
|
|
| 105 |
+ require.NoError(t, err) |
|
| 106 |
+ require.Equal(t, c.numberOfIntermediateContainers, len(remainingContainers), "Expected %v remaining intermediate containers, got %v", c.numberOfIntermediateContainers, len(remainingContainers)) |
|
| 107 |
+ }) |
|
| 108 |
+ } |
|
| 109 |
+} |
|
| 110 |
+ |
|
| 111 |
+func buildContainerIdsFilter(buildOutput io.Reader) (filters.Args, error) {
|
|
| 112 |
+ const intermediateContainerPrefix = " ---> Running in " |
|
| 113 |
+ filter := filters.NewArgs() |
|
| 114 |
+ |
|
| 115 |
+ dec := json.NewDecoder(buildOutput) |
|
| 116 |
+ for {
|
|
| 117 |
+ m := jsonmessage.JSONMessage{}
|
|
| 118 |
+ err := dec.Decode(&m) |
|
| 119 |
+ if err == io.EOF {
|
|
| 120 |
+ return filter, nil |
|
| 121 |
+ } |
|
| 122 |
+ if err != nil {
|
|
| 123 |
+ return filter, err |
|
| 124 |
+ } |
|
| 125 |
+ if ix := strings.Index(m.Stream, intermediateContainerPrefix); ix != -1 {
|
|
| 126 |
+ filter.Add("id", strings.TrimSpace(m.Stream[ix+len(intermediateContainerPrefix):]))
|
|
| 127 |
+ } |
|
| 128 |
+ } |
|
| 129 |
+} |
| 0 | 130 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,28 @@ |
| 0 |
+package build |
|
| 1 |
+ |
|
| 2 |
+import ( |
|
| 3 |
+ "fmt" |
|
| 4 |
+ "os" |
|
| 5 |
+ "testing" |
|
| 6 |
+ |
|
| 7 |
+ "github.com/docker/docker/internal/test/environment" |
|
| 8 |
+) |
|
| 9 |
+ |
|
| 10 |
+var testEnv *environment.Execution |
|
| 11 |
+ |
|
| 12 |
+func TestMain(m *testing.M) {
|
|
| 13 |
+ var err error |
|
| 14 |
+ testEnv, err = environment.New() |
|
| 15 |
+ if err != nil {
|
|
| 16 |
+ fmt.Println(err) |
|
| 17 |
+ os.Exit(1) |
|
| 18 |
+ } |
|
| 19 |
+ |
|
| 20 |
+ testEnv.Print() |
|
| 21 |
+ os.Exit(m.Run()) |
|
| 22 |
+} |
|
| 23 |
+ |
|
| 24 |
+func setupTest(t *testing.T) func() {
|
|
| 25 |
+ environment.ProtectAll(t, testEnv) |
|
| 26 |
+ return func() { testEnv.Clean(t) }
|
|
| 27 |
+} |