Skip to content

Commit

Permalink
Merge pull request #29563 from yongtang/21845-duplicate-mount-point-v…
Browse files Browse the repository at this point in the history 
…olumes-from

Fix duplicate mount points for multiple `--volumes-from` in `docker run`
  • Loading branch information
@cpuguy83
cpuguy83 committed Feb 7, 2017
2 parents bbd4c87 + 9526e5c commit 5381f9f
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 1 deletion.
13 changes: 12 additions & 1 deletion daemon/volumes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -121,7 +130,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
}
cp.Volume = v
}

dereferenceIfExists(cp.Destination)
mountPoints[cp.Destination] = cp
}
}
Expand Down Expand Up @@ -155,6 +164,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
}

binds[bind.Destination] = true
dereferenceIfExists(bind.Destination)
mountPoints[bind.Destination] = bind
}

Expand Down Expand Up @@ -199,6 +209,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
}

binds[mp.Destination] = true
dereferenceIfExists(mp.Destination)
mountPoints[mp.Destination] = mp
}

Expand Down
148 changes: 148 additions & 0 deletions integration-cli/docker_cli_volume_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}

0 comments on commit 5381f9f

Please sign in to comment.