From e25f9d09538840797c5fd6d8f45232c5c4143117 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 19 Dec 2017 17:25:16 -0800 Subject: [PATCH 1/4] integration-cli/TestRunModeIpcContainer: remove 1. The functionality of this test is superceded by `TestAPIIpcModeShareableAndContainer` (see integration-cli/docker_api_ipcmode_test.go). 2. This test won't work with --default-ipc-mode private. Signed-off-by: Kir Kolyshkin Upstream-commit: 519c06607ca7e8a544afddbd61ad57afe63a98b4 Component: engine --- .../integration-cli/docker_cli_run_test.go | 42 ------------------- 1 file changed, 42 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index e6c1d91fce..1749df07c1 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -2312,48 +2312,6 @@ func (s *DockerSuite) TestRunModeIpcHost(c *check.C) { } } -func (s *DockerSuite) TestRunModeIpcContainer(c *check.C) { - // Not applicable on Windows as uses Unix-specific capabilities - testRequires(c, SameHostDaemon, DaemonIsLinux) - - out, _ := dockerCmd(c, "run", "-d", "busybox", "sh", "-c", "echo -n test > /dev/shm/test && touch /dev/mqueue/toto && top") - - id := strings.TrimSpace(out) - state := inspectField(c, id, "State.Running") - if state != "true" { - c.Fatal("Container state is 'not running'") - } - pid1 := inspectField(c, id, "State.Pid") - - parentContainerIpc, err := os.Readlink(fmt.Sprintf("/proc/%s/ns/ipc", pid1)) - if err != nil { - c.Fatal(err) - } - - out, _ = dockerCmd(c, "run", fmt.Sprintf("--ipc=container:%s", id), "busybox", "readlink", "/proc/self/ns/ipc") - out = strings.Trim(out, "\n") - if parentContainerIpc != out { - c.Fatalf("IPC different with --ipc=container:%s %s != %s\n", id, parentContainerIpc, out) - } - - catOutput, _ := dockerCmd(c, "run", fmt.Sprintf("--ipc=container:%s", id), "busybox", "cat", "/dev/shm/test") - if catOutput != "test" { - c.Fatalf("Output of /dev/shm/test expected test but found: %s", catOutput) - } - - // check that /dev/mqueue is actually of mqueue type - grepOutput, _ := dockerCmd(c, "run", fmt.Sprintf("--ipc=container:%s", id), "busybox", "grep", "/dev/mqueue", "/proc/mounts") - if !strings.HasPrefix(grepOutput, "mqueue /dev/mqueue mqueue rw") { - c.Fatalf("Output of 'grep /proc/mounts' expected 'mqueue /dev/mqueue mqueue rw' but found: %s", grepOutput) - } - - lsOutput, _ := dockerCmd(c, "run", fmt.Sprintf("--ipc=container:%s", id), "busybox", "ls", "/dev/mqueue") - lsOutput = strings.Trim(lsOutput, "\n") - if lsOutput != "toto" { - c.Fatalf("Output of 'ls /dev/mqueue' expected 'toto' but found: %s", lsOutput) - } -} - func (s *DockerSuite) TestRunModeIpcContainerNotExists(c *check.C) { // Not applicable on Windows as uses Unix-specific capabilities testRequires(c, DaemonIsLinux) From dd0cbe7272fa58320ae0ba12ec5ad4bb1def8f2e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 19 Dec 2017 17:28:13 -0800 Subject: [PATCH 2/4] integration-cli/TestCleanupMounts: fix/improve `TestCleanupMountsAfterDaemonAndContainerKill` was supposedly written when the container mounts were visible from the host. Currently they all live in their own mount namespace and the only visible mount is the tmpfs one for shareable /dev/shm inside the container (i.e. /var/lib/docker/containers//shm), which will no longer be there in case of `--default-ipc-mode private` is used, and so the test will fail. Add a check if any container mounts are visible from the host, and skip the test if there are none, as there's nothing to check. `TestCleanupMountsAfterDaemonCrash`: fix in a similar way, keeping all the other checks it does, and skipping the "mounts gone" check if there were no mounts visible from the host. While at it, also fix the tests to use `d.Kill()` in order to not leave behind a stale `docker.pid` files. Signed-off-by: Kir Kolyshkin Upstream-commit: f5e01452d2c2a07bab48b4e05306ef9446770c4a Component: engine --- .../integration-cli/docker_cli_daemon_test.go | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_daemon_test.go b/components/engine/integration-cli/docker_cli_daemon_test.go index fb616261d4..5fbfac9a40 100644 --- a/components/engine/integration-cli/docker_cli_daemon_test.go +++ b/components/engine/integration-cli/docker_cli_daemon_test.go @@ -1441,13 +1441,19 @@ func (s *DockerDaemonSuite) TestCleanupMountsAfterDaemonAndContainerKill(c *chec out, err := d.Cmd("run", "-d", "busybox", "top") c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) id := strings.TrimSpace(out) - c.Assert(d.Signal(os.Kill), check.IsNil) + + // If there are no mounts with container id visible from the host + // (as those are in container's own mount ns), there is nothing + // to check here and the test should be skipped. mountOut, err := ioutil.ReadFile("/proc/self/mountinfo") c.Assert(err, check.IsNil, check.Commentf("Output: %s", mountOut)) + if !strings.Contains(string(mountOut), id) { + d.Stop(c) + c.Skip("no container mounts visible in host ns") + } - // container mounts should exist even after daemon has crashed. - comment := check.Commentf("%s should stay mounted from older daemon start:\nDaemon root repository %s\n%s", id, d.Root, mountOut) - c.Assert(strings.Contains(string(mountOut), id), check.Equals, true, comment) + // kill the daemon + c.Assert(d.Kill(), check.IsNil) // kill the container icmd.RunCommand(ctrBinary, "--address", "/var/run/docker/containerd/docker-containerd.sock", @@ -1459,7 +1465,7 @@ func (s *DockerDaemonSuite) TestCleanupMountsAfterDaemonAndContainerKill(c *chec // Now, container mounts should be gone. mountOut, err = ioutil.ReadFile("/proc/self/mountinfo") c.Assert(err, check.IsNil, check.Commentf("Output: %s", mountOut)) - comment = check.Commentf("%s is still mounted from older daemon start:\nDaemon root repository %s\n%s", id, d.Root, mountOut) + comment := check.Commentf("%s is still mounted from older daemon start:\nDaemon root repository %s\n%s", id, d.Root, mountOut) c.Assert(strings.Contains(string(mountOut), id), check.Equals, false, comment) d.Stop(c) @@ -2047,13 +2053,18 @@ func (s *DockerDaemonSuite) TestCleanupMountsAfterDaemonCrash(c *check.C) { c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) id := strings.TrimSpace(out) - c.Assert(s.d.Signal(os.Kill), check.IsNil) + // kill the daemon + c.Assert(s.d.Kill(), check.IsNil) + + // Check if there are mounts with container id visible from the host. + // If not, those mounts exist in container's own mount ns, and so + // the following check for mounts being cleared is pointless. + skipMountCheck := false mountOut, err := ioutil.ReadFile("/proc/self/mountinfo") c.Assert(err, check.IsNil, check.Commentf("Output: %s", mountOut)) - - // container mounts should exist even after daemon has crashed. - comment := check.Commentf("%s should stay mounted from older daemon start:\nDaemon root repository %s\n%s", id, s.d.Root, mountOut) - c.Assert(strings.Contains(string(mountOut), id), check.Equals, true, comment) + if !strings.Contains(string(mountOut), id) { + skipMountCheck = true + } // restart daemon. s.d.Start(c, "--live-restore") @@ -2070,10 +2081,13 @@ func (s *DockerDaemonSuite) TestCleanupMountsAfterDaemonCrash(c *check.C) { out, err = s.d.Cmd("stop", id) c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) + if skipMountCheck { + return + } // Now, container mounts should be gone. mountOut, err = ioutil.ReadFile("/proc/self/mountinfo") c.Assert(err, check.IsNil, check.Commentf("Output: %s", mountOut)) - comment = check.Commentf("%s is still mounted from older daemon start:\nDaemon root repository %s\n%s", id, s.d.Root, mountOut) + comment := check.Commentf("%s is still mounted from older daemon start:\nDaemon root repository %s\n%s", id, s.d.Root, mountOut) c.Assert(strings.Contains(string(mountOut), id), check.Equals, false, comment) } From 2a3410270828d2c8b442462c51a4537c18bead2c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 5 Dec 2017 11:25:37 -0800 Subject: [PATCH 3/4] TestBuildNotVerboseFailureRemote: fix false positive I got the following test failure on power: 10:00:56 10:00:56 ---------------------------------------------------------------------- 10:00:56 FAIL: docker_cli_build_test.go:3521: DockerSuite.TestBuildNotVerboseFailureRemote 10:00:56 10:00:56 docker_cli_build_test.go:3536: 10:00:56 c.Fatal(fmt.Errorf("Test[%s] expected that quiet stderr and verbose stdout are equal; quiet [%v], verbose [%v]", name, quietResult.Stderr(), result.Combined())) 10:00:56 ... Error: Test[quiet_build_wrong_remote] expected that quiet stderr and verbose stdout are equal; quiet [ 10:00:56 unable to prepare context: unable to download remote context http://something.invalid: Get http://something.invalid: dial tcp: lookup something.invalid on 172.29.128.11:53: no such host 10:00:56 ], verbose [unable to prepare context: unable to download remote context http://something.invalid: Get http://something.invalid: dial tcp: lookup something.invalid on 8.8.8.8:53: no such host 10:00:56 ] 10:00:56 10:00:56 10:00:56 ---------------------------------------------------------------------- The reason is, either more than one name server is configured, or nameserver was reconfigured in the middle of the test run. In any case, different nameserver IP in an error messages should not be treated as a failure, so let's strip those out. Signed-off-by: Kir Kolyshkin Upstream-commit: 3676bd8569f4df28a4f850cd4814e3558d8c03f6 Component: engine --- .../engine/integration-cli/docker_cli_build_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index e60f4d5a6e..8ae5e05ed8 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -3532,7 +3532,17 @@ func (s *DockerSuite) TestBuildNotVerboseFailureRemote(c *check.C) { result.Assert(c, icmd.Expected{ ExitCode: 1, }) - if strings.TrimSpace(quietResult.Stderr()) != strings.TrimSpace(result.Combined()) { + + // An error message should contain name server IP and port, like this: + // "dial tcp: lookup something.invalid on 172.29.128.11:53: no such host" + // The IP:port need to be removed in order to not trigger a test failur + // when more than one nameserver is configured. + // While at it, also strip excessive newlines. + normalize := func(msg string) string { + return strings.TrimSpace(regexp.MustCompile("[1-9][0-9.]+:[0-9]+").ReplaceAllLiteralString(msg, "")) + } + + if normalize(quietResult.Stderr()) != normalize(result.Combined()) { c.Fatal(fmt.Errorf("Test[%s] expected that quiet stderr and verbose stdout are equal; quiet [%v], verbose [%v]", name, quietResult.Stderr(), result.Combined())) } } From 8befd2d809534453fdb5018c2933f6f7153c269f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 6 Dec 2017 11:19:39 -0800 Subject: [PATCH 4/4] TestImportExtremelyLargeImageWorks: optimize DevZero According to https://github.com/golang/go/issues/5373, go recognizes (and optimizes for) the following syntax: ```go for i := range b { b[i] = 0 } ``` so let's use it. Limited testing shows ~7.5x speed increase, compared to the previously used syntax. Signed-off-by: Kir Kolyshkin Upstream-commit: f0cab0e28512de5eecc0412212425cc74d62af71 Component: engine --- components/engine/internal/testutil/helpers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/engine/internal/testutil/helpers.go b/components/engine/internal/testutil/helpers.go index 287b3cb48a..bb36322a80 100644 --- a/components/engine/internal/testutil/helpers.go +++ b/components/engine/internal/testutil/helpers.go @@ -20,8 +20,8 @@ var DevZero io.Reader = devZero{} type devZero struct{} func (d devZero) Read(p []byte) (n int, err error) { - for i := 0; i < len(p); i++ { - p[i] = '\x00' + for i := range p { + p[i] = 0 } return len(p), nil }