From 882223c0f8a55c2011277aba08f1487a2930c075 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 26 Sep 2014 14:28:21 -0400 Subject: [PATCH] Fix #8259 - Can't reuse symlink'd bindmount volumes.Get was not checking for symlinked paths meanwhile when adding a new volume it was following the symlink. So when trying to use a bind-mount that is a symlink, the volume is added with the correct path, but when another container tries to use the same volume it got a "Volume exists" error because volumes.Get returned nil and as such attempted to create a new volume. Docker-DCO-1.1-Signed-off-by: Brian Goff (github: cpuguy83) --- integration-cli/docker_cli_run_test.go | 31 ++++++++++++++++++++++++++ volumes/repository.go | 10 ++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index f1936bcecb..01a3f57638 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2221,3 +2221,34 @@ func TestRunRedirectStdout(t *testing.T) { logDone("run - redirect stdout") } + +// Regression test for https://github.com/docker/docker/issues/8259 +func TestRunReuseBindVolumeThatIsSymlink(t *testing.T) { + tmpDir, err := ioutil.TempDir(os.TempDir(), "testlink") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + linkPath := os.TempDir() + "/testlink2" + if err := os.Symlink(tmpDir, linkPath); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(linkPath) + + // Create first container + cmd := exec.Command(dockerBinary, "run", "-v", fmt.Sprintf("%s:/tmp/test", linkPath), "busybox", "ls", "-lh", "/tmp/test") + if _, err := runCommand(cmd); err != nil { + t.Fatal(err) + } + + // Create second container with same symlinked path + // This will fail if the referenced issue is hit with a "Volume exists" error + cmd = exec.Command(dockerBinary, "run", "-v", fmt.Sprintf("%s:/tmp/test", linkPath), "busybox", "ls", "-lh", "/tmp/test") + if out, _, err := runCommandWithOutput(cmd); err != nil { + t.Fatal(err, out) + } + + deleteAllContainers() + logDone("run - can remount old bindmount volume") +} diff --git a/volumes/repository.go b/volumes/repository.go index 1c5de96bb3..b09af8de26 100644 --- a/volumes/repository.go +++ b/volumes/repository.go @@ -97,12 +97,16 @@ func (r *Repository) restore() error { func (r *Repository) Get(path string) *Volume { r.lock.Lock() - vol := r.volumes[path] + vol := r.get(path) r.lock.Unlock() return vol } func (r *Repository) get(path string) *Volume { + path, err := filepath.EvalSymlinks(path) + if err != nil { + return nil + } return r.volumes[path] } @@ -129,6 +133,10 @@ func (r *Repository) remove(volume *Volume) { func (r *Repository) Delete(path string) error { r.lock.Lock() defer r.lock.Unlock() + path, err := filepath.EvalSymlinks(path) + if err != nil { + return err + } volume := r.get(path) if volume == nil { return fmt.Errorf("Volume %s does not exist", path)