diff --git a/daemon/volumes.go b/daemon/volumes.go index 10cf787709..fa149b05c5 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -85,6 +85,15 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo } }() + dereferenceIfExists := func(destination string) { + if v, ok := mountPoints[destination]; ok { + logrus.Debugf("Duplicate mount point '%s'", destination) + if v.Volume != nil { + daemon.volumes.Dereference(v.Volume, container.ID) + } + } + } + // 1. Read already configured mount points. for destination, point := range container.MountPoints { mountPoints[destination] = point @@ -121,7 +130,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo } cp.Volume = v } - + dereferenceIfExists(cp.Destination) mountPoints[cp.Destination] = cp } } @@ -155,6 +164,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo } binds[bind.Destination] = true + dereferenceIfExists(bind.Destination) mountPoints[bind.Destination] = bind } @@ -199,6 +209,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo } binds[mp.Destination] = true + dereferenceIfExists(mp.Destination) mountPoints[mp.Destination] = mp } diff --git a/integration-cli/docker_cli_volume_test.go b/integration-cli/docker_cli_volume_test.go index 4c5b090864..2bdb063030 100644 --- a/integration-cli/docker_cli_volume_test.go +++ b/integration-cli/docker_cli_volume_test.go @@ -3,12 +3,14 @@ package main import ( "fmt" "io/ioutil" + "net/http" "os" "os/exec" "path/filepath" "strings" "github.com/docker/docker/integration-cli/checker" + "github.com/docker/docker/integration-cli/request" icmd "github.com/docker/docker/pkg/testutil/cmd" "github.com/go-check/check" ) @@ -449,3 +451,149 @@ func (s *DockerSuite) TestVolumeCliInspectWithVolumeOpts(c *check.C) { c.Assert(strings.TrimSpace(out), checker.Contains, fmt.Sprintf("%s:%s", k2, v2)) c.Assert(strings.TrimSpace(out), checker.Contains, fmt.Sprintf("%s:%s", k3, v3)) } + +// Test case (1) for 21845: duplicate targets for --volumes-from +func (s *DockerSuite) TestDuplicateMountpointsForVolumesFrom(c *check.C) { + testRequires(c, DaemonIsLinux) + + image := "vimage" + buildImageSuccessfully(c, image, withDockerfile(` + FROM busybox + VOLUME ["/tmp/data"]`)) + + dockerCmd(c, "run", "--name=data1", image, "true") + dockerCmd(c, "run", "--name=data2", image, "true") + + out, _ := dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "data1") + data1 := strings.TrimSpace(out) + c.Assert(data1, checker.Not(checker.Equals), "") + + out, _ = dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "data2") + data2 := strings.TrimSpace(out) + c.Assert(data2, checker.Not(checker.Equals), "") + + // Both volume should exist + out, _ = dockerCmd(c, "volume", "ls", "-q") + c.Assert(strings.TrimSpace(out), checker.Contains, data1) + c.Assert(strings.TrimSpace(out), checker.Contains, data2) + + out, _, err := dockerCmdWithError("run", "--name=app", "--volumes-from=data1", "--volumes-from=data2", "-d", "busybox", "top") + c.Assert(err, checker.IsNil, check.Commentf("Out: %s", out)) + + // Only the second volume will be referenced, this is backward compatible + out, _ = dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "app") + c.Assert(strings.TrimSpace(out), checker.Equals, data2) + + dockerCmd(c, "rm", "-f", "-v", "app") + dockerCmd(c, "rm", "-f", "-v", "data1") + dockerCmd(c, "rm", "-f", "-v", "data2") + + // Both volume should not exist + out, _ = dockerCmd(c, "volume", "ls", "-q") + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data1) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data2) +} + +// Test case (2) for 21845: duplicate targets for --volumes-from and -v (bind) +func (s *DockerSuite) TestDuplicateMountpointsForVolumesFromAndBind(c *check.C) { + testRequires(c, DaemonIsLinux) + + image := "vimage" + buildImageSuccessfully(c, image, withDockerfile(` + FROM busybox + VOLUME ["/tmp/data"]`)) + + dockerCmd(c, "run", "--name=data1", image, "true") + dockerCmd(c, "run", "--name=data2", image, "true") + + out, _ := dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "data1") + data1 := strings.TrimSpace(out) + c.Assert(data1, checker.Not(checker.Equals), "") + + out, _ = dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "data2") + data2 := strings.TrimSpace(out) + c.Assert(data2, checker.Not(checker.Equals), "") + + // Both volume should exist + out, _ = dockerCmd(c, "volume", "ls", "-q") + c.Assert(strings.TrimSpace(out), checker.Contains, data1) + c.Assert(strings.TrimSpace(out), checker.Contains, data2) + + out, _, err := dockerCmdWithError("run", "--name=app", "--volumes-from=data1", "--volumes-from=data2", "-v", "/tmp/data:/tmp/data", "-d", "busybox", "top") + c.Assert(err, checker.IsNil, check.Commentf("Out: %s", out)) + + // No volume will be referenced (mount is /tmp/data), this is backward compatible + out, _ = dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "app") + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data1) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data2) + + dockerCmd(c, "rm", "-f", "-v", "app") + dockerCmd(c, "rm", "-f", "-v", "data1") + dockerCmd(c, "rm", "-f", "-v", "data2") + + // Both volume should not exist + out, _ = dockerCmd(c, "volume", "ls", "-q") + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data1) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data2) +} + +// Test case (3) for 21845: duplicate targets for --volumes-from and `Mounts` (API only) +func (s *DockerSuite) TestDuplicateMountpointsForVolumesFromAndMounts(c *check.C) { + testRequires(c, DaemonIsLinux) + + image := "vimage" + buildImageSuccessfully(c, image, withDockerfile(` + FROM busybox + VOLUME ["/tmp/data"]`)) + + dockerCmd(c, "run", "--name=data1", image, "true") + dockerCmd(c, "run", "--name=data2", image, "true") + + out, _ := dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "data1") + data1 := strings.TrimSpace(out) + c.Assert(data1, checker.Not(checker.Equals), "") + + out, _ = dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "data2") + data2 := strings.TrimSpace(out) + c.Assert(data2, checker.Not(checker.Equals), "") + + // Both volume should exist + out, _ = dockerCmd(c, "volume", "ls", "-q") + c.Assert(strings.TrimSpace(out), checker.Contains, data1) + c.Assert(strings.TrimSpace(out), checker.Contains, data2) + + // Mounts is available in API + status, body, err := request.SockRequest("POST", "/containers/create?name=app", map[string]interface{}{ + "Image": "busybox", + "Cmd": []string{"top"}, + "HostConfig": map[string]interface{}{ + "VolumesFrom": []string{ + "data1", + "data2", + }, + "Mounts": []map[string]interface{}{ + { + "Type": "bind", + "Source": "/tmp/data", + "Target": "/tmp/data", + }, + }}, + }, daemonHost()) + + c.Assert(err, checker.IsNil, check.Commentf(string(body))) + c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body))) + + // No volume will be referenced (mount is /tmp/data), this is backward compatible + out, _ = dockerCmd(c, "inspect", "--format", "{{(index .Mounts 0).Name}}", "app") + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data1) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data2) + + dockerCmd(c, "rm", "-f", "-v", "app") + dockerCmd(c, "rm", "-f", "-v", "data1") + dockerCmd(c, "rm", "-f", "-v", "data2") + + // Both volume should not exist + out, _ = dockerCmd(c, "volume", "ls", "-q") + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data1) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Contains), data2) +}