From 1331808a092274fcd56d44eb4f0f03e0f30a8ba0 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 12 Feb 2018 21:54:12 +0000 Subject: [PATCH 01/24] Migrate volumes tests in integration-cli to api tests This fix migrates volumes tests in integration-cli to api tests in integration/ Signed-off-by: Yong Tang (cherry picked from commit d896f87c0595134fa2f0787dad30b237815f233f) Signed-off-by: Sebastiaan van Stijn --- .../docker_api_volumes_test.go | 103 ---------------- .../integration/internal/container/ops.go | 19 +++ .../engine/integration/volume/main_test.go | 33 +++++ .../engine/integration/volume/volume_test.go | 115 ++++++++++++++++++ 4 files changed, 167 insertions(+), 103 deletions(-) delete mode 100644 components/engine/integration-cli/docker_api_volumes_test.go create mode 100644 components/engine/integration/volume/main_test.go create mode 100644 components/engine/integration/volume/volume_test.go diff --git a/components/engine/integration-cli/docker_api_volumes_test.go b/components/engine/integration-cli/docker_api_volumes_test.go deleted file mode 100644 index 65a96520921..00000000000 --- a/components/engine/integration-cli/docker_api_volumes_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package main - -import ( - "fmt" - "path/filepath" - "strings" - "time" - - "github.com/docker/docker/api/types/filters" - volumetypes "github.com/docker/docker/api/types/volume" - "github.com/docker/docker/client" - "github.com/docker/docker/integration-cli/checker" - "github.com/go-check/check" - "golang.org/x/net/context" -) - -func (s *DockerSuite) TestVolumesAPIList(c *check.C) { - prefix, _ := getPrefixAndSlashFromDaemonPlatform() - cid, _ := dockerCmd(c, "run", "-d", "-v", prefix+"/foo", "busybox") - - cli, err := client.NewEnvClient() - c.Assert(err, checker.IsNil) - defer cli.Close() - - container, err := cli.ContainerInspect(context.Background(), strings.TrimSpace(cid)) - c.Assert(err, checker.IsNil) - vname := container.Mounts[0].Name - - volumes, err := cli.VolumeList(context.Background(), filters.Args{}) - c.Assert(err, checker.IsNil) - - found := false - for _, vol := range volumes.Volumes { - if vol.Name == vname { - found = true - break - } - } - c.Assert(found, checker.Equals, true) -} - -func (s *DockerSuite) TestVolumesAPICreate(c *check.C) { - config := volumetypes.VolumesCreateBody{ - Name: "test", - } - - cli, err := client.NewEnvClient() - c.Assert(err, checker.IsNil) - defer cli.Close() - - vol, err := cli.VolumeCreate(context.Background(), config) - c.Assert(err, check.IsNil) - - c.Assert(filepath.Base(filepath.Dir(vol.Mountpoint)), checker.Equals, config.Name) -} - -func (s *DockerSuite) TestVolumesAPIRemove(c *check.C) { - prefix, _ := getPrefixAndSlashFromDaemonPlatform() - cid, _ := dockerCmd(c, "run", "-d", "-v", prefix+"/foo", "--name=test", "busybox") - - cli, err := client.NewEnvClient() - c.Assert(err, checker.IsNil) - defer cli.Close() - - container, err := cli.ContainerInspect(context.Background(), strings.TrimSpace(cid)) - c.Assert(err, checker.IsNil) - vname := container.Mounts[0].Name - - err = cli.VolumeRemove(context.Background(), vname, false) - c.Assert(err.Error(), checker.Contains, "volume is in use") - - dockerCmd(c, "rm", "-f", "test") - err = cli.VolumeRemove(context.Background(), vname, false) - c.Assert(err, checker.IsNil) -} - -func (s *DockerSuite) TestVolumesAPIInspect(c *check.C) { - config := volumetypes.VolumesCreateBody{ - Name: "test", - } - - // sampling current time minus a minute so to now have false positive in case of delays - now := time.Now().Truncate(time.Minute) - - cli, err := client.NewEnvClient() - c.Assert(err, checker.IsNil) - defer cli.Close() - - _, err = cli.VolumeCreate(context.Background(), config) - c.Assert(err, check.IsNil) - - vol, err := cli.VolumeInspect(context.Background(), config.Name) - c.Assert(err, checker.IsNil) - c.Assert(vol.Name, checker.Equals, config.Name) - - // comparing CreatedAt field time for the new volume to now. Removing a minute from both to avoid false positive - testCreatedAt, err := time.Parse(time.RFC3339, strings.TrimSpace(vol.CreatedAt)) - c.Assert(err, check.IsNil) - testCreatedAt = testCreatedAt.Truncate(time.Minute) - if !testCreatedAt.Equal(now) { - c.Assert(fmt.Errorf("Time Volume is CreatedAt not equal to current time"), check.NotNil) - } -} diff --git a/components/engine/integration/internal/container/ops.go b/components/engine/integration/internal/container/ops.go index e3d538bef16..9360527d370 100644 --- a/components/engine/integration/internal/container/ops.go +++ b/components/engine/integration/internal/container/ops.go @@ -1,6 +1,8 @@ package container import ( + "fmt" + containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/docker/go-connections/nat" @@ -57,3 +59,20 @@ func WithWorkingDir(dir string) func(*TestContainerConfig) { c.Config.WorkingDir = dir } } + +// WithVolume sets the volume of the container +func WithVolume(name string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + if c.Config.Volumes == nil { + c.Config.Volumes = map[string]struct{}{} + } + c.Config.Volumes[name] = struct{}{} + } +} + +// WithBind sets the bind mount of the container +func WithBind(src, target string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.HostConfig.Binds = append(c.HostConfig.Binds, fmt.Sprintf("%s:%s", src, target)) + } +} diff --git a/components/engine/integration/volume/main_test.go b/components/engine/integration/volume/main_test.go new file mode 100644 index 00000000000..206f7377ae0 --- /dev/null +++ b/components/engine/integration/volume/main_test.go @@ -0,0 +1,33 @@ +package volume // import "github.com/docker/docker/integration/volume" + +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) + } + err = environment.EnsureFrozenImagesLinux(testEnv) + 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) } +} diff --git a/components/engine/integration/volume/volume_test.go b/components/engine/integration/volume/volume_test.go new file mode 100644 index 00000000000..38ce5782e1d --- /dev/null +++ b/components/engine/integration/volume/volume_test.go @@ -0,0 +1,115 @@ +package volume + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + volumetypes "github.com/docker/docker/api/types/volume" + "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/integration/internal/request" + "github.com/docker/docker/internal/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestVolumesCreateAndList(t *testing.T) { + defer setupTest(t)() + client := request.NewAPIClient(t) + ctx := context.Background() + + name := t.Name() + vol, err := client.VolumeCreate(ctx, volumetypes.VolumesCreateBody{ + Name: name, + }) + require.NoError(t, err) + + expected := types.Volume{ + // Ignore timestamp of CreatedAt + CreatedAt: vol.CreatedAt, + Driver: "local", + Scope: "local", + Name: name, + Options: map[string]string{}, + Mountpoint: fmt.Sprintf("%s/volumes/%s/_data", testEnv.DaemonInfo.DockerRootDir, name), + } + assert.Equal(t, vol, expected) + + volumes, err := client.VolumeList(ctx, filters.Args{}) + require.NoError(t, err) + + assert.Equal(t, len(volumes.Volumes), 1) + assert.NotNil(t, volumes.Volumes[0]) + assert.Equal(t, *volumes.Volumes[0], expected) +} + +func TestVolumesRemove(t *testing.T) { + defer setupTest(t)() + client := request.NewAPIClient(t) + ctx := context.Background() + + prefix, _ := getPrefixAndSlashFromDaemonPlatform() + + id := container.Create(t, ctx, client, container.WithVolume(prefix+"foo")) + + c, err := client.ContainerInspect(ctx, id) + require.NoError(t, err) + vname := c.Mounts[0].Name + + err = client.VolumeRemove(ctx, vname, false) + testutil.ErrorContains(t, err, "volume is in use") + + err = client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{ + Force: true, + }) + require.NoError(t, err) + + err = client.VolumeRemove(ctx, vname, false) + require.NoError(t, err) +} + +func TestVolumesInspect(t *testing.T) { + defer setupTest(t)() + client := request.NewAPIClient(t) + ctx := context.Background() + + // sampling current time minus a minute so to now have false positive in case of delays + now := time.Now().Truncate(time.Minute) + + name := t.Name() + _, err := client.VolumeCreate(ctx, volumetypes.VolumesCreateBody{ + Name: name, + }) + require.NoError(t, err) + + vol, err := client.VolumeInspect(ctx, name) + require.NoError(t, err) + + expected := types.Volume{ + // Ignore timestamp of CreatedAt + CreatedAt: vol.CreatedAt, + Driver: "local", + Scope: "local", + Name: name, + Options: map[string]string{}, + Mountpoint: fmt.Sprintf("%s/volumes/%s/_data", testEnv.DaemonInfo.DockerRootDir, name), + } + assert.Equal(t, vol, expected) + + // comparing CreatedAt field time for the new volume to now. Removing a minute from both to avoid false positive + testCreatedAt, err := time.Parse(time.RFC3339, strings.TrimSpace(vol.CreatedAt)) + require.NoError(t, err) + testCreatedAt = testCreatedAt.Truncate(time.Minute) + assert.Equal(t, testCreatedAt.Equal(now), true, "Time Volume is CreatedAt not equal to current time") +} + +func getPrefixAndSlashFromDaemonPlatform() (prefix, slash string) { + if testEnv.OSType == "windows" { + return "c:", `\` + } + return "", "/" +} From 15237fd242fa513488de360f8b0c84a3a83df71e Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 12 Feb 2018 01:18:35 +0000 Subject: [PATCH 02/24] Migrate several docker rm tests to api tests This fix migrates several docker rm tests to api tests Signed-off-by: Yong Tang (cherry picked from commit 6bd4f4801b244555213f0040b9885033e99d4ae8) Signed-off-by: Sebastiaan van Stijn --- .../integration-cli/docker_cli_rm_test.go | 55 --------- .../integration/container/remove_test.go | 113 ++++++++++++++++++ 2 files changed, 113 insertions(+), 55 deletions(-) create mode 100644 components/engine/integration/container/remove_test.go diff --git a/components/engine/integration-cli/docker_cli_rm_test.go b/components/engine/integration-cli/docker_cli_rm_test.go index d281704a7b3..5942b8286d9 100644 --- a/components/engine/integration-cli/docker_cli_rm_test.go +++ b/components/engine/integration-cli/docker_cli_rm_test.go @@ -1,56 +1,11 @@ package main import ( - "io/ioutil" - "os" - "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/cli/build" "github.com/go-check/check" ) -func (s *DockerSuite) TestRmContainerWithRemovedVolume(c *check.C) { - testRequires(c, SameHostDaemon) - - prefix, slash := getPrefixAndSlashFromDaemonPlatform() - - tempDir, err := ioutil.TempDir("", "test-rm-container-with-removed-volume-") - if err != nil { - c.Fatalf("failed to create temporary directory: %s", tempDir) - } - defer os.RemoveAll(tempDir) - - dockerCmd(c, "run", "--name", "losemyvolumes", "-v", tempDir+":"+prefix+slash+"test", "busybox", "true") - - err = os.RemoveAll(tempDir) - c.Assert(err, check.IsNil) - - dockerCmd(c, "rm", "-v", "losemyvolumes") -} - -func (s *DockerSuite) TestRmContainerWithVolume(c *check.C) { - prefix, slash := getPrefixAndSlashFromDaemonPlatform() - - dockerCmd(c, "run", "--name", "foo", "-v", prefix+slash+"srv", "busybox", "true") - - dockerCmd(c, "rm", "-v", "foo") -} - -func (s *DockerSuite) TestRmContainerRunning(c *check.C) { - createRunningContainer(c, "foo") - - res, _, err := dockerCmdWithError("rm", "foo") - c.Assert(err, checker.NotNil, check.Commentf("Expected error, can't rm a running container")) - c.Assert(res, checker.Contains, "cannot remove a running container") -} - -func (s *DockerSuite) TestRmContainerForceRemoveRunning(c *check.C) { - createRunningContainer(c, "foo") - - // Stop then remove with -f - dockerCmd(c, "rm", "-f", "foo") -} - func (s *DockerSuite) TestRmContainerOrphaning(c *check.C) { dockerfile1 := `FROM busybox:latest ENTRYPOINT ["true"]` @@ -75,13 +30,3 @@ func (s *DockerSuite) TestRmContainerOrphaning(c *check.C) { c.Assert(out, checker.Contains, img1, check.Commentf("Orphaned container (could not find %q in docker images): %s", img1, out)) } - -func (s *DockerSuite) TestRmInvalidContainer(c *check.C) { - out, _, err := dockerCmdWithError("rm", "unknown") - c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm unknown container, got none")) - c.Assert(out, checker.Contains, "No such container") -} - -func createRunningContainer(c *check.C, name string) { - runSleepingContainer(c, "-dt", "--name", name) -} diff --git a/components/engine/integration/container/remove_test.go b/components/engine/integration/container/remove_test.go new file mode 100644 index 00000000000..bf55dd22c7e --- /dev/null +++ b/components/engine/integration/container/remove_test.go @@ -0,0 +1,113 @@ +package container // import "github.com/docker/docker/integration/container" + +import ( + "context" + "os" + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/integration/internal/request" + "github.com/docker/docker/internal/testutil" + "github.com/gotestyourself/gotestyourself/fs" + "github.com/gotestyourself/gotestyourself/poll" + "github.com/gotestyourself/gotestyourself/skip" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func getPrefixAndSlashFromDaemonPlatform() (prefix, slash string) { + if testEnv.OSType == "windows" { + return "c:", `\` + } + return "", "/" +} + +// Test case for #5244: `docker rm` fails if bind dir doesn't exist anymore +func TestRemoveContainerWithRemovedVolume(t *testing.T) { + skip.If(t, !testEnv.IsLocalDaemon()) + + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + prefix, slash := getPrefixAndSlashFromDaemonPlatform() + + tempDir := fs.NewDir(t, "test-rm-container-with-removed-volume", fs.WithMode(0755)) + defer tempDir.Remove() + + cID := container.Run(t, ctx, client, container.WithCmd("true"), container.WithBind(tempDir.Path(), prefix+slash+"test")) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) + + err := os.RemoveAll(tempDir.Path()) + require.NoError(t, err) + + err = client.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{ + RemoveVolumes: true, + }) + require.NoError(t, err) + + _, _, err = client.ContainerInspectWithRaw(ctx, cID, true) + testutil.ErrorContains(t, err, "No such container") +} + +// Test case for #2099/#2125 +func TestRemoveContainerWithVolume(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + prefix, slash := getPrefixAndSlashFromDaemonPlatform() + + cID := container.Run(t, ctx, client, container.WithCmd("true"), container.WithVolume(prefix+slash+"srv")) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) + + insp, _, err := client.ContainerInspectWithRaw(ctx, cID, true) + require.NoError(t, err) + assert.Equal(t, len(insp.Mounts), 1) + volName := insp.Mounts[0].Name + + err = client.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{ + RemoveVolumes: true, + }) + require.NoError(t, err) + + volumes, err := client.VolumeList(ctx, filters.NewArgs(filters.Arg("name", volName))) + require.NoError(t, err) + assert.Equal(t, len(volumes.Volumes), 0) +} + +func TestRemoveContainerRunning(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + cID := container.Run(t, ctx, client) + + err := client.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{}) + testutil.ErrorContains(t, err, "cannot remove a running container") +} + +func TestRemoveContainerForceRemoveRunning(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + cID := container.Run(t, ctx, client) + + err := client.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{ + Force: true, + }) + require.NoError(t, err) +} + +func TestRemoveInvalidContainer(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + err := client.ContainerRemove(ctx, "unknown", types.ContainerRemoveOptions{}) + testutil.ErrorContains(t, err, "No such container") +} From 7aa342f80894b42bd1d13a3d8b2d16383685dde9 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Fri, 23 Feb 2018 22:24:47 +0000 Subject: [PATCH 03/24] Update docker-py This fix update docker-py so that containers from the tests run could be cleaned up during teardown: ```diff -ENV DOCKER_PY_COMMIT 5e28dcaace5f7b70cbe44c313b7a3b288fa38916 +ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f ``` Signed-off-by: Yong Tang (cherry picked from commit 66935a0f64f0a72162fb3919c759f4f500b6c372) Signed-off-by: Sebastiaan van Stijn --- components/engine/Dockerfile | 2 +- components/engine/Dockerfile.aarch64 | 2 +- components/engine/Dockerfile.armhf | 2 +- components/engine/Dockerfile.ppc64le | 2 +- components/engine/Dockerfile.s390x | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/engine/Dockerfile b/components/engine/Dockerfile index 9b4033ef6fc..18a88e13417 100644 --- a/components/engine/Dockerfile +++ b/components/engine/Dockerfile @@ -142,7 +142,7 @@ RUN set -x \ && rm -rf "$GOPATH" # Get the "docker-py" source so we can run their integration tests -ENV DOCKER_PY_COMMIT 5e28dcaace5f7b70cbe44c313b7a3b288fa38916 +ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f # To run integration tests docker-pycreds is required. RUN git clone https://github.com/docker/docker-py.git /docker-py \ && cd /docker-py \ diff --git a/components/engine/Dockerfile.aarch64 b/components/engine/Dockerfile.aarch64 index 23a2f46d161..389b00f109e 100644 --- a/components/engine/Dockerfile.aarch64 +++ b/components/engine/Dockerfile.aarch64 @@ -106,7 +106,7 @@ RUN set -x \ && rm -rf "$GOPATH" # Get the "docker-py" source so we can run their integration tests -ENV DOCKER_PY_COMMIT 5e28dcaace5f7b70cbe44c313b7a3b288fa38916 +ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f # To run integration tests docker-pycreds is required. RUN git clone https://github.com/docker/docker-py.git /docker-py \ && cd /docker-py \ diff --git a/components/engine/Dockerfile.armhf b/components/engine/Dockerfile.armhf index 1bd2db6014d..b640396aee2 100644 --- a/components/engine/Dockerfile.armhf +++ b/components/engine/Dockerfile.armhf @@ -104,7 +104,7 @@ RUN set -x \ && rm -rf "$GOPATH" # Get the "docker-py" source so we can run their integration tests -ENV DOCKER_PY_COMMIT 5e28dcaace5f7b70cbe44c313b7a3b288fa38916 +ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f # To run integration tests docker-pycreds is required. RUN git clone https://github.com/docker/docker-py.git /docker-py \ && cd /docker-py \ diff --git a/components/engine/Dockerfile.ppc64le b/components/engine/Dockerfile.ppc64le index ea75224fea7..97a2fa995f8 100644 --- a/components/engine/Dockerfile.ppc64le +++ b/components/engine/Dockerfile.ppc64le @@ -102,7 +102,7 @@ RUN set -x \ && rm -rf "$GOPATH" # Get the "docker-py" source so we can run their integration tests -ENV DOCKER_PY_COMMIT 5e28dcaace5f7b70cbe44c313b7a3b288fa38916 +ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f # To run integration tests docker-pycreds is required. RUN git clone https://github.com/docker/docker-py.git /docker-py \ && cd /docker-py \ diff --git a/components/engine/Dockerfile.s390x b/components/engine/Dockerfile.s390x index 16aa1fdc6d5..2302750c9cc 100644 --- a/components/engine/Dockerfile.s390x +++ b/components/engine/Dockerfile.s390x @@ -96,7 +96,7 @@ RUN set -x \ && rm -rf "$GOPATH" # Get the "docker-py" source so we can run their integration tests -ENV DOCKER_PY_COMMIT 5e28dcaace5f7b70cbe44c313b7a3b288fa38916 +ENV DOCKER_PY_COMMIT 8b246db271a85d6541dc458838627e89c683e42f # To run integration tests docker-pycreds is required. RUN git clone https://github.com/docker/docker-py.git /docker-py \ && cd /docker-py \ From fd75fd2b4534d365ad4617a9c5a3dcd41af6f28b Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Fri, 23 Feb 2018 08:32:54 +0000 Subject: [PATCH 04/24] Clean the teardown process of network test We need to clean the resources created in some test cases, else in some cases we'll get below error for other tests: > FAIL: docker_experimental_network_test.go:37: DockerNetworkSuite.TestDockerNetworkMacvlanPersistance > docker_experimental_network_test.go:44: > ... > Command: ip link add dm-dummy0 type dummy > ExitCode: 2 > Error: exit status 2 > Stdout: > Stderr: RTNETLINK answers: File exists > ... Logically, each test case should be independent, the failure of previous test case should not have side-effect for the test cases followed. Signed-off-by: Dennis Chen (cherry picked from commit 57d85e7e54f7d074af8c496cba43ee18d3815207) Signed-off-by: Sebastiaan van Stijn --- .../docker_experimental_network_test.go | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/components/engine/integration-cli/docker_experimental_network_test.go b/components/engine/integration-cli/docker_experimental_network_test.go index 888970a0a33..fb20331631a 100644 --- a/components/engine/integration-cli/docker_experimental_network_test.go +++ b/components/engine/integration-cli/docker_experimental_network_test.go @@ -42,6 +42,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanPersistance(c *check.C) { master := "dm-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface that also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.60", "dm-persist") assertNwIsAvailable(c, "dm-persist") @@ -49,8 +51,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanPersistance(c *check.C) { s.d.Restart(c) // verify network is recreated from persistence assertNwIsAvailable(c, "dm-persist") - // cleanup the master interface that also collects the slave dev - deleteInterface(c, "dm-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkIpvlanPersistance(c *check.C) { @@ -60,6 +60,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanPersistance(c *check.C) { master := "di-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface that also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.70", "di-persist") assertNwIsAvailable(c, "di-persist") @@ -67,8 +69,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanPersistance(c *check.C) { s.d.Restart(c) // verify network is recreated from persistence assertNwIsAvailable(c, "di-persist") - // cleanup the master interface that also collects the slave dev - deleteInterface(c, "di-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkMacvlanSubIntCreate(c *check.C) { @@ -78,11 +78,11 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanSubIntCreate(c *check.C) { master := "dm-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.50", "dm-subinterface") assertNwIsAvailable(c, "dm-subinterface") - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "dm-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkIpvlanSubIntCreate(c *check.C) { @@ -92,11 +92,11 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanSubIntCreate(c *check.C) { master := "di-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.60", "di-subinterface") assertNwIsAvailable(c, "di-subinterface") - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "di-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkMacvlanOverlapParent(c *check.C) { @@ -105,6 +105,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanOverlapParent(c *check.C) { // master dummy interface 'dm' abbreviation represents 'docker macvlan' master := "dm-dummy0" createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) createVlanInterface(c, master, "dm-dummy0.40", "40") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.40", "dm-subinterface") @@ -113,8 +115,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanOverlapParent(c *check.C) { out, _, err := dockerCmdWithError("network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.40", "dm-parent-net-overlap") // verify that the overlap returns an error c.Assert(err, check.NotNil, check.Commentf(out)) - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "dm-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkIpvlanOverlapParent(c *check.C) { @@ -123,6 +123,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanOverlapParent(c *check.C) { // master dummy interface 'dm' abbreviation represents 'docker ipvlan' master := "di-dummy0" createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) createVlanInterface(c, master, "di-dummy0.30", "30") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.30", "di-subinterface") @@ -131,8 +133,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanOverlapParent(c *check.C) { out, _, err := dockerCmdWithError("network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.30", "di-parent-net-overlap") // verify that the overlap returns an error c.Assert(err, check.NotNil, check.Commentf(out)) - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "di-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkMacvlanMultiSubnet(c *check.C) { @@ -471,6 +471,7 @@ func (s *DockerSuite) TestDockerNetworkMacVlanExistingParent(c *check.C) { testRequires(c, DaemonIsLinux, macvlanKernelSupport, NotUserNamespace, NotArm, ExperimentalDaemon) netName := "dm-parent-exists" createMasterDummy(c, "dm-dummy0") + defer deleteInterface(c, "dm-dummy0") //out, err := createVlanInterface(c, "dm-parent", "dm-slave", "macvlan", "bridge") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0", netName) @@ -480,7 +481,6 @@ func (s *DockerSuite) TestDockerNetworkMacVlanExistingParent(c *check.C) { assertNwNotAvailable(c, netName) // verify the network delete did not delete the predefined link linkExists(c, "dm-dummy0") - deleteInterface(c, "dm-dummy0") } func (s *DockerSuite) TestDockerNetworkMacVlanSubinterface(c *check.C) { @@ -488,6 +488,8 @@ func (s *DockerSuite) TestDockerNetworkMacVlanSubinterface(c *check.C) { testRequires(c, DaemonIsLinux, macvlanKernelSupport, NotUserNamespace, NotArm, ExperimentalDaemon) netName := "dm-subinterface" createMasterDummy(c, "dm-dummy0") + // delete the parent interface which also collects the slave + defer deleteInterface(c, "dm-dummy0") createVlanInterface(c, "dm-dummy0", "dm-dummy0.20", "20") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.20", netName) @@ -510,8 +512,6 @@ func (s *DockerSuite) TestDockerNetworkMacVlanSubinterface(c *check.C) { assertNwNotAvailable(c, netName) // verify the network delete did not delete the predefined sub-interface linkExists(c, "dm-dummy0.20") - // delete the parent interface which also collects the slave - deleteInterface(c, "dm-dummy0") } func createMasterDummy(c *check.C, master string) { From 811b93ac9b287fd9f57bec2623197a9a7dcd5136 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Thu, 22 Feb 2018 02:30:51 -0800 Subject: [PATCH 05/24] Move containerIsStopped/containerIsInState to integration/internal/container This fix moves helper functions containerIsStopped and containerIsInState to integration/internal/container, so that they could be used outside of integration/container. Signed-off-by: Yong Tang (cherry picked from commit eda311c18f388ed4541dc44dcfba08cd4347a685) Signed-off-by: Sebastiaan van Stijn --- .../engine/integration/container/diff_test.go | 2 +- .../integration/container/inspect_test.go | 2 +- .../engine/integration/container/kill_test.go | 16 ++++---- .../integration/container/links_linux_test.go | 2 +- .../engine/integration/container/nat_test.go | 4 +- .../integration/container/pause_test.go | 8 ++-- .../integration/container/rename_test.go | 18 ++++---- .../integration/container/resize_test.go | 6 +-- .../integration/container/stats_test.go | 2 +- .../engine/integration/container/stop_test.go | 37 ++--------------- .../container/update_linux_test.go | 2 +- .../integration/container/update_test.go | 2 +- .../integration/internal/container/states.go | 41 +++++++++++++++++++ 13 files changed, 76 insertions(+), 66 deletions(-) create mode 100644 components/engine/integration/internal/container/states.go diff --git a/components/engine/integration/container/diff_test.go b/components/engine/integration/container/diff_test.go index 63ac19e9ad4..df602369983 100644 --- a/components/engine/integration/container/diff_test.go +++ b/components/engine/integration/container/diff_test.go @@ -29,7 +29,7 @@ func TestDiffFilenameShownInOutput(t *testing.T) { // a "Files/" prefix. lookingFor := containertypes.ContainerChangeResponseItem{Kind: archive.ChangeAdd, Path: "/foo/bar"} if testEnv.OSType == "windows" { - poll.WaitOn(t, containerIsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second)) lookingFor = containertypes.ContainerChangeResponseItem{Kind: archive.ChangeModify, Path: "Files/foo/bar"} } diff --git a/components/engine/integration/container/inspect_test.go b/components/engine/integration/container/inspect_test.go index 5ecd287bfe2..2eace554e4c 100644 --- a/components/engine/integration/container/inspect_test.go +++ b/components/engine/integration/container/inspect_test.go @@ -30,7 +30,7 @@ func TestInspectCpusetInConfigPre120(t *testing.T) { c.HostConfig.Resources.CpusetCpus = "0" }, ) - poll.WaitOn(t, containerIsInState(ctx, client, name, "exited"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, name, "exited"), poll.WithDelay(100*time.Millisecond)) _, body, err := client.ContainerInspectWithRaw(ctx, name, false) require.NoError(t, err) diff --git a/components/engine/integration/container/kill_test.go b/components/engine/integration/container/kill_test.go index d399213ac62..09e37ac0d19 100644 --- a/components/engine/integration/container/kill_test.go +++ b/components/engine/integration/container/kill_test.go @@ -23,11 +23,11 @@ func TestKillContainerInvalidSignal(t *testing.T) { err := client.ContainerKill(ctx, id, "0") require.EqualError(t, err, "Error response from daemon: Invalid signal: 0") - poll.WaitOn(t, containerIsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond)) err = client.ContainerKill(ctx, id, "SIG42") require.EqualError(t, err, "Error response from daemon: Invalid signal: SIG42") - poll.WaitOn(t, containerIsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond)) } func TestKillContainer(t *testing.T) { @@ -64,7 +64,7 @@ func TestKillContainer(t *testing.T) { err := client.ContainerKill(ctx, id, tc.signal) require.NoError(t, err) - poll.WaitOn(t, containerIsInState(ctx, client, id, tc.status), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, id, tc.status), poll.WithDelay(100*time.Millisecond)) }) } } @@ -104,7 +104,7 @@ func TestKillWithStopSignalAndRestartPolicies(t *testing.T) { err := client.ContainerKill(ctx, id, "TERM") require.NoError(t, err) - poll.WaitOn(t, containerIsInState(ctx, client, id, tc.status), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, id, tc.status), poll.WithDelay(100*time.Millisecond)) }) } } @@ -141,11 +141,11 @@ func TestKillDifferentUserContainer(t *testing.T) { id := container.Run(t, ctx, client, func(c *container.TestContainerConfig) { c.Config.User = "daemon" }) - poll.WaitOn(t, containerIsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerKill(ctx, id, "SIGKILL") require.NoError(t, err) - poll.WaitOn(t, containerIsInState(ctx, client, id, "exited"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, id, "exited"), poll.WithDelay(100*time.Millisecond)) } func TestInspectOomKilledTrue(t *testing.T) { @@ -160,7 +160,7 @@ func TestInspectOomKilledTrue(t *testing.T) { c.HostConfig.Resources.Memory = 32 * 1024 * 1024 }) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) @@ -177,7 +177,7 @@ func TestInspectOomKilledFalse(t *testing.T) { name := "testoomkilled" cID := container.Run(t, ctx, client, container.WithName(name), container.WithCmd("sh", "-c", "echo hello world")) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) diff --git a/components/engine/integration/container/links_linux_test.go b/components/engine/integration/container/links_linux_test.go index e2e0b9d9eaa..87b27e321b0 100644 --- a/components/engine/integration/container/links_linux_test.go +++ b/components/engine/integration/container/links_linux_test.go @@ -31,7 +31,7 @@ func TestLinksEtcHostsContentMatch(t *testing.T) { cID := container.Run(t, ctx, client, container.WithCmd("cat", "/etc/hosts"), container.WithNetworkMode("host")) - poll.WaitOn(t, containerIsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) body, err := client.ContainerLogs(ctx, cID, types.ContainerLogsOptions{ ShowStdout: true, diff --git a/components/engine/integration/container/nat_test.go b/components/engine/integration/container/nat_test.go index df3451fbc56..ad93e59f493 100644 --- a/components/engine/integration/container/nat_test.go +++ b/components/engine/integration/container/nat_test.go @@ -69,7 +69,7 @@ func TestNetworkLoopbackNat(t *testing.T) { cID := container.Run(t, ctx, client, container.WithCmd("sh", "-c", fmt.Sprintf("stty raw && nc -w 5 %s 8080", endpoint.String())), container.WithTty(true), container.WithNetworkMode("container:server")) - poll.WaitOn(t, containerIsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) body, err := client.ContainerLogs(ctx, cID, types.ContainerLogsOptions{ ShowStdout: true, @@ -98,7 +98,7 @@ func startServerContainer(t *testing.T, msg string, port int) string { } }) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) return cID } diff --git a/components/engine/integration/container/pause_test.go b/components/engine/integration/container/pause_test.go index 4e643572213..cacc3621de3 100644 --- a/components/engine/integration/container/pause_test.go +++ b/components/engine/integration/container/pause_test.go @@ -27,7 +27,7 @@ func TestPause(t *testing.T) { name := "testeventpause" cID := container.Run(t, ctx, client, container.WithName(name)) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) since := request.DaemonUnixTime(ctx, t, client, testEnv) @@ -59,7 +59,7 @@ func TestPauseFailsOnWindowsServerContainers(t *testing.T) { ctx := context.Background() cID := container.Run(t, ctx, client) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerPause(ctx, cID) testutil.ErrorContains(t, err, "cannot pause Windows Server Containers") @@ -73,7 +73,7 @@ func TestPauseStopPausedContainer(t *testing.T) { ctx := context.Background() cID := container.Run(t, ctx, client) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerPause(ctx, cID) require.NoError(t, err) @@ -81,7 +81,7 @@ func TestPauseStopPausedContainer(t *testing.T) { err = client.ContainerStop(ctx, cID, nil) require.NoError(t, err) - poll.WaitOn(t, containerIsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) } func getEventActions(t *testing.T, messages <-chan events.Message, errs <-chan error) []string { diff --git a/components/engine/integration/container/rename_test.go b/components/engine/integration/container/rename_test.go index da2bed17c7c..2138ee578eb 100644 --- a/components/engine/integration/container/rename_test.go +++ b/components/engine/integration/container/rename_test.go @@ -51,7 +51,7 @@ func TestRenameStoppedContainer(t *testing.T) { oldName := "first_name" cID := container.Run(t, ctx, client, container.WithName(oldName), container.WithCmd("sh")) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) @@ -73,7 +73,7 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { oldName := "first_name" cID := container.Run(t, ctx, client, container.WithName(oldName)) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) newName := "new_name" + stringid.GenerateNonCryptoID() err := client.ContainerRename(ctx, oldName, newName) @@ -87,7 +87,7 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { testutil.ErrorContains(t, err, "No such container: "+oldName) cID = container.Run(t, ctx, client, container.WithName(oldName)) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) inspect, err = client.ContainerInspect(ctx, cID) require.NoError(t, err) @@ -101,7 +101,7 @@ func TestRenameInvalidName(t *testing.T) { oldName := "first_name" cID := container.Run(t, ctx, client, container.WithName(oldName)) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerRename(ctx, oldName, "new:invalid") testutil.ErrorContains(t, err, "Invalid container name") @@ -136,7 +136,7 @@ func TestRenameAnonymousContainer(t *testing.T) { err = client.ContainerStart(ctx, "container1", types.ContainerStartOptions{}) require.NoError(t, err) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) count := "-c" if testEnv.OSType == "windows" { @@ -148,7 +148,7 @@ func TestRenameAnonymousContainer(t *testing.T) { } c.HostConfig.NetworkMode = "network1" }, container.WithCmd("ping", count, "1", "container1")) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) @@ -162,7 +162,7 @@ func TestRenameContainerWithSameName(t *testing.T) { client := request.NewAPIClient(t) cID := container.Run(t, ctx, client, container.WithName("old")) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerRename(ctx, "old", "old") testutil.ErrorContains(t, err, "Renaming a container with the same name") err = client.ContainerRename(ctx, cID, "old") @@ -182,10 +182,10 @@ func TestRenameContainerWithLinkedContainer(t *testing.T) { client := request.NewAPIClient(t) db1ID := container.Run(t, ctx, client, container.WithName("db1")) - poll.WaitOn(t, containerIsInState(ctx, client, db1ID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, db1ID, "running"), poll.WithDelay(100*time.Millisecond)) app1ID := container.Run(t, ctx, client, container.WithName("app1"), container.WithLinks("db1:/mysql")) - poll.WaitOn(t, containerIsInState(ctx, client, app1ID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, app1ID, "running"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerRename(ctx, "app1", "app2") require.NoError(t, err) diff --git a/components/engine/integration/container/resize_test.go b/components/engine/integration/container/resize_test.go index 0ada5c32e4c..0a5f26067e8 100644 --- a/components/engine/integration/container/resize_test.go +++ b/components/engine/integration/container/resize_test.go @@ -23,7 +23,7 @@ func TestResize(t *testing.T) { cID := container.Run(t, ctx, client) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerResize(ctx, cID, types.ResizeOptions{ Height: 40, @@ -39,7 +39,7 @@ func TestResizeWithInvalidSize(t *testing.T) { cID := container.Run(t, ctx, client) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) endpoint := "/containers/" + cID + "/resize?h=foo&w=bar" res, _, err := req.Post(endpoint) @@ -54,7 +54,7 @@ func TestResizeWhenContainerNotStarted(t *testing.T) { cID := container.Run(t, ctx, client, container.WithCmd("echo")) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) err := client.ContainerResize(ctx, cID, types.ResizeOptions{ Height: 40, diff --git a/components/engine/integration/container/stats_test.go b/components/engine/integration/container/stats_test.go index fdf85f44f63..9c0b9484983 100644 --- a/components/engine/integration/container/stats_test.go +++ b/components/engine/integration/container/stats_test.go @@ -28,7 +28,7 @@ func TestStats(t *testing.T) { cID := container.Run(t, ctx, client) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) resp, err := client.ContainerStats(ctx, cID, false) require.NoError(t, err) diff --git a/components/engine/integration/container/stop_test.go b/components/engine/integration/container/stop_test.go index 305596ac85e..4ecd06dd2c3 100644 --- a/components/engine/integration/container/stop_test.go +++ b/components/engine/integration/container/stop_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/docker/docker/api/types" - "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/request" "github.com/gotestyourself/gotestyourself/icmd" @@ -30,7 +29,7 @@ func TestStopContainerWithRestartPolicyAlways(t *testing.T) { } for _, name := range names { - poll.WaitOn(t, containerIsInState(ctx, client, name, "running", "restarting"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, name, "running", "restarting"), poll.WithDelay(100*time.Millisecond)) } for _, name := range names { @@ -39,7 +38,7 @@ func TestStopContainerWithRestartPolicyAlways(t *testing.T) { } for _, name := range names { - poll.WaitOn(t, containerIsStopped(ctx, client, name), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsStopped(ctx, client, name), poll.WithDelay(100*time.Millisecond)) } } @@ -52,7 +51,7 @@ func TestDeleteDevicemapper(t *testing.T) { id := container.Run(t, ctx, client, container.WithName("foo"), container.WithCmd("echo")) - poll.WaitOn(t, containerIsStopped(ctx, client, id), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsStopped(ctx, client, id), poll.WithDelay(100*time.Millisecond)) inspect, err := client.ContainerInspect(ctx, id) require.NoError(t, err) @@ -70,33 +69,3 @@ func TestDeleteDevicemapper(t *testing.T) { err = client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{}) require.NoError(t, err) } - -func containerIsStopped(ctx context.Context, client client.APIClient, containerID string) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - inspect, err := client.ContainerInspect(ctx, containerID) - - switch { - case err != nil: - return poll.Error(err) - case !inspect.State.Running: - return poll.Success() - default: - return poll.Continue("waiting for container to be stopped") - } - } -} - -func containerIsInState(ctx context.Context, client client.APIClient, containerID string, state ...string) func(log poll.LogT) poll.Result { - return func(log poll.LogT) poll.Result { - inspect, err := client.ContainerInspect(ctx, containerID) - if err != nil { - return poll.Error(err) - } - for _, v := range state { - if inspect.State.Status == v { - return poll.Success() - } - } - return poll.Continue("waiting for container to be running, currently %s", inspect.State.Status) - } -} diff --git a/components/engine/integration/container/update_linux_test.go b/components/engine/integration/container/update_linux_test.go index 9028f303868..c898dc1d3b8 100644 --- a/components/engine/integration/container/update_linux_test.go +++ b/components/engine/integration/container/update_linux_test.go @@ -31,7 +31,7 @@ func TestUpdateMemory(t *testing.T) { } }) - poll.WaitOn(t, containerIsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) const ( setMemory int64 = 314572800 diff --git a/components/engine/integration/container/update_test.go b/components/engine/integration/container/update_test.go index aee95804a1c..651e84cb222 100644 --- a/components/engine/integration/container/update_test.go +++ b/components/engine/integration/container/update_test.go @@ -39,7 +39,7 @@ func TestUpdateRestartPolicy(t *testing.T) { timeout = 180 * time.Second } - poll.WaitOn(t, containerIsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(timeout)) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(timeout)) inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) diff --git a/components/engine/integration/internal/container/states.go b/components/engine/integration/internal/container/states.go new file mode 100644 index 00000000000..27d9973e00a --- /dev/null +++ b/components/engine/integration/internal/container/states.go @@ -0,0 +1,41 @@ +package container + +import ( + "strings" + + "github.com/docker/docker/client" + "github.com/gotestyourself/gotestyourself/poll" + "golang.org/x/net/context" +) + +// IsStopped verifies the container is in stopped state. +func IsStopped(ctx context.Context, client client.APIClient, containerID string) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + inspect, err := client.ContainerInspect(ctx, containerID) + + switch { + case err != nil: + return poll.Error(err) + case !inspect.State.Running: + return poll.Success() + default: + return poll.Continue("waiting for container to be stopped") + } + } +} + +// IsInState verifies the container is in one of the specified state, e.g., "running", "exited", etc. +func IsInState(ctx context.Context, client client.APIClient, containerID string, state ...string) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + inspect, err := client.ContainerInspect(ctx, containerID) + if err != nil { + return poll.Error(err) + } + for _, v := range state { + if inspect.State.Status == v { + return poll.Success() + } + } + return poll.Continue("waiting for container to be one of (%s), currently %s", strings.Join(state, ", "), inspect.State.Status) + } +} From 89f42b5a6e6e5e6f4ee2f3f9ff4b5f1d7194e85b Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Thu, 22 Feb 2018 03:25:10 -0800 Subject: [PATCH 06/24] Migrate some config secret tests to api test This fix migrates some secret create tests to api tests, and remove redundant TestConfigCreate. Signed-off-by: Yong Tang (cherry picked from commit 99e28188507bbcb925b0c09df6b53cdd882d24c5) Signed-off-by: Sebastiaan van Stijn --- .../docker_cli_secret_create_test.go | 39 ------------------ .../engine/integration/secret/secret_test.go | 40 ++++++++----------- 2 files changed, 17 insertions(+), 62 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_secret_create_test.go b/components/engine/integration-cli/docker_cli_secret_create_test.go index 839c3922acb..a807e4e7e73 100644 --- a/components/engine/integration-cli/docker_cli_secret_create_test.go +++ b/components/engine/integration-cli/docker_cli_secret_create_test.go @@ -12,45 +12,6 @@ import ( "github.com/go-check/check" ) -func (s *DockerSwarmSuite) TestSecretCreate(c *check.C) { - d := s.AddDaemon(c, true, true) - - testName := "test_secret" - id := d.CreateSecret(c, swarm.SecretSpec{ - Annotations: swarm.Annotations{ - Name: testName, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("secrets: %s", id)) - - secret := d.GetSecret(c, id) - c.Assert(secret.Spec.Name, checker.Equals, testName) -} - -func (s *DockerSwarmSuite) TestSecretCreateWithLabels(c *check.C) { - d := s.AddDaemon(c, true, true) - - testName := "test_secret" - id := d.CreateSecret(c, swarm.SecretSpec{ - Annotations: swarm.Annotations{ - Name: testName, - Labels: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("secrets: %s", id)) - - secret := d.GetSecret(c, id) - c.Assert(secret.Spec.Name, checker.Equals, testName) - c.Assert(len(secret.Spec.Labels), checker.Equals, 2) - c.Assert(secret.Spec.Labels["key1"], checker.Equals, "value1") - c.Assert(secret.Spec.Labels["key2"], checker.Equals, "value2") -} - // Test case for 28884 func (s *DockerSwarmSuite) TestSecretCreateResolve(c *check.C) { d := s.AddDaemon(c, true, true) diff --git a/components/engine/integration/secret/secret_test.go b/components/engine/integration/secret/secret_test.go index 3b5e66a5bf1..8a292f005b2 100644 --- a/components/engine/integration/secret/secret_test.go +++ b/components/engine/integration/secret/secret_test.go @@ -129,7 +129,7 @@ func createSecret(ctx context.Context, t *testing.T, client client.APIClient, na return secret.ID } -func TestSecretsCreate(t *testing.T) { +func TestSecretsCreateAndDelete(t *testing.T) { skip.If(t, testEnv.DaemonInfo.OSType != "linux") defer setupTest(t)() @@ -141,8 +141,7 @@ func TestSecretsCreate(t *testing.T) { ctx := context.Background() testName := "test_secret" - createSecret(ctx, t, client, testName, []byte("TESTINGDATA"), nil) - require.NoError(t, err) + secretID := createSecret(ctx, t, client, testName, []byte("TESTINGDATA"), nil) // create an already existin secret, daemon should return a status code of 409 _, err = client.SecretCreate(ctx, swarmtypes.SecretSpec{ @@ -152,27 +151,8 @@ func TestSecretsCreate(t *testing.T) { Data: []byte("TESTINGDATA"), }) testutil.ErrorContains(t, err, "already exists") -} - -func TestSecretsDelete(t *testing.T) { - skip.If(t, testEnv.DaemonInfo.OSType != "linux") - - defer setupTest(t)() - d := swarm.NewSwarm(t, testEnv) - defer d.Stop(t) - client, err := client.NewClientWithOpts(client.WithHost((d.Sock()))) - require.NoError(t, err) - - ctx := context.Background() - - testName := "test_secret" - secretID := createSecret(ctx, t, client, testName, []byte("TESTINGDATA"), nil) - require.NoError(t, err) - - insp, _, err := client.SecretInspectWithRaw(ctx, secretID) - require.NoError(t, err) - assert.Equal(t, insp.ID, secretID) + // Ported from original TestSecretsDelete err = client.SecretRemove(ctx, secretID) require.NoError(t, err) @@ -181,6 +161,20 @@ func TestSecretsDelete(t *testing.T) { err = client.SecretRemove(ctx, "non-existin") testutil.ErrorContains(t, err, "No such secret: non-existin") + + // Ported from original TestSecretsCreteaWithLabels + testName = "test_secret_with_labels" + secretID = createSecret(ctx, t, client, testName, []byte("TESTINGDATA"), map[string]string{ + "key1": "value1", + "key2": "value2", + }) + + insp, _, err := client.SecretInspectWithRaw(ctx, secretID) + require.NoError(t, err) + assert.Equal(t, insp.Spec.Name, testName) + assert.Equal(t, len(insp.Spec.Labels), 2) + assert.Equal(t, insp.Spec.Labels["key1"], "value1") + assert.Equal(t, insp.Spec.Labels["key2"], "value2") } func TestSecretsUpdate(t *testing.T) { From b2ff741b5fb7093690b8b90d6c8f941f19d0305f Mon Sep 17 00:00:00 2001 From: junzhe and mnussbaum Date: Sat, 10 Feb 2018 00:03:47 +0000 Subject: [PATCH 07/24] Fix empty LogPath with non-blocking logging mode This fixes an issue where the container LogPath was empty when the non-blocking logging mode was enabled. This change sets the LogPath on the container as soon as the path is generated, instead of setting the LogPath on a logger struct and then attempting to pull it off that logger at a later point. That attempt to pull the LogPath off the logger was error prone since it assumed that the logger would only ever be a single type. Prior to this change docker inspect returned an empty string for LogPath. This caused issues with tools that rely on docker inspect output to discover container logs, e.g. Kubernetes. This commit also removes some LogPath methods that are now unnecessary and are never invoked. Signed-off-by: junzhe and mnussbaum (cherry picked from commit 20ca612a59c45c0bd58c71c199a7ebd2a6bf1a9e) Signed-off-by: Sebastiaan van Stijn --- components/engine/container/container.go | 9 ++-- .../engine/container/container_unit_test.go | 53 +++++++++++++++++++ .../daemon/logger/jsonfilelog/jsonfilelog.go | 5 -- .../daemon/logger/loggerutils/logfile.go | 7 --- 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index 46e592ab45f..461139b4355 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -38,7 +38,7 @@ import ( "github.com/docker/docker/runconfig" "github.com/docker/docker/volume" "github.com/docker/go-connections/nat" - "github.com/docker/go-units" + units "github.com/docker/go-units" "github.com/docker/libnetwork" "github.com/docker/libnetwork/netlabel" "github.com/docker/libnetwork/options" @@ -391,6 +391,8 @@ func (container *Container) StartLogger() (logger.Logger, error) { if err != nil { return nil, err } + + container.LogPath = info.LogPath } l, err := initDriver(info) @@ -974,11 +976,6 @@ func (container *Container) startLogging() error { copier.Run() container.LogDriver = l - // set LogPath field only for json-file logdriver - if jl, ok := l.(*jsonfilelog.JSONFileLogger); ok { - container.LogPath = jl.LogPath() - } - return nil } diff --git a/components/engine/container/container_unit_test.go b/components/engine/container/container_unit_test.go index f6ded8fa9dc..858124abdbc 100644 --- a/components/engine/container/container_unit_test.go +++ b/components/engine/container/container_unit_test.go @@ -1,12 +1,16 @@ package container // import "github.com/docker/docker/container" import ( + "fmt" + "io/ioutil" "path/filepath" "testing" "github.com/docker/docker/api/types/container" swarmtypes "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/daemon/logger/jsonfilelog" "github.com/docker/docker/pkg/signal" + "github.com/stretchr/testify/require" ) func TestContainerStopSignal(t *testing.T) { @@ -66,3 +70,52 @@ func TestContainerSecretReferenceDestTarget(t *testing.T) { t.Fatalf("expected secret dest %q; received %q", expected, d) } } + +func TestContainerLogPathSetForJSONFileLogger(t *testing.T) { + containerRoot, err := ioutil.TempDir("", "TestContainerLogPathSetForJSONFileLogger") + require.NoError(t, err) + + c := &Container{ + Config: &container.Config{}, + HostConfig: &container.HostConfig{ + LogConfig: container.LogConfig{ + Type: jsonfilelog.Name, + }, + }, + ID: "TestContainerLogPathSetForJSONFileLogger", + Root: containerRoot, + } + + _, err = c.StartLogger() + require.NoError(t, err) + + expectedLogPath, err := filepath.Abs(filepath.Join(containerRoot, fmt.Sprintf("%s-json.log", c.ID))) + require.NoError(t, err) + require.Equal(t, c.LogPath, expectedLogPath) +} + +func TestContainerLogPathSetForRingLogger(t *testing.T) { + containerRoot, err := ioutil.TempDir("", "TestContainerLogPathSetForRingLogger") + require.NoError(t, err) + + c := &Container{ + Config: &container.Config{}, + HostConfig: &container.HostConfig{ + LogConfig: container.LogConfig{ + Type: jsonfilelog.Name, + Config: map[string]string{ + "mode": string(container.LogModeNonBlock), + }, + }, + }, + ID: "TestContainerLogPathSetForRingLogger", + Root: containerRoot, + } + + _, err = c.StartLogger() + require.NoError(t, err) + + expectedLogPath, err := filepath.Abs(filepath.Join(containerRoot, fmt.Sprintf("%s-json.log", c.ID))) + require.NoError(t, err) + require.Equal(t, c.LogPath, expectedLogPath) +} diff --git a/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go b/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go index 92484697d52..7d637f85736 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go @@ -150,11 +150,6 @@ func ValidateLogOpt(cfg map[string]string) error { return nil } -// LogPath returns the location the given json logger logs to. -func (l *JSONFileLogger) LogPath() string { - return l.writer.LogPath() -} - // Close closes underlying file and signals all readers to stop. func (l *JSONFileLogger) Close() error { l.mu.Lock() diff --git a/components/engine/daemon/logger/loggerutils/logfile.go b/components/engine/daemon/logger/loggerutils/logfile.go index a9d9d633db7..b533726c420 100644 --- a/components/engine/daemon/logger/loggerutils/logfile.go +++ b/components/engine/daemon/logger/loggerutils/logfile.go @@ -130,13 +130,6 @@ func rotate(name string, maxFiles int) error { return nil } -// LogPath returns the location the given writer logs to. -func (w *LogFile) LogPath() string { - w.mu.Lock() - defer w.mu.Unlock() - return w.f.Name() -} - // MaxFiles return maximum number of files func (w *LogFile) MaxFiles() int { return w.maxFiles From bae8ccdbd7954c9ed88cf32a5167f1b8c03045f9 Mon Sep 17 00:00:00 2001 From: mnussbaum Date: Tue, 27 Feb 2018 19:07:06 +0000 Subject: [PATCH 08/24] Clean-up after container unit test Remove temp directories and close file loggers in container unit tests. Signed-off-by: mnussbaum (cherry picked from commit 07d5446fe27cb92d881df48be6e8a6510d9608b0) Signed-off-by: Sebastiaan van Stijn --- components/engine/container/container_unit_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/components/engine/container/container_unit_test.go b/components/engine/container/container_unit_test.go index 858124abdbc..863a47a1f22 100644 --- a/components/engine/container/container_unit_test.go +++ b/components/engine/container/container_unit_test.go @@ -3,6 +3,7 @@ package container // import "github.com/docker/docker/container" import ( "fmt" "io/ioutil" + "os" "path/filepath" "testing" @@ -74,6 +75,7 @@ func TestContainerSecretReferenceDestTarget(t *testing.T) { func TestContainerLogPathSetForJSONFileLogger(t *testing.T) { containerRoot, err := ioutil.TempDir("", "TestContainerLogPathSetForJSONFileLogger") require.NoError(t, err) + defer os.RemoveAll(containerRoot) c := &Container{ Config: &container.Config{}, @@ -86,8 +88,9 @@ func TestContainerLogPathSetForJSONFileLogger(t *testing.T) { Root: containerRoot, } - _, err = c.StartLogger() + logger, err := c.StartLogger() require.NoError(t, err) + defer logger.Close() expectedLogPath, err := filepath.Abs(filepath.Join(containerRoot, fmt.Sprintf("%s-json.log", c.ID))) require.NoError(t, err) @@ -97,6 +100,7 @@ func TestContainerLogPathSetForJSONFileLogger(t *testing.T) { func TestContainerLogPathSetForRingLogger(t *testing.T) { containerRoot, err := ioutil.TempDir("", "TestContainerLogPathSetForRingLogger") require.NoError(t, err) + defer os.RemoveAll(containerRoot) c := &Container{ Config: &container.Config{}, @@ -112,8 +116,9 @@ func TestContainerLogPathSetForRingLogger(t *testing.T) { Root: containerRoot, } - _, err = c.StartLogger() + logger, err := c.StartLogger() require.NoError(t, err) + defer logger.Close() expectedLogPath, err := filepath.Abs(filepath.Join(containerRoot, fmt.Sprintf("%s-json.log", c.ID))) require.NoError(t, err) From e4022bf9340841fd79e27558cf5896a8d95f4483 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 26 Feb 2018 15:23:18 -0800 Subject: [PATCH 09/24] Migrate config inspect test to api test This fix migrates config inspect test in integration-cli to api test. Signed-off-by: Yong Tang (cherry picked from commit 4b99d782079dc390c2d8fb78f6973bbeee7d8a47) Signed-off-by: Sebastiaan van Stijn --- .../docker_cli_config_inspect_test.go | 68 ------------------- .../engine/integration/config/config_test.go | 25 +++++++ 2 files changed, 25 insertions(+), 68 deletions(-) delete mode 100644 components/engine/integration-cli/docker_cli_config_inspect_test.go diff --git a/components/engine/integration-cli/docker_cli_config_inspect_test.go b/components/engine/integration-cli/docker_cli_config_inspect_test.go deleted file mode 100644 index ba4e80f0705..00000000000 --- a/components/engine/integration-cli/docker_cli_config_inspect_test.go +++ /dev/null @@ -1,68 +0,0 @@ -// +build !windows - -package main - -import ( - "encoding/json" - - "github.com/docker/docker/api/types/swarm" - "github.com/docker/docker/integration-cli/checker" - "github.com/go-check/check" -) - -func (s *DockerSwarmSuite) TestConfigInspect(c *check.C) { - d := s.AddDaemon(c, true, true) - - testName := "test_config" - id := d.CreateConfig(c, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: testName, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("configs: %s", id)) - - config := d.GetConfig(c, id) - c.Assert(config.Spec.Name, checker.Equals, testName) - - out, err := d.Cmd("config", "inspect", testName) - c.Assert(err, checker.IsNil, check.Commentf(out)) - - var configs []swarm.Config - c.Assert(json.Unmarshal([]byte(out), &configs), checker.IsNil) - c.Assert(configs, checker.HasLen, 1) -} - -func (s *DockerSwarmSuite) TestConfigInspectMultiple(c *check.C) { - d := s.AddDaemon(c, true, true) - - testNames := []string{ - "test0", - "test1", - } - for _, n := range testNames { - id := d.CreateConfig(c, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: n, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("configs: %s", id)) - - config := d.GetConfig(c, id) - c.Assert(config.Spec.Name, checker.Equals, n) - - } - - args := []string{ - "config", - "inspect", - } - args = append(args, testNames...) - out, err := d.Cmd(args...) - c.Assert(err, checker.IsNil, check.Commentf(out)) - - var configs []swarm.Config - c.Assert(json.Unmarshal([]byte(out), &configs), checker.IsNil) - c.Assert(configs, checker.HasLen, 2) -} diff --git a/components/engine/integration/config/config_test.go b/components/engine/integration/config/config_test.go index c152be59bf2..4e31b205ee2 100644 --- a/components/engine/integration/config/config_test.go +++ b/components/engine/integration/config/config_test.go @@ -2,6 +2,7 @@ package config import ( "bytes" + "encoding/json" "sort" "testing" "time" @@ -327,3 +328,27 @@ func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) time.Sleep(100 * time.Millisecond) } } + +func TestConfigInspect(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + + defer setupTest(t)() + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + client, err := client.NewClientWithOpts(client.WithHost((d.Sock()))) + require.NoError(t, err) + + ctx := context.Background() + + testName := t.Name() + configID := createConfig(ctx, t, client, testName, []byte("TESTINGDATA"), nil) + + insp, body, err := client.ConfigInspectWithRaw(ctx, configID) + require.NoError(t, err) + assert.Equal(t, insp.Spec.Name, testName) + + var config swarmtypes.Config + err = json.Unmarshal(body, &config) + require.NoError(t, err) + assert.Equal(t, config, insp) +} From 8b54bcc057b51490cc722da64e0c50ea8337628d Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 21 Feb 2018 22:00:05 -0800 Subject: [PATCH 10/24] Migrate events tests to api tests This fix migrates events tests in integration-cli to api tests. Signed-off-by: Yong Tang (cherry picked from commit 3a749157d2c2b320fea49f7aa4d4eb634f52662f) Signed-off-by: Sebastiaan van Stijn --- .../integration-cli/docker_api_events_test.go | 74 ------------------- .../integration/internal/request/client.go | 6 +- .../engine/integration/system/event_test.go | 61 ++++++++++++++- 3 files changed, 63 insertions(+), 78 deletions(-) delete mode 100644 components/engine/integration-cli/docker_api_events_test.go diff --git a/components/engine/integration-cli/docker_api_events_test.go b/components/engine/integration-cli/docker_api_events_test.go deleted file mode 100644 index a95422f5817..00000000000 --- a/components/engine/integration-cli/docker_api_events_test.go +++ /dev/null @@ -1,74 +0,0 @@ -package main - -import ( - "encoding/json" - "io" - "net/http" - "net/url" - "strconv" - "strings" - "time" - - "github.com/docker/docker/integration-cli/checker" - "github.com/docker/docker/integration-cli/request" - "github.com/docker/docker/pkg/jsonmessage" - "github.com/go-check/check" -) - -func (s *DockerSuite) TestEventsAPIEmptyOutput(c *check.C) { - type apiResp struct { - resp *http.Response - err error - } - chResp := make(chan *apiResp) - go func() { - resp, body, err := request.Get("/events") - body.Close() - chResp <- &apiResp{resp, err} - }() - - select { - case r := <-chResp: - c.Assert(r.err, checker.IsNil) - c.Assert(r.resp.StatusCode, checker.Equals, http.StatusOK) - case <-time.After(3 * time.Second): - c.Fatal("timeout waiting for events api to respond, should have responded immediately") - } -} - -func (s *DockerSuite) TestEventsAPIBackwardsCompatible(c *check.C) { - since := daemonTime(c).Unix() - ts := strconv.FormatInt(since, 10) - - out := runSleepingContainer(c, "--name=foo", "-d") - containerID := strings.TrimSpace(out) - c.Assert(waitRun(containerID), checker.IsNil) - - q := url.Values{} - q.Set("since", ts) - - _, body, err := request.Get("/events?" + q.Encode()) - c.Assert(err, checker.IsNil) - defer body.Close() - - dec := json.NewDecoder(body) - var containerCreateEvent *jsonmessage.JSONMessage - for { - var event jsonmessage.JSONMessage - if err := dec.Decode(&event); err != nil { - if err == io.EOF { - break - } - c.Fatal(err) - } - if event.Status == "create" && event.ID == containerID { - containerCreateEvent = &event - break - } - } - - c.Assert(containerCreateEvent, checker.Not(checker.IsNil)) - c.Assert(containerCreateEvent.Status, checker.Equals, "create") - c.Assert(containerCreateEvent.ID, checker.Equals, containerID) - c.Assert(containerCreateEvent.From, checker.Equals, "busybox") -} diff --git a/components/engine/integration/internal/request/client.go b/components/engine/integration/internal/request/client.go index 367db14c591..34e589ec866 100644 --- a/components/engine/integration/internal/request/client.go +++ b/components/engine/integration/internal/request/client.go @@ -20,8 +20,8 @@ func NewAPIClient(t *testing.T, ops ...func(*client.Client) error) client.APICli return clt } -// daemonTime provides the current time on the daemon host -func daemonTime(ctx context.Context, t *testing.T, client client.APIClient, testEnv *environment.Execution) time.Time { +// DaemonTime provides the current time on the daemon host +func DaemonTime(ctx context.Context, t *testing.T, client client.APIClient, testEnv *environment.Execution) time.Time { if testEnv.IsLocalDaemon() { return time.Now() } @@ -37,6 +37,6 @@ func daemonTime(ctx context.Context, t *testing.T, client client.APIClient, test // DaemonUnixTime returns the current time on the daemon host with nanoseconds precision. // It return the time formatted how the client sends timestamps to the server. func DaemonUnixTime(ctx context.Context, t *testing.T, client client.APIClient, testEnv *environment.Execution) string { - dt := daemonTime(ctx, t, client, testEnv) + dt := DaemonTime(ctx, t, client, testEnv) return fmt.Sprintf("%d.%09d", dt.Unix(), int64(dt.Nanosecond())) } diff --git a/components/engine/integration/system/event_test.go b/components/engine/integration/system/event_test.go index 7f041956904..688d7c27def 100644 --- a/components/engine/integration/system/event_test.go +++ b/components/engine/integration/system/event_test.go @@ -2,15 +2,22 @@ package system // import "github.com/docker/docker/integration/system" import ( "context" + "encoding/json" + "io" + "net/http" + "net/url" + "strconv" "testing" - "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/strslice" + req "github.com/docker/docker/integration-cli/request" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/request" + "github.com/docker/docker/pkg/jsonmessage" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -58,3 +65,55 @@ func TestEvents(t *testing.T) { } } + +// Test case for #18888: Events messages have been switched from generic +// `JSONMessage` to `events.Message` types. The switch does not break the +// backward compatibility so old `JSONMessage` could still be used. +// This test verifies that backward compatibility maintains. +func TestEventsBackwardsCompatible(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + since := request.DaemonTime(ctx, t, client, testEnv) + ts := strconv.FormatInt(since.Unix(), 10) + + cID := container.Create(t, ctx, client) + + // In case there is no events, the API should have responded immediately (not blocking), + // The test here makes sure the response time is less than 3 sec. + expectedTime := time.Now().Add(3 * time.Second) + emptyResp, emptyBody, err := req.Get("/events") + require.NoError(t, err) + defer emptyBody.Close() + assert.Equal(t, http.StatusOK, emptyResp.StatusCode) + assert.True(t, time.Now().Before(expectedTime), "timeout waiting for events api to respond, should have responded immediately") + + // We also test to make sure the `events.Message` is compatible with `JSONMessage` + q := url.Values{} + q.Set("since", ts) + _, body, err := req.Get("/events?" + q.Encode()) + require.NoError(t, err) + defer body.Close() + + dec := json.NewDecoder(body) + var containerCreateEvent *jsonmessage.JSONMessage + for { + var event jsonmessage.JSONMessage + if err := dec.Decode(&event); err != nil { + if err == io.EOF { + break + } + t.Fatal(err) + } + if event.Status == "create" && event.ID == cID { + containerCreateEvent = &event + break + } + } + + assert.NotNil(t, containerCreateEvent) + assert.Equal(t, "create", containerCreateEvent.Status) + assert.Equal(t, cID, containerCreateEvent.ID) + assert.Equal(t, "busybox", containerCreateEvent.From) +} From 0fd259afcb4faacf2a64afc24ba30b716206c095 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 28 Feb 2018 17:15:25 +0000 Subject: [PATCH 11/24] Remove docker_cli_secret_inspect_test.go as the test (TestSecretInspectMultiple) seems to have been covered pretty well in cli: https://github.com/docker/cli/blob/master/cli/command/secret/inspect_test.go Signed-off-by: Yong Tang (cherry picked from commit 3d38adb20c619b87edab72e51ff0fd1cf6e08691) Signed-off-by: Sebastiaan van Stijn --- .../docker_cli_secret_inspect_test.go | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 components/engine/integration-cli/docker_cli_secret_inspect_test.go diff --git a/components/engine/integration-cli/docker_cli_secret_inspect_test.go b/components/engine/integration-cli/docker_cli_secret_inspect_test.go deleted file mode 100644 index 429e9ad1089..00000000000 --- a/components/engine/integration-cli/docker_cli_secret_inspect_test.go +++ /dev/null @@ -1,45 +0,0 @@ -// +build !windows - -package main - -import ( - "encoding/json" - - "github.com/docker/docker/api/types/swarm" - "github.com/docker/docker/integration-cli/checker" - "github.com/go-check/check" -) - -func (s *DockerSwarmSuite) TestSecretInspectMultiple(c *check.C) { - d := s.AddDaemon(c, true, true) - - testNames := []string{ - "test0", - "test1", - } - for _, n := range testNames { - id := d.CreateSecret(c, swarm.SecretSpec{ - Annotations: swarm.Annotations{ - Name: n, - }, - Data: []byte("TESTINGDATA"), - }) - c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("secrets: %s", id)) - - secret := d.GetSecret(c, id) - c.Assert(secret.Spec.Name, checker.Equals, n) - - } - - args := []string{ - "secret", - "inspect", - } - args = append(args, testNames...) - out, err := d.Cmd(args...) - c.Assert(err, checker.IsNil, check.Commentf(out)) - - var secrets []swarm.Secret - c.Assert(json.Unmarshal([]byte(out), &secrets), checker.IsNil) - c.Assert(secrets, checker.HasLen, 2) -} From 53a10ae2bc6db55b73fa98149e83d50c27636e51 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 28 Feb 2018 21:35:56 +0000 Subject: [PATCH 12/24] Enhancement of replacing ContainerCreate with helper funcs in tests This fix is a minor enhancement to replace several ContainerCreate with helper funcs of `container.Create` in tests. Signed-off-by: Yong Tang (cherry picked from commit 6ad4720c78d6ac61a60a3e7ed1d0c0119c5d103e) Signed-off-by: Sebastiaan van Stijn --- .../container/daemon_linux_test.go | 24 +++++++------------ .../engine/integration/container/ps_test.go | 23 ++++-------------- 2 files changed, 12 insertions(+), 35 deletions(-) diff --git a/components/engine/integration/container/daemon_linux_test.go b/components/engine/integration/container/daemon_linux_test.go index 872c7ab4cce..46881bca563 100644 --- a/components/engine/integration/container/daemon_linux_test.go +++ b/components/engine/integration/container/daemon_linux_test.go @@ -9,8 +9,8 @@ import ( "testing" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/container" "github.com/docker/docker/integration-cli/daemon" + "github.com/docker/docker/integration/internal/container" "github.com/stretchr/testify/assert" "golang.org/x/sys/unix" ) @@ -36,22 +36,14 @@ func TestContainerStartOnDaemonRestart(t *testing.T) { assert.NoError(t, err, "error creating client") ctx := context.Background() - c, err := client.ContainerCreate(ctx, - &container.Config{ - Image: "busybox", - Cmd: []string{"top"}, - }, - nil, - nil, - "", - ) - assert.NoError(t, err, "error creating test container") - defer client.ContainerRemove(ctx, c.ID, types.ContainerRemoveOptions{Force: true}) - - err = client.ContainerStart(ctx, c.ID, types.ContainerStartOptions{}) + + cID := container.Create(t, ctx, client) + defer client.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true}) + + err = client.ContainerStart(ctx, cID, types.ContainerStartOptions{}) assert.NoError(t, err, "error starting test container") - inspect, err := client.ContainerInspect(ctx, c.ID) + inspect, err := client.ContainerInspect(ctx, cID) assert.NoError(t, err, "error getting inspect data") ppid := getContainerdShimPid(t, inspect) @@ -67,7 +59,7 @@ func TestContainerStartOnDaemonRestart(t *testing.T) { d.Start(t, "--iptables=false") - err = client.ContainerStart(ctx, c.ID, types.ContainerStartOptions{}) + err = client.ContainerStart(ctx, cID, types.ContainerStartOptions{}) assert.NoError(t, err, "failed to start test container") } diff --git a/components/engine/integration/container/ps_test.go b/components/engine/integration/container/ps_test.go index dfcb0e2efe6..358276b36af 100644 --- a/components/engine/integration/container/ps_test.go +++ b/components/engine/integration/container/ps_test.go @@ -5,9 +5,8 @@ import ( "testing" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" - "github.com/docker/docker/api/types/network" + "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/request" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,23 +17,9 @@ func TestPsFilter(t *testing.T) { client := request.NewAPIClient(t) ctx := context.Background() - createContainerForFilter := func(ctx context.Context, name string) string { - body, err := client.ContainerCreate(ctx, - &container.Config{ - Cmd: []string{"top"}, - Image: "busybox", - }, - &container.HostConfig{}, - &network.NetworkingConfig{}, - name, - ) - require.NoError(t, err) - return body.ID - } - - prev := createContainerForFilter(ctx, "prev") - createContainerForFilter(ctx, "top") - next := createContainerForFilter(ctx, "next") + prev := container.Create(t, ctx, client, container.WithName("prev")) + container.Create(t, ctx, client, container.WithName("top")) + next := container.Create(t, ctx, client, container.WithName("next")) containerIDs := func(containers []types.Container) []string { entries := []string{} From d4444e3446cf78b77e8a546b82c50f8eafb74fd5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 1 Mar 2018 14:16:19 +0100 Subject: [PATCH 13/24] Fix "expected" and "actual" being reversed Signed-off-by: Sebastiaan van Stijn (cherry picked from commit a2517cbf62d75c48861337182aa841c5089f8ac4) Signed-off-by: Sebastiaan van Stijn --- components/engine/integration/container/nat_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/engine/integration/container/nat_test.go b/components/engine/integration/container/nat_test.go index ad93e59f493..6997ed05151 100644 --- a/components/engine/integration/container/nat_test.go +++ b/components/engine/integration/container/nat_test.go @@ -36,7 +36,7 @@ func TestNetworkNat(t *testing.T) { data, err := ioutil.ReadAll(conn) require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(string(data)), msg) + assert.Equal(t, msg, strings.TrimSpace(string(data))) } func TestNetworkLocalhostTCPNat(t *testing.T) { @@ -53,7 +53,7 @@ func TestNetworkLocalhostTCPNat(t *testing.T) { data, err := ioutil.ReadAll(conn) require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(string(data)), msg) + assert.Equal(t, msg, strings.TrimSpace(string(data))) } func TestNetworkLoopbackNat(t *testing.T) { @@ -81,7 +81,7 @@ func TestNetworkLoopbackNat(t *testing.T) { _, err = io.Copy(&b, body) require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(b.String()), msg) + assert.Equal(t, msg, strings.TrimSpace(b.String())) } func startServerContainer(t *testing.T, msg string, port int) string { @@ -109,7 +109,7 @@ func getExternalAddress(t *testing.T) net.IP { ifaceAddrs, err := iface.Addrs() require.NoError(t, err) - assert.NotEqual(t, len(ifaceAddrs), 0) + assert.NotEqual(t, 0, len(ifaceAddrs)) ifaceIP, _, err := net.ParseCIDR(ifaceAddrs[0].String()) require.NoError(t, err) From e695ffea63c2d765c82dcab05b95199eab16a834 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Thu, 1 Mar 2018 22:38:34 +0000 Subject: [PATCH 14/24] Migrate export tests to api tests This fix migrates export tests in integration-cli to api tests. Signed-off-by: Yong Tang (cherry picked from commit 4e702cf70d50ee5b0737270f27d9973fd3084c66) Signed-off-by: Sebastiaan van Stijn --- .../docker_cli_export_import_test.go | 21 +------- .../integration/container/export_test.go | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+), 19 deletions(-) create mode 100644 components/engine/integration/container/export_test.go diff --git a/components/engine/integration-cli/docker_cli_export_import_test.go b/components/engine/integration-cli/docker_cli_export_import_test.go index 45f29d54ed7..6405c1bb5e5 100644 --- a/components/engine/integration-cli/docker_cli_export_import_test.go +++ b/components/engine/integration-cli/docker_cli_export_import_test.go @@ -9,25 +9,8 @@ import ( "github.com/gotestyourself/gotestyourself/icmd" ) -// export an image and try to import it into a new one -func (s *DockerSuite) TestExportContainerAndImportImage(c *check.C) { - testRequires(c, DaemonIsLinux) - containerID := "testexportcontainerandimportimage" - - dockerCmd(c, "run", "--name", containerID, "busybox", "true") - - out, _ := dockerCmd(c, "export", containerID) - - result := icmd.RunCmd(icmd.Cmd{ - Command: []string{dockerBinary, "import", "-", "repo/testexp:v1"}, - Stdin: strings.NewReader(out), - }) - result.Assert(c, icmd.Success) - - cleanedImageID := strings.TrimSpace(result.Combined()) - c.Assert(cleanedImageID, checker.Not(checker.Equals), "", check.Commentf("output should have been an image id")) -} - +// TODO: Move this test to docker/cli, as it is essentially the same test +// as TestExportContainerAndImportImage except output to a file. // Used to test output flag in the export command func (s *DockerSuite) TestExportContainerWithOutputAndImportImage(c *check.C) { testRequires(c, DaemonIsLinux) diff --git a/components/engine/integration/container/export_test.go b/components/engine/integration/container/export_test.go new file mode 100644 index 00000000000..657b1fce412 --- /dev/null +++ b/components/engine/integration/container/export_test.go @@ -0,0 +1,53 @@ +package container // import "github.com/docker/docker/integration/container" + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/integration/internal/request" + "github.com/docker/docker/pkg/jsonmessage" + "github.com/gotestyourself/gotestyourself/poll" + "github.com/gotestyourself/gotestyourself/skip" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// export an image and try to import it into a new one +func TestExportContainerAndImportImage(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + + defer setupTest(t)() + client := request.NewAPIClient(t) + ctx := context.Background() + + cID := container.Run(t, ctx, client, container.WithCmd("true")) + poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) + + reference := "repo/testexp:v1" + exportResp, err := client.ContainerExport(ctx, cID) + require.NoError(t, err) + importResp, err := client.ImageImport(ctx, types.ImageImportSource{ + Source: exportResp, + SourceName: "-", + }, reference, types.ImageImportOptions{}) + require.NoError(t, err) + + // If the import is successfully, then the message output should contain + // the image ID and match with the output from `docker images`. + + dec := json.NewDecoder(importResp) + var jm jsonmessage.JSONMessage + err = dec.Decode(&jm) + require.NoError(t, err) + + images, err := client.ImageList(ctx, types.ImageListOptions{ + Filters: filters.NewArgs(filters.Arg("reference", reference)), + }) + require.NoError(t, err) + assert.Equal(t, jm.Status, images[0].ID) +} From 1f529b2bd2787d4a28c58bbd452968679ad078d6 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Fri, 2 Mar 2018 14:45:14 +0100 Subject: [PATCH 15/24] Fixes some integration/container test to run on remote daemon ``` docker build -f Dockerfile.e2e -t moby-e2e . docker run -v /var/run/docker.sock:/var/run/docker.sock \ -e TEST_INTEGRATION_DIR=/tests/integration/container \ -e DOCKER_API_VERSION=1.36 moby-e2e ``` Signed-off-by: Vincent Demeester (cherry picked from commit 18dd1d9aba3c79d355abaa7f498b88ad816f7d04) Signed-off-by: Sebastiaan van Stijn --- .../engine/integration-cli/check_test.go | 2 +- .../container/daemon_linux_test.go | 2 ++ .../integration/container/links_linux_test.go | 2 +- .../container/mounts_linux_test.go | 3 ++- .../engine/integration/container/nat_test.go | 6 +++--- .../integration/container/pause_test.go | 9 ++++----- .../integration/container/remove_test.go | 2 +- .../integration/container/rename_test.go | 20 +++++++++++-------- .../integration/container/restart_test.go | 2 ++ .../integration/service/inspect_test.go | 2 +- .../internal/test/environment/environment.go | 8 +++++++- 11 files changed, 36 insertions(+), 22 deletions(-) diff --git a/components/engine/integration-cli/check_test.go b/components/engine/integration-cli/check_test.go index 8df93dc2f65..18d3d3780a1 100644 --- a/components/engine/integration-cli/check_test.go +++ b/components/engine/integration-cli/check_test.go @@ -84,7 +84,7 @@ type DockerSuite struct { } func (s *DockerSuite) OnTimeout(c *check.C) { - if !testEnv.IsLocalDaemon() { + if testEnv.IsRemoteDaemon() { return } path := filepath.Join(os.Getenv("DEST"), "docker.pid") diff --git a/components/engine/integration/container/daemon_linux_test.go b/components/engine/integration/container/daemon_linux_test.go index 46881bca563..1424e74009f 100644 --- a/components/engine/integration/container/daemon_linux_test.go +++ b/components/engine/integration/container/daemon_linux_test.go @@ -11,6 +11,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/integration/internal/container" + "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" "golang.org/x/sys/unix" ) @@ -26,6 +27,7 @@ import ( // the container process, then start dockerd back up and attempt to start the // container again. func TestContainerStartOnDaemonRestart(t *testing.T) { + skip.If(t, testEnv.IsRemoteDaemon(), "cannot start daemon on remote test run") t.Parallel() d := daemon.New(t, "", "dockerd", daemon.Config{}) diff --git a/components/engine/integration/container/links_linux_test.go b/components/engine/integration/container/links_linux_test.go index 87b27e321b0..d230898ed0e 100644 --- a/components/engine/integration/container/links_linux_test.go +++ b/components/engine/integration/container/links_linux_test.go @@ -20,7 +20,7 @@ import ( ) func TestLinksEtcHostsContentMatch(t *testing.T) { - skip.If(t, !testEnv.IsLocalDaemon()) + skip.If(t, testEnv.IsRemoteDaemon()) hosts, err := ioutil.ReadFile("/etc/hosts") skip.If(t, os.IsNotExist(err)) diff --git a/components/engine/integration/container/mounts_linux_test.go b/components/engine/integration/container/mounts_linux_test.go index ceec706271a..71bdccc71be 100644 --- a/components/engine/integration/container/mounts_linux_test.go +++ b/components/engine/integration/container/mounts_linux_test.go @@ -23,6 +23,7 @@ import ( ) func TestContainerShmNoLeak(t *testing.T) { + skip.If(t, testEnv.IsRemoteDaemon(), "cannot start daemon on remote test run") t.Parallel() d := daemon.New(t, "docker", "dockerd", daemon.Config{}) client, err := d.NewClient() @@ -94,7 +95,7 @@ func TestContainerShmNoLeak(t *testing.T) { func TestContainerNetworkMountsNoChown(t *testing.T) { // chown only applies to Linux bind mounted volumes; must be same host to verify - skip.If(t, testEnv.DaemonInfo.OSType != "linux" || !testEnv.IsLocalDaemon()) + skip.If(t, testEnv.DaemonInfo.OSType != "linux" || testEnv.IsRemoteDaemon()) defer setupTest(t)() diff --git a/components/engine/integration/container/nat_test.go b/components/engine/integration/container/nat_test.go index 6997ed05151..e2bb0f9b072 100644 --- a/components/engine/integration/container/nat_test.go +++ b/components/engine/integration/container/nat_test.go @@ -22,7 +22,7 @@ import ( ) func TestNetworkNat(t *testing.T) { - skip.If(t, !testEnv.IsLocalDaemon()) + skip.If(t, testEnv.IsRemoteDaemon()) defer setupTest(t)() @@ -40,7 +40,7 @@ func TestNetworkNat(t *testing.T) { } func TestNetworkLocalhostTCPNat(t *testing.T) { - skip.If(t, !testEnv.IsLocalDaemon()) + skip.If(t, testEnv.IsRemoteDaemon()) defer setupTest(t)() @@ -57,7 +57,7 @@ func TestNetworkLocalhostTCPNat(t *testing.T) { } func TestNetworkLoopbackNat(t *testing.T) { - skip.If(t, !testEnv.IsLocalDaemon()) + skip.If(t, testEnv.IsRemoteDaemon()) msg := "it works" startServerContainer(t, msg, 8080) diff --git a/components/engine/integration/container/pause_test.go b/components/engine/integration/container/pause_test.go index cacc3621de3..7c2aa96c754 100644 --- a/components/engine/integration/container/pause_test.go +++ b/components/engine/integration/container/pause_test.go @@ -25,20 +25,19 @@ func TestPause(t *testing.T) { client := request.NewAPIClient(t) ctx := context.Background() - name := "testeventpause" - cID := container.Run(t, ctx, client, container.WithName(name)) + cID := container.Run(t, ctx, client) poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) since := request.DaemonUnixTime(ctx, t, client, testEnv) - err := client.ContainerPause(ctx, name) + err := client.ContainerPause(ctx, cID) require.NoError(t, err) inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) assert.Equal(t, inspect.State.Paused, true) - err = client.ContainerUnpause(ctx, name) + err = client.ContainerUnpause(ctx, cID) require.NoError(t, err) until := request.DaemonUnixTime(ctx, t, client, testEnv) @@ -46,7 +45,7 @@ func TestPause(t *testing.T) { messages, errs := client.Events(ctx, types.EventsOptions{ Since: since, Until: until, - Filters: filters.NewArgs(filters.Arg("container", name)), + Filters: filters.NewArgs(filters.Arg("container", cID)), }) assert.Equal(t, getEventActions(t, messages, errs), []string{"pause", "unpause"}) } diff --git a/components/engine/integration/container/remove_test.go b/components/engine/integration/container/remove_test.go index bf55dd22c7e..22fa26af67c 100644 --- a/components/engine/integration/container/remove_test.go +++ b/components/engine/integration/container/remove_test.go @@ -27,7 +27,7 @@ func getPrefixAndSlashFromDaemonPlatform() (prefix, slash string) { // Test case for #5244: `docker rm` fails if bind dir doesn't exist anymore func TestRemoveContainerWithRemovedVolume(t *testing.T) { - skip.If(t, !testEnv.IsLocalDaemon()) + skip.If(t, testEnv.IsRemoteDaemon()) defer setupTest(t)() ctx := context.Background() diff --git a/components/engine/integration/container/rename_test.go b/components/engine/integration/container/rename_test.go index 2138ee578eb..3567aee1f54 100644 --- a/components/engine/integration/container/rename_test.go +++ b/components/engine/integration/container/rename_test.go @@ -55,7 +55,7 @@ func TestRenameStoppedContainer(t *testing.T) { inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.Name, "/"+oldName) + assert.Equal(t, "/"+oldName, inspect.Name) newName := "new_name" + stringid.GenerateNonCryptoID() err = client.ContainerRename(ctx, oldName, newName) @@ -63,7 +63,7 @@ func TestRenameStoppedContainer(t *testing.T) { inspect, err = client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.Name, "/"+newName) + assert.Equal(t, "/"+newName, inspect.Name) } func TestRenameRunningContainerAndReuse(t *testing.T) { @@ -81,7 +81,7 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.Name, "/"+newName) + assert.Equal(t, "/"+newName, inspect.Name) _, err = client.ContainerInspect(ctx, oldName) testutil.ErrorContains(t, err, "No such container: "+oldName) @@ -91,7 +91,7 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { inspect, err = client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.Name, "/"+oldName) + assert.Equal(t, "/"+oldName, inspect.Name) } func TestRenameInvalidName(t *testing.T) { @@ -108,7 +108,7 @@ func TestRenameInvalidName(t *testing.T) { inspect, err := client.ContainerInspect(ctx, oldName) require.NoError(t, err) - assert.Equal(t, inspect.ID, cID) + assert.Equal(t, cID, inspect.ID) } // Test case for GitHub issue 22466 @@ -133,6 +133,10 @@ func TestRenameAnonymousContainer(t *testing.T) { }) err = client.ContainerRename(ctx, cID, "container1") require.NoError(t, err) + // Stop/Start the container to get registered + // FIXME(vdemeester) this is a really weird behavior as it fails otherwise + err = client.ContainerStop(ctx, "container1", nil) + require.NoError(t, err) err = client.ContainerStart(ctx, "container1", types.ContainerStartOptions{}) require.NoError(t, err) @@ -152,7 +156,7 @@ func TestRenameAnonymousContainer(t *testing.T) { inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.State.ExitCode, 0) + assert.Equal(t, 0, inspect.State.ExitCode, "container %s exited with the wrong exitcode: %+v", cID, inspect) } // TODO: should be a unit test @@ -175,7 +179,7 @@ func TestRenameContainerWithSameName(t *testing.T) { // of the linked container should be updated so that the other // container could still reference to the container that is renamed. func TestRenameContainerWithLinkedContainer(t *testing.T) { - skip.If(t, !testEnv.IsLocalDaemon()) + skip.If(t, testEnv.IsRemoteDaemon()) defer setupTest(t)() ctx := context.Background() @@ -192,5 +196,5 @@ func TestRenameContainerWithLinkedContainer(t *testing.T) { inspect, err := client.ContainerInspect(ctx, "app2/mysql") require.NoError(t, err) - assert.Equal(t, inspect.ID, db1ID) + assert.Equal(t, db1ID, inspect.ID) } diff --git a/components/engine/integration/container/restart_test.go b/components/engine/integration/container/restart_test.go index accaf2cd80b..7a2576e219f 100644 --- a/components/engine/integration/container/restart_test.go +++ b/components/engine/integration/container/restart_test.go @@ -9,9 +9,11 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/integration-cli/daemon" + "github.com/gotestyourself/gotestyourself/skip" ) func TestDaemonRestartKillContainers(t *testing.T) { + skip.If(t, testEnv.IsRemoteDaemon(), "cannot start daemon on remote test run") type testCase struct { desc string config *container.Config diff --git a/components/engine/integration/service/inspect_test.go b/components/engine/integration/service/inspect_test.go index fdb22cf2f75..8cd24bc31b6 100644 --- a/components/engine/integration/service/inspect_test.go +++ b/components/engine/integration/service/inspect_test.go @@ -18,7 +18,7 @@ import ( ) func TestInspect(t *testing.T) { - skip.IfCondition(t, !testEnv.IsLocalDaemon()) + skip.IfCondition(t, testEnv.IsRemoteDaemon()) defer setupTest(t)() d := swarm.NewSwarm(t, testEnv) defer d.Stop(t) diff --git a/components/engine/internal/test/environment/environment.go b/components/engine/internal/test/environment/environment.go index bde4c0a1bc6..16f61463346 100644 --- a/components/engine/internal/test/environment/environment.go +++ b/components/engine/internal/test/environment/environment.go @@ -96,7 +96,7 @@ func toSlash(path string) string { } // IsLocalDaemon is true if the daemon under test is on the same -// host as the CLI. +// host as the test process. // // Deterministically working out the environment in which CI is running // to evaluate whether the daemon is local or remote is not possible through @@ -115,6 +115,12 @@ func (e *Execution) IsLocalDaemon() bool { return os.Getenv("DOCKER_REMOTE_DAEMON") == "" } +// IsRemoteDaemon is true if the daemon under test is on different host +// as the test process. +func (e *Execution) IsRemoteDaemon() bool { + return !e.IsLocalDaemon() +} + // Print the execution details to stdout // TODO: print everything func (e *Execution) Print() { From 618f290b37858e591f377d0b6051f18c3b459a31 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Fri, 2 Mar 2018 19:02:50 +0000 Subject: [PATCH 16/24] Improvement in integration tests This fix adds several improvement: 1. No need for explicit ContainerRemove as it has been handled in setupTest() 2. Added `container.WithImage` helper function and used it in commit tests. Signed-off-by: Yong Tang (cherry picked from commit 6ab465804b0b8cec6c5ac278a21151d49e34885d) Signed-off-by: Sebastiaan van Stijn --- components/engine/integration/container/logs_test.go | 1 - components/engine/integration/image/commit_test.go | 12 +++++------- .../engine/integration/internal/container/ops.go | 7 +++++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/components/engine/integration/container/logs_test.go b/components/engine/integration/container/logs_test.go index 1157da14b8f..2d7306582fd 100644 --- a/components/engine/integration/container/logs_test.go +++ b/components/engine/integration/container/logs_test.go @@ -20,7 +20,6 @@ func TestLogsFollowTailEmpty(t *testing.T) { ctx := context.Background() id := container.Run(t, ctx, client, container.WithCmd("sleep", "100000")) - defer client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{Force: true}) logs, err := client.ContainerLogs(ctx, id, types.ContainerLogsOptions{ShowStdout: true, Tail: "2"}) if logs != nil { diff --git a/components/engine/integration/image/commit_test.go b/components/engine/integration/image/commit_test.go index a515b706af4..39fc956db11 100644 --- a/components/engine/integration/image/commit_test.go +++ b/components/engine/integration/image/commit_test.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/container" + "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/request" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -16,10 +16,9 @@ func TestCommitInheritsEnv(t *testing.T) { client := request.NewAPIClient(t) ctx := context.Background() - createResp1, err := client.ContainerCreate(ctx, &container.Config{Image: "busybox"}, nil, nil, "") - require.NoError(t, err) + cID1 := container.Create(t, ctx, client) - commitResp1, err := client.ContainerCommit(ctx, createResp1.ID, types.ContainerCommitOptions{ + commitResp1, err := client.ContainerCommit(ctx, cID1, types.ContainerCommitOptions{ Changes: []string{"ENV PATH=/bin"}, Reference: "test-commit-image", }) @@ -31,10 +30,9 @@ func TestCommitInheritsEnv(t *testing.T) { expectedEnv1 := []string{"PATH=/bin"} assert.Equal(t, expectedEnv1, image1.Config.Env) - createResp2, err := client.ContainerCreate(ctx, &container.Config{Image: image1.ID}, nil, nil, "") - require.NoError(t, err) + cID2 := container.Create(t, ctx, client, container.WithImage(image1.ID)) - commitResp2, err := client.ContainerCommit(ctx, createResp2.ID, types.ContainerCommitOptions{ + commitResp2, err := client.ContainerCommit(ctx, cID2, types.ContainerCommitOptions{ Changes: []string{"ENV PATH=/usr/bin:$PATH"}, Reference: "test-commit-image", }) diff --git a/components/engine/integration/internal/container/ops.go b/components/engine/integration/internal/container/ops.go index 9360527d370..b4ad66f93dd 100644 --- a/components/engine/integration/internal/container/ops.go +++ b/components/engine/integration/internal/container/ops.go @@ -22,6 +22,13 @@ func WithLinks(links ...string) func(*TestContainerConfig) { } } +// WithImage sets the image of the container +func WithImage(image string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.Config.Image = image + } +} + // WithCmd sets the comannds of the container func WithCmd(cmds ...string) func(*TestContainerConfig) { return func(c *TestContainerConfig) { From 6fbd783f33f094de314656d486932dcec007f7b3 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Fri, 2 Mar 2018 22:54:29 +0000 Subject: [PATCH 17/24] Migrate docker rm tests to api tests This fix migrates docker rm test in integration-cli to api tests. Signed-off-by: Yong Tang (cherry picked from commit ed58ba99fb28ceac56063b7f003f38b597ddef80) Signed-off-by: Sebastiaan van Stijn --- .../integration-cli/docker_cli_rm_test.go | 32 ---------- .../engine/integration/image/remove_test.go | 60 +++++++++++++++++++ 2 files changed, 60 insertions(+), 32 deletions(-) delete mode 100644 components/engine/integration-cli/docker_cli_rm_test.go create mode 100644 components/engine/integration/image/remove_test.go diff --git a/components/engine/integration-cli/docker_cli_rm_test.go b/components/engine/integration-cli/docker_cli_rm_test.go deleted file mode 100644 index 5942b8286d9..00000000000 --- a/components/engine/integration-cli/docker_cli_rm_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package main - -import ( - "github.com/docker/docker/integration-cli/checker" - "github.com/docker/docker/integration-cli/cli/build" - "github.com/go-check/check" -) - -func (s *DockerSuite) TestRmContainerOrphaning(c *check.C) { - dockerfile1 := `FROM busybox:latest - ENTRYPOINT ["true"]` - img := "test-container-orphaning" - dockerfile2 := `FROM busybox:latest - ENTRYPOINT ["true"] - MAINTAINER Integration Tests` - - // build first dockerfile - buildImageSuccessfully(c, img, build.WithDockerfile(dockerfile1)) - img1 := getIDByName(c, img) - // run container on first image - dockerCmd(c, "run", img) - // rebuild dockerfile with a small addition at the end - buildImageSuccessfully(c, img, build.WithDockerfile(dockerfile2)) - // try to remove the image, should not error out. - out, _, err := dockerCmdWithError("rmi", img) - c.Assert(err, check.IsNil, check.Commentf("Expected to removing the image, but failed: %s", out)) - - // check if we deleted the first image - out, _ = dockerCmd(c, "images", "-q", "--no-trunc") - c.Assert(out, checker.Contains, img1, check.Commentf("Orphaned container (could not find %q in docker images): %s", img1, out)) - -} diff --git a/components/engine/integration/image/remove_test.go b/components/engine/integration/image/remove_test.go new file mode 100644 index 00000000000..825724bd039 --- /dev/null +++ b/components/engine/integration/image/remove_test.go @@ -0,0 +1,60 @@ +package image // import "github.com/docker/docker/integration/image" + +import ( + "context" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/integration/internal/request" + "github.com/docker/docker/internal/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRemoveImageOrphaning(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + img := "test-container-orphaning" + + // Create a container from busybox, and commit a small change so we have a new image + cID1 := container.Create(t, ctx, client, container.WithCmd("")) + commitResp1, err := client.ContainerCommit(ctx, cID1, types.ContainerCommitOptions{ + Changes: []string{`ENTRYPOINT ["true"]`}, + Reference: img, + }) + require.NoError(t, err) + + // verifies that reference now points to first image + resp, _, err := client.ImageInspectWithRaw(ctx, img) + require.NoError(t, err) + assert.Equal(t, resp.ID, commitResp1.ID) + + // Create a container from created image, and commit a small change with same reference name + cID2 := container.Create(t, ctx, client, container.WithImage(img), container.WithCmd("")) + commitResp2, err := client.ContainerCommit(ctx, cID2, types.ContainerCommitOptions{ + Changes: []string{`LABEL Maintainer="Integration Tests"`}, + Reference: img, + }) + require.NoError(t, err) + + // verifies that reference now points to second image + resp, _, err = client.ImageInspectWithRaw(ctx, img) + require.NoError(t, err) + assert.Equal(t, resp.ID, commitResp2.ID) + + // try to remove the image, should not error out. + _, err = client.ImageRemove(ctx, img, types.ImageRemoveOptions{}) + require.NoError(t, err) + + // check if the first image is still there + resp, _, err = client.ImageInspectWithRaw(ctx, commitResp1.ID) + require.NoError(t, err) + assert.Equal(t, resp.ID, commitResp1.ID) + + // check if the second image has been deleted + _, _, err = client.ImageInspectWithRaw(ctx, commitResp2.ID) + testutil.ErrorContains(t, err, "No such image:") +} From b3929fad23df021920c8fa58ab7ab5180983ac8d Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sat, 3 Mar 2018 01:06:49 +0000 Subject: [PATCH 18/24] Remove unnecessary container.WithName in kill test This fix removes several unnecessary `container.WithName` usage in docker kill integration test. Signed-off-by: Yong Tang (cherry picked from commit 1778719d6ac166250acfaebe5dd99b5e9d151c3e) Signed-off-by: Sebastiaan van Stijn --- components/engine/integration/container/kill_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/components/engine/integration/container/kill_test.go b/components/engine/integration/container/kill_test.go index 09e37ac0d19..d28f982e09a 100644 --- a/components/engine/integration/container/kill_test.go +++ b/components/engine/integration/container/kill_test.go @@ -155,8 +155,7 @@ func TestInspectOomKilledTrue(t *testing.T) { ctx := context.Background() client := request.NewAPIClient(t) - name := "testoomkilled" - cID := container.Run(t, ctx, client, container.WithName(name), container.WithCmd("sh", "-c", "x=a; while true; do x=$x$x$x$x; done"), func(c *container.TestContainerConfig) { + cID := container.Run(t, ctx, client, container.WithCmd("sh", "-c", "x=a; while true; do x=$x$x$x$x; done"), func(c *container.TestContainerConfig) { c.HostConfig.Resources.Memory = 32 * 1024 * 1024 }) @@ -174,8 +173,7 @@ func TestInspectOomKilledFalse(t *testing.T) { ctx := context.Background() client := request.NewAPIClient(t) - name := "testoomkilled" - cID := container.Run(t, ctx, client, container.WithName(name), container.WithCmd("sh", "-c", "echo hello world")) + cID := container.Run(t, ctx, client, container.WithCmd("sh", "-c", "echo hello world")) poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond)) From 04ebff9f98ecbc14503029bfa944c00249f8d24e Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 5 Mar 2018 16:51:46 +0000 Subject: [PATCH 19/24] Address `expected` vs `actual` in integration tests This fix addresses `expected` vs `actual` in integration tests so that they match `assert.Equal(t, expected, actual)` Signed-off-by: Yong Tang (cherry picked from commit 8a854e933b3dbb26cfce28b920cff61909412c6f) Signed-off-by: Sebastiaan van Stijn --- components/engine/integration/container/inspect_test.go | 4 ++-- components/engine/integration/container/kill_test.go | 4 ++-- components/engine/integration/container/pause_test.go | 6 +++--- components/engine/integration/container/remove_test.go | 4 ++-- components/engine/integration/container/resize_test.go | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/components/engine/integration/container/inspect_test.go b/components/engine/integration/container/inspect_test.go index 2eace554e4c..c7ea23b5179 100644 --- a/components/engine/integration/container/inspect_test.go +++ b/components/engine/integration/container/inspect_test.go @@ -40,9 +40,9 @@ func TestInspectCpusetInConfigPre120(t *testing.T) { require.NoError(t, err, "unable to unmarshal body for version 1.19: %s", err) config, ok := inspectJSON["Config"] - assert.Equal(t, ok, true, "Unable to find 'Config'") + assert.Equal(t, true, ok, "Unable to find 'Config'") cfg := config.(map[string]interface{}) _, ok = cfg["Cpuset"] - assert.Equal(t, ok, true, "API version 1.19 expected to include Cpuset in 'Config'") + assert.Equal(t, true, ok, "API version 1.19 expected to include Cpuset in 'Config'") } diff --git a/components/engine/integration/container/kill_test.go b/components/engine/integration/container/kill_test.go index d28f982e09a..dbffc802a55 100644 --- a/components/engine/integration/container/kill_test.go +++ b/components/engine/integration/container/kill_test.go @@ -163,7 +163,7 @@ func TestInspectOomKilledTrue(t *testing.T) { inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.State.OOMKilled, true) + assert.Equal(t, true, inspect.State.OOMKilled) } func TestInspectOomKilledFalse(t *testing.T) { @@ -179,5 +179,5 @@ func TestInspectOomKilledFalse(t *testing.T) { inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.State.OOMKilled, false) + assert.Equal(t, false, inspect.State.OOMKilled) } diff --git a/components/engine/integration/container/pause_test.go b/components/engine/integration/container/pause_test.go index 7c2aa96c754..bf9f9c3d8ff 100644 --- a/components/engine/integration/container/pause_test.go +++ b/components/engine/integration/container/pause_test.go @@ -35,7 +35,7 @@ func TestPause(t *testing.T) { inspect, err := client.ContainerInspect(ctx, cID) require.NoError(t, err) - assert.Equal(t, inspect.State.Paused, true) + assert.Equal(t, true, inspect.State.Paused) err = client.ContainerUnpause(ctx, cID) require.NoError(t, err) @@ -47,7 +47,7 @@ func TestPause(t *testing.T) { Until: until, Filters: filters.NewArgs(filters.Arg("container", cID)), }) - assert.Equal(t, getEventActions(t, messages, errs), []string{"pause", "unpause"}) + assert.Equal(t, []string{"pause", "unpause"}, getEventActions(t, messages, errs)) } func TestPauseFailsOnWindowsServerContainers(t *testing.T) { @@ -88,7 +88,7 @@ func getEventActions(t *testing.T, messages <-chan events.Message, errs <-chan e for { select { case err := <-errs: - assert.Equal(t, err == nil || err == io.EOF, true) + assert.True(t, err == nil || err == io.EOF) return actions case e := <-messages: actions = append(actions, e.Status) diff --git a/components/engine/integration/container/remove_test.go b/components/engine/integration/container/remove_test.go index 22fa26af67c..98aacdd2056 100644 --- a/components/engine/integration/container/remove_test.go +++ b/components/engine/integration/container/remove_test.go @@ -66,7 +66,7 @@ func TestRemoveContainerWithVolume(t *testing.T) { insp, _, err := client.ContainerInspectWithRaw(ctx, cID, true) require.NoError(t, err) - assert.Equal(t, len(insp.Mounts), 1) + assert.Equal(t, 1, len(insp.Mounts)) volName := insp.Mounts[0].Name err = client.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{ @@ -76,7 +76,7 @@ func TestRemoveContainerWithVolume(t *testing.T) { volumes, err := client.VolumeList(ctx, filters.NewArgs(filters.Arg("name", volName))) require.NoError(t, err) - assert.Equal(t, len(volumes.Volumes), 0) + assert.Equal(t, 0, len(volumes.Volumes)) } func TestRemoveContainerRunning(t *testing.T) { diff --git a/components/engine/integration/container/resize_test.go b/components/engine/integration/container/resize_test.go index 0a5f26067e8..18438ea8253 100644 --- a/components/engine/integration/container/resize_test.go +++ b/components/engine/integration/container/resize_test.go @@ -44,7 +44,7 @@ func TestResizeWithInvalidSize(t *testing.T) { endpoint := "/containers/" + cID + "/resize?h=foo&w=bar" res, _, err := req.Post(endpoint) require.NoError(t, err) - assert.Equal(t, res.StatusCode, http.StatusBadRequest) + assert.Equal(t, http.StatusBadRequest, res.StatusCode) } func TestResizeWhenContainerNotStarted(t *testing.T) { From e945f930249e4fd73584f0754d842d2ea6e03d45 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 5 Mar 2018 15:29:02 -0500 Subject: [PATCH 20/24] Adds a unit test for plugin request timeout Signed-off-by: Brian Goff (cherry picked from commit 7ca971fb495e4de4aa4455964625974464d86920) Signed-off-by: Sebastiaan van Stijn --- components/engine/pkg/plugins/client_test.go | 43 ++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/components/engine/pkg/plugins/client_test.go b/components/engine/pkg/plugins/client_test.go index 44dd0fa81c7..10c8d8fd567 100644 --- a/components/engine/pkg/plugins/client_test.go +++ b/components/engine/pkg/plugins/client_test.go @@ -2,6 +2,7 @@ package plugins // import "github.com/docker/docker/pkg/plugins" import ( "bytes" + "context" "encoding/json" "io" "net/http" @@ -13,7 +14,9 @@ import ( "github.com/docker/docker/pkg/plugins/transport" "github.com/docker/go-connections/tlsconfig" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -232,3 +235,43 @@ func TestClientSendFile(t *testing.T) { } assert.Equal(t, m, output) } + +func TestClientWithRequestTimeout(t *testing.T) { + timeout := 1 * time.Millisecond + testHandler := func(w http.ResponseWriter, r *http.Request) { + time.Sleep(timeout + 1*time.Millisecond) + w.WriteHeader(http.StatusOK) + } + + srv := httptest.NewServer(http.HandlerFunc(testHandler)) + defer srv.Close() + + client := &Client{http: srv.Client(), requestFactory: &testRequestWrapper{srv}} + _, err := client.callWithRetry("/Plugin.Hello", nil, false, WithRequestTimeout(timeout)) + require.Error(t, err, "expected error") + + err = errors.Cause(err) + + switch e := err.(type) { + case *url.Error: + err = e.Err + } + require.Equal(t, context.DeadlineExceeded, err) +} + +type testRequestWrapper struct { + *httptest.Server +} + +func (w *testRequestWrapper) NewRequest(path string, data io.Reader) (*http.Request, error) { + req, err := http.NewRequest("POST", path, data) + if err != nil { + return nil, err + } + u, err := url.Parse(w.Server.URL) + if err != nil { + return nil, err + } + req.URL = u + return req, nil +} From df6211ff9eb6319ca29348a1292d350e445f26a8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 6 Mar 2018 16:17:13 +0100 Subject: [PATCH 21/24] Remove duplicate TestServiceUpdatePort The TestAPIServiceUpdatePort test performs exactly the same steps. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 36e1646e4f010ea033643c6df3d9c3dccc166ed2) Signed-off-by: Sebastiaan van Stijn --- .../docker_cli_service_update_test.go | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_service_update_test.go b/components/engine/integration-cli/docker_cli_service_update_test.go index 8f0717074b2..7b590a0f4e5 100644 --- a/components/engine/integration-cli/docker_cli_service_update_test.go +++ b/components/engine/integration-cli/docker_cli_service_update_test.go @@ -11,41 +11,6 @@ import ( "github.com/go-check/check" ) -func (s *DockerSwarmSuite) TestServiceUpdatePort(c *check.C) { - d := s.AddDaemon(c, true, true) - - serviceName := "TestServiceUpdatePort" - serviceArgs := append([]string{"service", "create", "--detach", "--no-resolve-image", "--name", serviceName, "-p", "8080:8081", defaultSleepImage}, sleepCommandForDaemonPlatform()...) - - // Create a service with a port mapping of 8080:8081. - out, err := d.Cmd(serviceArgs...) - c.Assert(err, checker.IsNil) - waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) - - // Update the service: changed the port mapping from 8080:8081 to 8082:8083. - _, err = d.Cmd("service", "update", "--detach", "--publish-add", "8082:8083", "--publish-rm", "8081", serviceName) - c.Assert(err, checker.IsNil) - - // Inspect the service and verify port mapping - expected := []swarm.PortConfig{ - { - Protocol: "tcp", - PublishedPort: 8082, - TargetPort: 8083, - PublishMode: "ingress", - }, - } - - out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.EndpointSpec.Ports }}", serviceName) - c.Assert(err, checker.IsNil) - - var portConfig []swarm.PortConfig - if err := json.Unmarshal([]byte(out), &portConfig); err != nil { - c.Fatalf("invalid JSON in inspect result: %v (%s)", err, out) - } - c.Assert(portConfig, checker.DeepEquals, expected) -} - func (s *DockerSwarmSuite) TestServiceUpdateLabel(c *check.C) { d := s.AddDaemon(c, true, true) out, err := d.Cmd("service", "create", "--detach", "--no-resolve-image", "--name=test", "busybox", "top") From ae56939688d28f0cac4976208af4948b4c52972e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 9 Mar 2018 12:27:47 -0500 Subject: [PATCH 22/24] Remove unnecessary diff tests Signed-off-by: Daniel Nephin (cherry picked from commit 038f3add5191240058c7a4154556553c5493ea44) Signed-off-by: Sebastiaan van Stijn --- .../engine/integration/container/diff_test.go | 73 +++---------------- 1 file changed, 9 insertions(+), 64 deletions(-) diff --git a/components/engine/integration/container/diff_test.go b/components/engine/integration/container/diff_test.go index df602369983..de5ff4e21aa 100644 --- a/components/engine/integration/container/diff_test.go +++ b/components/engine/integration/container/diff_test.go @@ -10,13 +10,11 @@ import ( "github.com/docker/docker/integration/internal/request" "github.com/docker/docker/pkg/archive" "github.com/gotestyourself/gotestyourself/poll" - "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// ensure that an added file shows up in docker diff -func TestDiffFilenameShownInOutput(t *testing.T) { +func TestDiff(t *testing.T) { defer setupTest(t)() client := request.NewAPIClient(t) ctx := context.Background() @@ -27,72 +25,19 @@ func TestDiffFilenameShownInOutput(t *testing.T) { // it will take a few seconds to exit. Also there's no way in Windows to // differentiate between an Add or a Modify, and all files are under // a "Files/" prefix. - lookingFor := containertypes.ContainerChangeResponseItem{Kind: archive.ChangeAdd, Path: "/foo/bar"} + expected := []containertypes.ContainerChangeResponseItem{ + {Kind: archive.ChangeAdd, Path: "/foo"}, + {Kind: archive.ChangeAdd, Path: "/foo/bar"}, + } if testEnv.OSType == "windows" { poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second)) - lookingFor = containertypes.ContainerChangeResponseItem{Kind: archive.ChangeModify, Path: "Files/foo/bar"} - } - - items, err := client.ContainerDiff(ctx, cID) - require.NoError(t, err) - assert.Contains(t, items, lookingFor) -} - -// test to ensure GH #3840 doesn't occur any more -func TestDiffEnsureInitLayerFilesAreIgnored(t *testing.T) { - skip.If(t, testEnv.DaemonInfo.OSType != "linux") - - defer setupTest(t)() - client := request.NewAPIClient(t) - ctx := context.Background() - - // this is a list of files which shouldn't show up in `docker diff` - initLayerFiles := []string{"/etc/resolv.conf", "/etc/hostname", "/etc/hosts", "/.dockerenv"} - containerCount := 5 - - // we might not run into this problem from the first run, so start a few containers - for i := 0; i < containerCount; i++ { - cID := container.Run(t, ctx, client, container.WithCmd("sh", "-c", `echo foo > /root/bar`)) - - items, err := client.ContainerDiff(ctx, cID) - require.NoError(t, err) - for _, item := range items { - assert.NotContains(t, initLayerFiles, item.Path) + expected = []containertypes.ContainerChangeResponseItem{ + {Kind: archive.ChangeModify, Path: "Files/foo"}, + {Kind: archive.ChangeModify, Path: "Files/foo/bar"}, } } -} - -func TestDiffEnsureDefaultDevs(t *testing.T) { - skip.If(t, testEnv.DaemonInfo.OSType != "linux") - - defer setupTest(t)() - client := request.NewAPIClient(t) - ctx := context.Background() - - cID := container.Run(t, ctx, client, container.WithCmd("sleep", "0")) items, err := client.ContainerDiff(ctx, cID) require.NoError(t, err) - - expected := []containertypes.ContainerChangeResponseItem{ - {Kind: archive.ChangeModify, Path: "/dev"}, - {Kind: archive.ChangeAdd, Path: "/dev/full"}, // busybox - {Kind: archive.ChangeModify, Path: "/dev/ptmx"}, // libcontainer - {Kind: archive.ChangeAdd, Path: "/dev/mqueue"}, - {Kind: archive.ChangeAdd, Path: "/dev/kmsg"}, - {Kind: archive.ChangeAdd, Path: "/dev/fd"}, - {Kind: archive.ChangeAdd, Path: "/dev/ptmx"}, - {Kind: archive.ChangeAdd, Path: "/dev/null"}, - {Kind: archive.ChangeAdd, Path: "/dev/random"}, - {Kind: archive.ChangeAdd, Path: "/dev/stdout"}, - {Kind: archive.ChangeAdd, Path: "/dev/stderr"}, - {Kind: archive.ChangeAdd, Path: "/dev/tty1"}, - {Kind: archive.ChangeAdd, Path: "/dev/stdin"}, - {Kind: archive.ChangeAdd, Path: "/dev/tty"}, - {Kind: archive.ChangeAdd, Path: "/dev/urandom"}, - } - - for _, item := range items { - assert.Contains(t, expected, item) - } + assert.Equal(t, expected, items) } From c25858e94c7d0a69152fbdfa0b402e6913e4193b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 12 Mar 2018 18:08:59 -0700 Subject: [PATCH 23/24] TestLinksEtcHostsContentMatch: use container.Exec() I am not quite sure why but this test is sometimes failing like this: > 15:21:41 --- FAIL: TestLinksEtcHostsContentMatch (0.53s) > 15:21:41 assertions.go:226: > > Error Trace: links_linux_test.go:46 > 15:21:41 > Error: Not equal: > 15:21:41 > expected: "127.0.0.1\tlocalhost\n::1\tlocalhost > ip6-localhost > ip6-loopback\nfe00::0\tip6-localnet\nff00::0\tip6-mcastprefix\nff02::1\tip6-allnodes\nff02::2\tip6-allrouters\n172.17.0.2\tf53feb6df161\n" > 15:21:41 > received: "" To eliminate some possible failures (like ignoring stderr from `cat` or its exit code), let's use container.Exec() to read a file from a container. Fixes: e6bd20edcbf ("Migrate some integration-cli test to api tests") Signed-off-by: Kir Kolyshkin (cherry picked from commit ad2f88d8ccbd9dd0a8d9c4f96ece3956f60489df) Signed-off-by: Sebastiaan van Stijn --- .../integration/container/links_linux_test.go | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/components/engine/integration/container/links_linux_test.go b/components/engine/integration/container/links_linux_test.go index d230898ed0e..c844c4916fb 100644 --- a/components/engine/integration/container/links_linux_test.go +++ b/components/engine/integration/container/links_linux_test.go @@ -1,19 +1,15 @@ package container // import "github.com/docker/docker/integration/container" import ( - "bytes" "context" "io/ioutil" "os" "testing" - "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/request" - "github.com/docker/docker/pkg/stdcopy" - "github.com/gotestyourself/gotestyourself/poll" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -29,21 +25,13 @@ func TestLinksEtcHostsContentMatch(t *testing.T) { client := request.NewAPIClient(t) ctx := context.Background() - cID := container.Run(t, ctx, client, container.WithCmd("cat", "/etc/hosts"), container.WithNetworkMode("host")) - - poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) - - body, err := client.ContainerLogs(ctx, cID, types.ContainerLogsOptions{ - ShowStdout: true, - }) - require.NoError(t, err) - defer body.Close() - - var b bytes.Buffer - _, err = stdcopy.StdCopy(&b, ioutil.Discard, body) + cID := container.Run(t, ctx, client, container.WithNetworkMode("host")) + res, err := container.Exec(ctx, client, cID, []string{"cat", "/etc/hosts"}) require.NoError(t, err) + require.Empty(t, res.Stderr()) + require.Equal(t, 0, res.ExitCode) - assert.Equal(t, string(hosts), b.String()) + assert.Equal(t, string(hosts), res.Stdout()) } func TestLinksContainerNames(t *testing.T) { From a3540962fd234faa30385d5f6ef3a96c61610ccd Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 13 Mar 2018 15:39:23 -0400 Subject: [PATCH 24/24] Add some tests to the volume store Signed-off-by: Brian Goff (cherry picked from commit 834d0e262ac248191c09bcdb2b86ee92edb6aaf0) Signed-off-by: Sebastiaan van Stijn --- .../engine/integration/volume/volume_test.go | 2 - components/engine/volume/store/db.go | 11 +- components/engine/volume/store/db_test.go | 51 +++++++++ .../engine/volume/store/restore_test.go | 55 ++++++++++ components/engine/volume/store/store.go | 15 ++- components/engine/volume/store/store_test.go | 103 ++++++++++++++++++ 6 files changed, 231 insertions(+), 6 deletions(-) create mode 100644 components/engine/volume/store/db_test.go create mode 100644 components/engine/volume/store/restore_test.go diff --git a/components/engine/integration/volume/volume_test.go b/components/engine/integration/volume/volume_test.go index 38ce5782e1d..20dcfef9a69 100644 --- a/components/engine/integration/volume/volume_test.go +++ b/components/engine/integration/volume/volume_test.go @@ -34,7 +34,6 @@ func TestVolumesCreateAndList(t *testing.T) { Driver: "local", Scope: "local", Name: name, - Options: map[string]string{}, Mountpoint: fmt.Sprintf("%s/volumes/%s/_data", testEnv.DaemonInfo.DockerRootDir, name), } assert.Equal(t, vol, expected) @@ -95,7 +94,6 @@ func TestVolumesInspect(t *testing.T) { Driver: "local", Scope: "local", Name: name, - Options: map[string]string{}, Mountpoint: fmt.Sprintf("%s/volumes/%s/_data", testEnv.DaemonInfo.DockerRootDir, name), } assert.Equal(t, vol, expected) diff --git a/components/engine/volume/store/db.go b/components/engine/volume/store/db.go index fe6dbae9afa..5a280ca2dca 100644 --- a/components/engine/volume/store/db.go +++ b/components/engine/volume/store/db.go @@ -4,6 +4,7 @@ import ( "encoding/json" "github.com/boltdb/bolt" + "github.com/docker/docker/errdefs" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -28,7 +29,10 @@ func setMeta(tx *bolt.Tx, name string, meta volumeMetadata) error { if err != nil { return err } - b := tx.Bucket(volumeBucketName) + b, err := tx.CreateBucketIfNotExists(volumeBucketName) + if err != nil { + return errors.Wrap(err, "error creating volume bucket") + } return errors.Wrap(b.Put([]byte(name), metaJSON), "error setting volume metadata") } @@ -42,8 +46,11 @@ func (s *VolumeStore) getMeta(name string) (volumeMetadata, error) { func getMeta(tx *bolt.Tx, name string, meta *volumeMetadata) error { b := tx.Bucket(volumeBucketName) + if b == nil { + return errdefs.NotFound(errors.New("volume bucket does not exist")) + } val := b.Get([]byte(name)) - if string(val) == "" { + if len(val) == 0 { return nil } if err := json.Unmarshal(val, meta); err != nil { diff --git a/components/engine/volume/store/db_test.go b/components/engine/volume/store/db_test.go new file mode 100644 index 00000000000..9c45cd13baf --- /dev/null +++ b/components/engine/volume/store/db_test.go @@ -0,0 +1,51 @@ +package store + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + "time" + + "github.com/boltdb/bolt" + "github.com/stretchr/testify/require" +) + +func TestSetGetMeta(t *testing.T) { + t.Parallel() + + dir, err := ioutil.TempDir("", "test-set-get") + require.NoError(t, err) + defer os.RemoveAll(dir) + + db, err := bolt.Open(filepath.Join(dir, "db"), 0600, &bolt.Options{Timeout: 1 * time.Second}) + require.NoError(t, err) + + store := &VolumeStore{db: db} + + _, err = store.getMeta("test") + require.Error(t, err) + + err = db.Update(func(tx *bolt.Tx) error { + _, err := tx.CreateBucket(volumeBucketName) + return err + }) + require.NoError(t, err) + + meta, err := store.getMeta("test") + require.NoError(t, err) + require.Equal(t, volumeMetadata{}, meta) + + testMeta := volumeMetadata{ + Name: "test", + Driver: "fake", + Labels: map[string]string{"a": "1", "b": "2"}, + Options: map[string]string{"foo": "bar"}, + } + err = store.setMeta("test", testMeta) + require.NoError(t, err) + + meta, err = store.getMeta("test") + require.NoError(t, err) + require.Equal(t, testMeta, meta) +} diff --git a/components/engine/volume/store/restore_test.go b/components/engine/volume/store/restore_test.go new file mode 100644 index 00000000000..83efd36b63e --- /dev/null +++ b/components/engine/volume/store/restore_test.go @@ -0,0 +1,55 @@ +package store + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/docker/docker/volume" + volumedrivers "github.com/docker/docker/volume/drivers" + volumetestutils "github.com/docker/docker/volume/testutils" + "github.com/stretchr/testify/require" +) + +func TestRestore(t *testing.T) { + t.Parallel() + + dir, err := ioutil.TempDir("", "test-restore") + require.NoError(t, err) + defer os.RemoveAll(dir) + + driverName := "test-restore" + volumedrivers.Register(volumetestutils.NewFakeDriver(driverName), driverName) + defer volumedrivers.Unregister("test-restore") + + s, err := New(dir) + require.NoError(t, err) + defer s.Shutdown() + + _, err = s.Create("test1", driverName, nil, nil) + require.NoError(t, err) + + testLabels := map[string]string{"a": "1"} + testOpts := map[string]string{"foo": "bar"} + _, err = s.Create("test2", driverName, testOpts, testLabels) + require.NoError(t, err) + + s.Shutdown() + + s, err = New(dir) + require.NoError(t, err) + + v, err := s.Get("test1") + require.NoError(t, err) + + dv := v.(volume.DetailedVolume) + var nilMap map[string]string + require.Equal(t, nilMap, dv.Options()) + require.Equal(t, nilMap, dv.Labels()) + + v, err = s.Get("test2") + require.NoError(t, err) + dv = v.(volume.DetailedVolume) + require.Equal(t, testOpts, dv.Options()) + require.Equal(t, testLabels, dv.Labels()) +} diff --git a/components/engine/volume/store/store.go b/components/engine/volume/store/store.go index 643096a781a..70dd8b2d89a 100644 --- a/components/engine/volume/store/store.go +++ b/components/engine/volume/store/store.go @@ -29,7 +29,10 @@ type volumeWrapper struct { } func (v volumeWrapper) Options() map[string]string { - options := map[string]string{} + if v.options == nil { + return nil + } + options := make(map[string]string, len(v.options)) for key, value := range v.options { options[key] = value } @@ -37,7 +40,15 @@ func (v volumeWrapper) Options() map[string]string { } func (v volumeWrapper) Labels() map[string]string { - return v.labels + if v.labels == nil { + return nil + } + + labels := make(map[string]string, len(v.labels)) + for key, value := range v.labels { + labels[key] = value + } + return labels } func (v volumeWrapper) Scope() string { diff --git a/components/engine/volume/store/store_test.go b/components/engine/volume/store/store_test.go index 54b7d7cfcef..e53c728dd0a 100644 --- a/components/engine/volume/store/store_test.go +++ b/components/engine/volume/store/store_test.go @@ -12,6 +12,8 @@ import ( "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" volumetestutils "github.com/docker/docker/volume/testutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCreate(t *testing.T) { @@ -291,6 +293,7 @@ func TestDefererencePluginOnCreateError(t *testing.T) { pg := volumetestutils.NewFakePluginGetter(p) volumedrivers.RegisterPluginGetter(pg) + defer volumedrivers.RegisterPluginGetter(nil) dir, err := ioutil.TempDir("", "test-plugin-deref-err") if err != nil { @@ -320,3 +323,103 @@ func TestDefererencePluginOnCreateError(t *testing.T) { t.Fatalf("expected 1 plugin reference, got: %d", refs) } } + +func TestRefDerefRemove(t *testing.T) { + t.Parallel() + + driverName := "test-ref-deref-remove" + s, cleanup := setupTest(t, driverName) + defer cleanup(t) + + v, err := s.CreateWithRef("test", driverName, "test-ref", nil, nil) + require.NoError(t, err) + + err = s.Remove(v) + require.Error(t, err) + require.Equal(t, errVolumeInUse, err.(*OpErr).Err) + + s.Dereference(v, "test-ref") + err = s.Remove(v) + require.NoError(t, err) +} + +func TestGet(t *testing.T) { + t.Parallel() + + driverName := "test-get" + s, cleanup := setupTest(t, driverName) + defer cleanup(t) + + _, err := s.Get("not-exist") + require.Error(t, err) + require.Equal(t, errNoSuchVolume, err.(*OpErr).Err) + + v1, err := s.Create("test", driverName, nil, map[string]string{"a": "1"}) + require.NoError(t, err) + + v2, err := s.Get("test") + require.NoError(t, err) + require.Equal(t, v1, v2) + + dv := v2.(volume.DetailedVolume) + require.Equal(t, "1", dv.Labels()["a"]) + + err = s.Remove(v1) + require.NoError(t, err) +} + +func TestGetWithRef(t *testing.T) { + t.Parallel() + + driverName := "test-get-with-ref" + s, cleanup := setupTest(t, driverName) + defer cleanup(t) + + _, err := s.GetWithRef("not-exist", driverName, "test-ref") + require.Error(t, err) + + v1, err := s.Create("test", driverName, nil, map[string]string{"a": "1"}) + require.NoError(t, err) + + v2, err := s.GetWithRef("test", driverName, "test-ref") + require.NoError(t, err) + require.Equal(t, v1, v2) + + err = s.Remove(v2) + require.Error(t, err) + require.Equal(t, errVolumeInUse, err.(*OpErr).Err) + + s.Dereference(v2, "test-ref") + err = s.Remove(v2) + require.NoError(t, err) +} + +func setupTest(t *testing.T, name string) (*VolumeStore, func(*testing.T)) { + t.Helper() + s, cleanup := newTestStore(t) + + volumedrivers.Register(volumetestutils.NewFakeDriver(name), name) + return s, func(t *testing.T) { + cleanup(t) + volumedrivers.Unregister(name) + } +} + +func newTestStore(t *testing.T) (*VolumeStore, func(*testing.T)) { + t.Helper() + + dir, err := ioutil.TempDir("", "store-root") + require.NoError(t, err) + + cleanup := func(t *testing.T) { + err := os.RemoveAll(dir) + assert.NoError(t, err) + } + + s, err := New(dir) + assert.NoError(t, err) + return s, func(t *testing.T) { + s.Shutdown() + cleanup(t) + } +}