From 0b32e0ce4f6c71f205956823ad4de001d8f5b6bf Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Mon, 7 May 2018 13:28:55 +0800 Subject: [PATCH 1/7] Add 'LABEL' command from '--label' to the last stage This PR is tring to fix issue #36996. Currently for multi-stage build, if `--target` specified, the `--label` option will be ignored. The root cause is the last stage build will remove the `LABEL` command(s) node created from the `--label` option. In order to address this issue, we can create `LABEL` command(s) and add it/tem to the last stage. Signed-off-by: Dennis Chen Upstream-commit: 9c238ebd55e4105ad7f7edc04231ea61bb278ae8 Component: engine --- .../engine/builder/dockerfile/builder.go | 21 ++++++++++++++++-- .../dockerfile/instructions/commands.go | 22 ++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/components/engine/builder/dockerfile/builder.go b/components/engine/builder/dockerfile/builder.go index 21d84cb513..c63661e7f8 100644 --- a/components/engine/builder/dockerfile/builder.go +++ b/components/engine/builder/dockerfile/builder.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "io/ioutil" + "sort" "strings" "time" @@ -208,13 +209,26 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder { return b } +// Build 'LABEL' command(s) from '--label' options and add to the last stage +func buildLabelOptions(labels map[string]string, stages []instructions.Stage) { + keys := []string{} + for key := range labels { + keys = append(keys, key) + } + + // Sort the label to have a repeatable order + sort.Strings(keys) + for _, key := range keys { + value := labels[key] + stages[len(stages)-1].AddCommand(instructions.NewLabelCommand(key, value, true)) + } +} + // Build runs the Dockerfile builder by parsing the Dockerfile and executing // the instructions from the file. func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*builder.Result, error) { defer b.imageSources.Unmount() - addNodesForLabelOption(dockerfile.AST, b.options.Labels) - stages, metaArgs, err := instructions.Parse(dockerfile.AST) if err != nil { if instructions.IsUnknownInstruction(err) { @@ -231,6 +245,9 @@ func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*buil stages = stages[:targetIx+1] } + // Add 'LABEL' command specified by '--label' option to the last stage + buildLabelOptions(b.options.Labels, stages) + dockerfile.PrintWarnings(b.Stderr) dispatchState, err := b.dispatchDockerfileWithCancellation(stages, metaArgs, dockerfile.EscapeToken, source) if err != nil { diff --git a/components/engine/builder/dockerfile/instructions/commands.go b/components/engine/builder/dockerfile/instructions/commands.go index 9d864e5325..633a2b3fc7 100644 --- a/components/engine/builder/dockerfile/instructions/commands.go +++ b/components/engine/builder/dockerfile/instructions/commands.go @@ -110,17 +110,37 @@ type MaintainerCommand struct { Maintainer string } +// NewLabelCommand creates a new 'LABEL' command +func NewLabelCommand(k string, v string, NoExp bool) *LabelCommand { + kvp := KeyValuePair{Key: k, Value: v} + c := "LABEL " + c += kvp.String() + nc := withNameAndCode{code: c, name: "label"} + cmd := &LabelCommand{ + withNameAndCode: nc, + Labels: KeyValuePairs{ + kvp, + }, + noExpand: NoExp, + } + return cmd +} + // LabelCommand : LABEL some json data describing the image // // Sets the Label variable foo to bar, // type LabelCommand struct { withNameAndCode - Labels KeyValuePairs // kvp slice instead of map to preserve ordering + Labels KeyValuePairs // kvp slice instead of map to preserve ordering + noExpand bool } // Expand variables func (c *LabelCommand) Expand(expander SingleWordExpander) error { + if c.noExpand { + return nil + } return expandKvpsInPlace(c.Labels, expander) } From e46ae6ff6cdb8af50c19b6792d75b122dded09d6 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Mon, 7 May 2018 15:06:07 +0800 Subject: [PATCH 2/7] Add test case for `--label` with `--target` Add a new test case `TestBuildLabelWithTargets` to cover the Docker builder with both `--label` and `--target` options. Signed-off-by: Dennis Chen Upstream-commit: f7add4262b69294aa8035c4794236193ce4bc68e Component: engine --- .../engine/integration/build/build_test.go | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/components/engine/integration/build/build_test.go b/components/engine/integration/build/build_test.go index 52c03b4734..c0aea0fe6b 100644 --- a/components/engine/integration/build/build_test.go +++ b/components/engine/integration/build/build_test.go @@ -173,6 +173,81 @@ func TestBuildMultiStageParentConfig(t *testing.T) { assert.Check(t, is.Contains(image.Config.Env, "WHO=parent")) } +// Test cases in #36996 +func TestBuildLabelWithTargets(t *testing.T) { + bldName := "build-a" + testLabels := map[string]string{ + "foo": "bar", + "dead": "beef", + } + + dockerfile := ` + FROM busybox AS target-a + CMD ["/dev"] + LABEL label-a=inline-a + FROM busybox AS target-b + CMD ["/dist"] + LABEL label-b=inline-b + ` + + ctx := context.Background() + source := fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile)) + defer source.Close() + + apiclient := testEnv.APIClient() + // For `target-a` build + resp, err := apiclient.ImageBuild(ctx, + source.AsTarReader(t), + types.ImageBuildOptions{ + Remove: true, + ForceRemove: true, + Tags: []string{bldName}, + Labels: testLabels, + Target: "target-a", + }) + assert.NilError(t, err) + _, err = io.Copy(ioutil.Discard, resp.Body) + resp.Body.Close() + assert.NilError(t, err) + + image, _, err := apiclient.ImageInspectWithRaw(ctx, bldName) + assert.NilError(t, err) + + testLabels["label-a"] = "inline-a" + for k, v := range testLabels { + x, ok := image.Config.Labels[k] + assert.Assert(t, ok) + assert.Assert(t, x == v) + } + + // For `target-b` build + bldName = "build-b" + delete(testLabels, "label-a") + resp, err = apiclient.ImageBuild(ctx, + source.AsTarReader(t), + types.ImageBuildOptions{ + Remove: true, + ForceRemove: true, + Tags: []string{bldName}, + Labels: testLabels, + Target: "target-b", + }) + assert.NilError(t, err) + _, err = io.Copy(ioutil.Discard, resp.Body) + resp.Body.Close() + assert.NilError(t, err) + + image, _, err = apiclient.ImageInspectWithRaw(ctx, bldName) + assert.NilError(t, err) + + testLabels["label-b"] = "inline-b" + for k, v := range testLabels { + x, ok := image.Config.Labels[k] + assert.Assert(t, ok) + assert.Assert(t, x == v) + } +} + func TestBuildWithEmptyLayers(t *testing.T) { dockerfile := ` FROM busybox From 7273d67395233f2cf83c977af9662cdf3e9da8f6 Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Tue, 8 May 2018 17:15:57 +0800 Subject: [PATCH 3/7] Remove unused 'label' related functions Since we use `NewLabelCommand()` instead of `addNodesForLabelOption()` to create the 'LABEL' commands from '--label' options, so all the related functions should be removed. Signed-off-by: Dennis Chen Upstream-commit: c7b543164daed58fbea36471592438b4e53ab748 Component: engine --- .../engine/builder/dockerfile/builder.go | 9 ----- .../engine/builder/dockerfile/builder_test.go | 35 ------------------- .../builder/dockerfile/parser/line_parsers.go | 31 ---------------- .../dockerfile/parser/line_parsers_test.go | 26 -------------- 4 files changed, 101 deletions(-) delete mode 100644 components/engine/builder/dockerfile/builder_test.go diff --git a/components/engine/builder/dockerfile/builder.go b/components/engine/builder/dockerfile/builder.go index c63661e7f8..1455fd966e 100644 --- a/components/engine/builder/dockerfile/builder.go +++ b/components/engine/builder/dockerfile/builder.go @@ -350,15 +350,6 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions. return dispatchRequest.state, nil } -func addNodesForLabelOption(dockerfile *parser.Node, labels map[string]string) { - if len(labels) == 0 { - return - } - - node := parser.NodeFromLabels(labels) - dockerfile.Children = append(dockerfile.Children, node) -} - // 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. diff --git a/components/engine/builder/dockerfile/builder_test.go b/components/engine/builder/dockerfile/builder_test.go deleted file mode 100644 index 6c73b6cced..0000000000 --- a/components/engine/builder/dockerfile/builder_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package dockerfile // import "github.com/docker/docker/builder/dockerfile" - -import ( - "strings" - "testing" - - "github.com/docker/docker/builder/dockerfile/parser" - "github.com/gotestyourself/gotestyourself/assert" - is "github.com/gotestyourself/gotestyourself/assert/cmp" -) - -func TestAddNodesForLabelOption(t *testing.T) { - dockerfile := "FROM scratch" - result, err := parser.Parse(strings.NewReader(dockerfile)) - assert.Check(t, err) - - labels := map[string]string{ - "org.e": "cli-e", - "org.d": "cli-d", - "org.c": "cli-c", - "org.b": "cli-b", - "org.a": "cli-a", - } - nodes := result.AST - addNodesForLabelOption(nodes, labels) - - expected := []string{ - "FROM scratch", - `LABEL "org.a"='cli-a' "org.b"='cli-b' "org.c"='cli-c' "org.d"='cli-d' "org.e"='cli-e'`, - } - assert.Check(t, is.Len(nodes.Children, 2)) - for i, v := range nodes.Children { - assert.Check(t, is.Equal(expected[i], v.Original)) - } -} diff --git a/components/engine/builder/dockerfile/parser/line_parsers.go b/components/engine/builder/dockerfile/parser/line_parsers.go index 94091f5f6b..c454835373 100644 --- a/components/engine/builder/dockerfile/parser/line_parsers.go +++ b/components/engine/builder/dockerfile/parser/line_parsers.go @@ -10,12 +10,9 @@ import ( "encoding/json" "errors" "fmt" - "sort" "strings" "unicode" "unicode/utf8" - - "github.com/docker/docker/builder/dockerfile/command" ) var ( @@ -205,34 +202,6 @@ func parseLabel(rest string, d *Directive) (*Node, map[string]bool, error) { return node, nil, err } -// NodeFromLabels returns a Node for the injected labels -func NodeFromLabels(labels map[string]string) *Node { - keys := []string{} - for key := range labels { - keys = append(keys, key) - } - // Sort the label to have a repeatable order - sort.Strings(keys) - - labelPairs := []string{} - var rootNode *Node - var prevNode *Node - for _, key := range keys { - value := labels[key] - labelPairs = append(labelPairs, fmt.Sprintf("%q='%s'", key, value)) - // Value must be single quoted to prevent env variable expansion - // See https://github.com/docker/docker/issues/26027 - node := newKeyValueNode(key, "'"+value+"'") - rootNode, prevNode = appendKeyValueNode(node, rootNode, prevNode) - } - - return &Node{ - Value: command.Label, - Original: commandLabel + " " + strings.Join(labelPairs, " "), - Next: rootNode, - } -} - // parses a statement containing one or more keyword definition(s) and/or // value assignments, like `name1 name2= name3="" name4=value`. // Note that this is a stricter format than the old format of assignment, diff --git a/components/engine/builder/dockerfile/parser/line_parsers_test.go b/components/engine/builder/dockerfile/parser/line_parsers_test.go index 50b8d03c23..f7580efa3e 100644 --- a/components/engine/builder/dockerfile/parser/line_parsers_test.go +++ b/components/engine/builder/dockerfile/parser/line_parsers_test.go @@ -42,32 +42,6 @@ func TestParseNameValNewFormat(t *testing.T) { assert.DeepEqual(t, expected, node, cmpNodeOpt) } -func TestNodeFromLabels(t *testing.T) { - labels := map[string]string{ - "foo": "bar", - "weird": "first' second", - } - expected := &Node{ - Value: "label", - Original: `LABEL "foo"='bar' "weird"='first' second'`, - Next: &Node{ - Value: "foo", - Next: &Node{ - Value: "'bar'", - Next: &Node{ - Value: "weird", - Next: &Node{ - Value: "'first' second'", - }, - }, - }, - }, - } - - node := NodeFromLabels(labels) - assert.DeepEqual(t, expected, node, cmpNodeOpt) -} - func TestParseNameValWithoutVal(t *testing.T) { directive := Directive{} // In Config.Env, a variable without `=` is removed from the environment. (#31634) From 822d607b7958bd7dd856851ae93b873d27fd45db Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 17 May 2018 15:07:34 +0200 Subject: [PATCH 4/7] Make integration cleanup step less noisy The `deleteAllImages()` cleanup step in the integration suite was printing a message for each image that was removed. These messages were not very informative (given that `removeImage()` prints an error if image removal failed), and made the test output harder to read (more difficult to see error-conditions when scanning the output). This patch removes the messages to make the output slightly less noisy. Before this patch applied: --- PASS: TestBuildMultiStageOnBuild (7.15s) main_test.go:32: Removing image sha256:9db3ddbaaadd52804d8a417081f68db41fd4b8f80c85c1b4c4aee2d9b584c074 main_test.go:32: Removing image sha256:7eeb04d90b5e62a99ac6a5b2c10b9ba54b89b176fe2783e41461581c482852b3 main_test.go:32: Removing image sha256:adf42475eefff99b4a611c1a5d8353c4d0a011a7f7b9dc59a75d951cd54fa77f main_test.go:32: Removing image sha256:c547a770806e0445f5dfc255683ced771a23be6157ba8d0617bb9ab55dcee6d1 main_test.go:32: Removing image sha256:b79659c3e6d34faf2a075f1df1ea2c805833982f112666b25a466177b5d1352d main_test.go:32: Removing image sha256:29430078cdc927c19c87416cd1fb1ec386f167c5e201ee5dfb1644dcf268a3c2 === RUN TestBuildUncleanTarFilenames --- PASS: TestBuildUncleanTarFilenames (2.47s) main_test.go:32: Removing image sha256:0a6418fb221dc2f25085a1a7e507e01c4a3938cc5c65f1cb85a8c0fb09d6814f main_test.go:32: Removing image sha256:3d6e4bd0cce01ce5823b40dcb717cd16b3b4b769ff73dd86fa448aac49aa6d7c main_test.go:32: Removing image sha256:98e3f335e874612668335b3a5f125a1e5cbd0f6c79a7c3f719529b69d0abf2a5 main_test.go:32: Removing image sha256:08919f344b382fd1447da7f3e8ffd2a7125f5f7d191ed7d33242736dbe3c59cf main_test.go:32: Removing image sha256:d144b3c13838e841ec319a17e1046471d726bb2aa3211e167a6a53f766a2dcdc main_test.go:32: Removing image sha256:7c768ec742d628020f50c99dc5af32400b78534ca9fc4c01a9f00ec0ab19193a main_test.go:32: Removing image sha256:4c26c71d142045fbf3448aa1f6363d5a7a803cb438a78a4b20b7c847df03d50a main_test.go:32: Removing image sha256:08dcd63c964f2dbb17ff2665b6b86993fb14c0d3e169da187ac48f078a560d25 main_test.go:32: Removing image sha256:c1743fab233f36f2d7f83cb13f8c10ff06bdbda8f8a218d25a3796d1bc2f9e84 === RUN TestBuildMultiStageLayerLeak --- PASS: TestBuildMultiStageLayerLeak (5.59s) main_test.go:32: Removing image sha256:5e9974558276c34d7c9aab3fad408d433047b7b15bdae43ab5048adc58a15431 main_test.go:32: Removing image sha256:ac3c613c8c48794237c1e46ac0657ebbb1132910a240086bb2e9df9770fdc017 main_test.go:32: Removing image sha256:a936df268131ad427f7b4b66ce3dbb1e41866d7269a4d383cebcb1c5930d3346 main_test.go:32: Removing image sha256:5e613ea0ce7dbc908b0315c49585ae43ad6c34158e9e0b59a3dc93b00ef0ea41 main_test.go:32: Removing image sha256:e676f4ec41a42823b6d91e05e3290d3827f9175dea6fba5d8b769aa13aa7e082 main_test.go:32: Removing image sha256:93c8daab2703126b23d957d4d6b04f07949356f1cd95d4f8fdbededf4ab5c21e With this patch applied: === RUN TestBuildMultiStageOnBuild --- PASS: TestBuildMultiStageOnBuild (6.74s) === RUN TestBuildUncleanTarFilenames --- PASS: TestBuildUncleanTarFilenames (2.49s) === RUN TestBuildMultiStageLayerLeak --- PASS: TestBuildMultiStageLayerLeak (5.14s) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 5afce21e2efe572be3a256b748bf269afdd645a5 Component: engine --- components/engine/internal/test/environment/clean.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/components/engine/internal/test/environment/clean.go b/components/engine/internal/test/environment/clean.go index 8ef44e7075..e92006fc46 100644 --- a/components/engine/internal/test/environment/clean.go +++ b/components/engine/internal/test/environment/clean.go @@ -25,7 +25,7 @@ type logT interface { // Clean the environment, preserving protected objects (images, containers, ...) // and removing everything else. It's meant to run after any tests so that they don't // depend on each others. -func (e *Execution) Clean(t testingT) { +func (e *Execution) Clean(t assert.TestingT) { if ht, ok := t.(test.HelperT); ok { ht.Helper() } @@ -112,7 +112,7 @@ func getAllContainers(ctx context.Context, t assert.TestingT, client client.Cont return containers } -func deleteAllImages(t testingT, apiclient client.ImageAPIClient, protectedImages map[string]struct{}) { +func deleteAllImages(t assert.TestingT, apiclient client.ImageAPIClient, protectedImages map[string]struct{}) { if ht, ok := t.(test.HelperT); ok { ht.Helper() } @@ -123,15 +123,12 @@ func deleteAllImages(t testingT, apiclient client.ImageAPIClient, protectedImage for _, image := range images { tags := tagsFromImageSummary(image) if len(tags) == 0 { - t.Logf("Removing image %s", image.ID) removeImage(ctx, t, apiclient, image.ID) continue } for _, tag := range tags { if _, ok := protectedImages[tag]; !ok { - t.Logf("Removing image %s", tag) removeImage(ctx, t, apiclient, tag) - continue } } } From 30e8f43743e50ace7a89d4dc453f740df4f30a8b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 3 May 2018 14:08:25 -0700 Subject: [PATCH 5/7] aufs: use a single logger Simplify the code by using a single logger instance. While at it, use WithError in Umount. Signed-off-by: Kir Kolyshkin Upstream-commit: c6e2af54256211b8ac757e9b25caa6fb6c9b3c6e Component: engine --- components/engine/daemon/graphdriver/aufs/aufs.go | 15 ++++++--------- .../engine/daemon/graphdriver/aufs/mount.go | 3 +-- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/components/engine/daemon/graphdriver/aufs/aufs.go b/components/engine/daemon/graphdriver/aufs/aufs.go index 2fa5c6952a..5d1de2135d 100644 --- a/components/engine/daemon/graphdriver/aufs/aufs.go +++ b/components/engine/daemon/graphdriver/aufs/aufs.go @@ -62,6 +62,8 @@ var ( enableDirpermLock sync.Once enableDirperm bool + + logger = logrus.WithField("storage-driver", "aufs") ) func init() { @@ -109,7 +111,7 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap switch fsMagic { case graphdriver.FsMagicAufs, graphdriver.FsMagicBtrfs, graphdriver.FsMagicEcryptfs: - logrus.WithField("storage-driver", "aufs").Errorf("AUFS is not supported over %s", backingFs) + logger.Errorf("AUFS is not supported over %s", backingFs) return nil, graphdriver.ErrIncompatibleFS } @@ -143,7 +145,6 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } } - logger := logrus.WithField("storage-driver", "aufs") for _, path := range []string{"mnt", "diff"} { p := filepath.Join(root, path) @@ -306,10 +307,7 @@ func (a *Driver) Remove(id string) error { mountpoint = a.getMountpoint(id) } - logger := logrus.WithFields(logrus.Fields{ - "storage-driver": "aufs", - "layer": id, - }) + logger := logger.WithField("layer", id) var retries int for { @@ -439,7 +437,7 @@ func (a *Driver) Put(id string) error { err := a.unmount(m) if err != nil { - logrus.WithField("storage-driver", "aufs").Debugf("Failed to unmount %s aufs: %v", id, err) + logger.Debugf("Failed to unmount %s aufs: %v", id, err) } return err } @@ -597,7 +595,7 @@ func (a *Driver) Cleanup() error { for _, m := range dirs { if err := a.unmount(m); err != nil { - logrus.WithField("storage-driver", "aufs").Debugf("error unmounting %s: %s", m, err) + logger.Debugf("error unmounting %s: %s", m, err) } } return mountpk.RecursiveUnmount(a.root) @@ -652,7 +650,6 @@ func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err erro // useDirperm checks dirperm1 mount option can be used with the current // version of aufs. func useDirperm() bool { - logger := logrus.WithField("storage-driver", "aufs") enableDirpermLock.Do(func() { base, err := ioutil.TempDir("", "docker-aufs-base") if err != nil { diff --git a/components/engine/daemon/graphdriver/aufs/mount.go b/components/engine/daemon/graphdriver/aufs/mount.go index c0a89c1a01..9f5510380c 100644 --- a/components/engine/daemon/graphdriver/aufs/mount.go +++ b/components/engine/daemon/graphdriver/aufs/mount.go @@ -5,14 +5,13 @@ package aufs // import "github.com/docker/docker/daemon/graphdriver/aufs" import ( "os/exec" - "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) // Unmount the target specified. func Unmount(target string) error { if err := exec.Command("auplink", target, "flush").Run(); err != nil { - logrus.WithField("storage-driver", "aufs").Warnf("Couldn't run auplink before unmount %s: %s", target, err) + logger.WithError(err).Warnf("Couldn't run auplink before unmount %s", target) } return unix.Unmount(target, 0) } From 91ebfe260a43e7e23adb9031c7f153ae34cb4bb5 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 3 May 2018 14:09:32 -0700 Subject: [PATCH 6/7] aufs: log reason why aufs is not supported. In case aufs driver is not supported because supportsAufs() said so, it is not possible to get a real reason from the logs. To fix, log the error returned. Note we're not using WithError here as the error message itself is the sole message we want to print (i.e. there's nothing to add to it). Signed-off-by: Kir Kolyshkin Upstream-commit: 91f85d1c784f3dc9d892b2af2f51d6b6f3b0be69 Component: engine --- components/engine/daemon/graphdriver/aufs/aufs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/daemon/graphdriver/aufs/aufs.go b/components/engine/daemon/graphdriver/aufs/aufs.go index 5d1de2135d..9152252770 100644 --- a/components/engine/daemon/graphdriver/aufs/aufs.go +++ b/components/engine/daemon/graphdriver/aufs/aufs.go @@ -86,9 +86,9 @@ type Driver struct { // Init returns a new AUFS driver. // An error is returned if AUFS is not supported. func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (graphdriver.Driver, error) { - // Try to load the aufs kernel module if err := supportsAufs(); err != nil { + logger.Error(err) return nil, graphdriver.ErrNotSupported } From b179a3fc0d4828ad63f4ad80a4c75abe7081742c Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Thu, 17 May 2018 15:42:42 +0800 Subject: [PATCH 7/7] Some slight tweaks for the integration test `arm64` needs get more time duration for the test to finish. `pty.Start()` opens a file, so the caller should close it explicitly, else the file I/O can result in unexpected data synchronization issue. All those changes will not affect the test itself. Signed-off-by: Dennis Chen Upstream-commit: 476d7872efb60b1ef1bc7d9d83952f9dbc8f8798 Component: engine --- components/engine/hack/make.sh | 2 +- .../engine/integration-cli/docker_cli_exec_unix_test.go | 9 +++++---- components/engine/integration/internal/swarm/service.go | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/components/engine/hack/make.sh b/components/engine/hack/make.sh index be90bc019a..cd9232a4a5 100755 --- a/components/engine/hack/make.sh +++ b/components/engine/hack/make.sh @@ -159,7 +159,7 @@ ORIG_BUILDFLAGS+=( $REBUILD_FLAG ) BUILDFLAGS=( $BUILDFLAGS "${ORIG_BUILDFLAGS[@]}" ) # Test timeout. -if [ "${DOCKER_ENGINE_GOARCH}" == "arm" ]; then +if [ "${DOCKER_ENGINE_GOARCH}" == "arm64" ] || [ "${DOCKER_ENGINE_GOARCH}" == "arm" ]; then : ${TIMEOUT:=10m} elif [ "${DOCKER_ENGINE_GOARCH}" == "windows" ]; then : ${TIMEOUT:=8m} diff --git a/components/engine/integration-cli/docker_cli_exec_unix_test.go b/components/engine/integration-cli/docker_cli_exec_unix_test.go index 6608a7b704..337a90b116 100644 --- a/components/engine/integration-cli/docker_cli_exec_unix_test.go +++ b/components/engine/integration-cli/docker_cli_exec_unix_test.go @@ -25,7 +25,10 @@ func (s *DockerSuite) TestExecInteractiveStdinClose(c *check.C) { c.Assert(err, checker.IsNil) b := bytes.NewBuffer(nil) - go io.Copy(b, p) + go func() { + io.Copy(b, p) + p.Close() + }() ch := make(chan error) go func() { ch <- cmd.Wait() }() @@ -33,9 +36,7 @@ func (s *DockerSuite) TestExecInteractiveStdinClose(c *check.C) { select { case err := <-ch: c.Assert(err, checker.IsNil) - bs := b.Bytes() - bs = bytes.Trim(bs, "\x00") - output := string(bs[:]) + output := b.String() c.Assert(strings.TrimSpace(output), checker.Equals, "hello") case <-time.After(5 * time.Second): c.Fatal("timed out running docker exec") diff --git a/components/engine/integration/internal/swarm/service.go b/components/engine/integration/internal/swarm/service.go index 9035feeaeb..e6a1bfcdd0 100644 --- a/components/engine/integration/internal/swarm/service.go +++ b/components/engine/integration/internal/swarm/service.go @@ -22,7 +22,7 @@ func ServicePoll(config *poll.Settings) { config.Timeout = 30 * time.Second config.Delay = 100 * time.Millisecond if runtime.GOARCH == "arm64" || runtime.GOARCH == "arm" { - config.Timeout = 1 * time.Minute + config.Timeout = 90 * time.Second } }