Propagate unmount events to the external volume drivers.

Signed-off-by: David Calavera <david.calavera@gmail.com>

daemon/container.go

@@ -346,6 +346,8 @@ func (container *Container) cleanup() {
 	for _, eConfig := range container.execCommands.s {
 		container.daemon.unregisterExecCommand(eConfig)
 	}
+
+	container.UnmountVolumes(true)
 }
 
 func (container *Container) KillSig(sig int) error {
@@ -469,6 +471,7 @@ func (container *Container) Stop(seconds int) error {
 			return err
 		}
 	}
+
 	return nil
 }
 
@@ -564,16 +567,10 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) {
 	if err := container.Mount(); err != nil {
 		return nil, err
 	}
-	var paths []string
-	unmount := func() {
-		for _, p := range paths {
-			syscall.Unmount(p, 0)
-		}
-	}
 	defer func() {
 		if err != nil {
 			// unmount any volumes
-			unmount()
+			container.UnmountVolumes(true)
 			// unmount the container's rootfs
 			container.Unmount()
 		}
@@ -587,7 +584,6 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) {
 		if err != nil {
 			return nil, err
 		}
-		paths = append(paths, dest)
 		if err := mount.Mount(m.Source, dest, "bind", "rbind,ro"); err != nil {
 			return nil, err
 		}
@@ -618,7 +614,7 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) {
 	}
 	return ioutils.NewReadCloserWrapper(archive, func() error {
 			err := archive.Close()
-			unmount()
+			container.UnmountVolumes(true)
 			container.Unmount()
 			return err
 		}),
@@ -1090,3 +1086,48 @@ func (container *Container) shouldRestart() bool {
 	return container.hostConfig.RestartPolicy.Name == "always" ||
 		(container.hostConfig.RestartPolicy.Name == "on-failure" && container.ExitCode != 0)
 }
+
+func (container *Container) UnmountVolumes(forceSyscall bool) error {
+	for _, m := range container.MountPoints {
+		dest, err := container.GetResourcePath(m.Destination)
+		if err != nil {
+			return err
+		}
+
+		if forceSyscall {
+			syscall.Unmount(dest, 0)
+		}
+
+		if m.Volume != nil {
+			if err := m.Volume.Unmount(); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func (container *Container) copyImagePathContent(v volume.Volume, destination string) error {
+	rootfs, err := symlink.FollowSymlinkInScope(filepath.Join(container.basefs, destination), container.basefs)
+	if err != nil {
+		return err
+	}
+
+	if _, err = ioutil.ReadDir(rootfs); err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return err
+	}
+
+	path, err := v.Mount()
+	if err != nil {
+		return err
+	}
+
+	if err := copyExistingContents(rootfs, path); err != nil {
+		return err
+	}
+
+	return v.Unmount()
+}

daemon/create.go

@@ -10,7 +10,6 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/docker/pkg/stringid"
-	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/runconfig"
 	"github.com/docker/libcontainer/label"
 )
@@ -120,21 +119,20 @@ func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.Hos
 		if err != nil {
 			return nil, nil, err
 		}
-		if stat, err := os.Stat(path); err == nil && !stat.IsDir() {
+
+		stat, err := os.Stat(path)
+		if err == nil && !stat.IsDir() {
 			return nil, nil, fmt.Errorf("cannot mount volume over existing file, file exists %s", path)
 		}
+
 		v, err := createVolume(name, config.VolumeDriver)
 		if err != nil {
 			return nil, nil, err
 		}
-		rootfs, err := symlink.FollowSymlinkInScope(filepath.Join(container.basefs, destination), container.basefs)
-		if err != nil {
-			return nil, nil, err
-		}
-		if path, err = v.Mount(); err != nil {
+
+		if err := container.copyImagePathContent(v, destination); err != nil {
 			return nil, nil, err
 		}
-		copyExistingContents(rootfs, path)
 
 		container.addMountPointWithVolume(destination, v, true)
 	}

daemon/delete.go

@@ -70,6 +70,7 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error
 			}
 		}
 		container.LogEvent("destroy")
+
 		if config.RemoveVolume {
 			container.removeMountPoints()
 		}

integration-cli/docker_cli_run_test.go

@@ -440,14 +440,6 @@ func (s *DockerSuite) TestRunCreateVolumesInSymlinkDir(c *check.C) {
 	}
 }
 
-// Regression test for #4830
-func (s *DockerSuite) TestRunWithRelativePath(c *check.C) {
-	runCmd := exec.Command(dockerBinary, "run", "-v", "tmp:/other-tmp", "busybox", "true")
-	if _, _, _, err := runCommandWithStdoutStderr(runCmd); err == nil {
-		c.Fatalf("relative path should result in an error")
-	}
-}
-
 func (s *DockerSuite) TestRunVolumesMountedAsReadonly(c *check.C) {
 	cmd := exec.Command(dockerBinary, "run", "-v", "/test:/test:ro", "busybox", "touch", "/test/somefile")
 	if code, err := runCommand(cmd); err == nil || code == 0 {

integration-cli/docker_cli_start_volume_driver_unix_test.go

@@ -10,7 +10,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 
@@ -18,25 +17,40 @@ import (
 )
 
 func init() {
-	check.Suite(&ExternalVolumeSuite{
+	check.Suite(&DockerExternalVolumeSuite{
 		ds: &DockerSuite{},
 	})
 }
 
-type ExternalVolumeSuite struct {
+type eventCounter struct {
+	activations int
+	creations   int
+	removals    int
+	mounts      int
+	unmounts    int
+	paths       int
+}
+
+type DockerExternalVolumeSuite struct {
 	server *httptest.Server
 	ds     *DockerSuite
+	d      *Daemon
+	ec     *eventCounter
 }
 
-func (s *ExternalVolumeSuite) SetUpTest(c *check.C) {
+func (s *DockerExternalVolumeSuite) SetUpTest(c *check.C) {
+	s.d = NewDaemon(c)
 	s.ds.SetUpTest(c)
+	s.ec = &eventCounter{}
+
 }
 
-func (s *ExternalVolumeSuite) TearDownTest(c *check.C) {
+func (s *DockerExternalVolumeSuite) TearDownTest(c *check.C) {
+	s.d.Stop()
 	s.ds.TearDownTest(c)
 }
 
-func (s *ExternalVolumeSuite) SetUpSuite(c *check.C) {
+func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) {
 	mux := http.NewServeMux()
 	s.server = httptest.NewServer(mux)
 
@@ -44,26 +58,30 @@ func (s *ExternalVolumeSuite) SetUpSuite(c *check.C) {
 		name string
 	}
 
-	hostVolumePath := func(name string) string {
-		return fmt.Sprintf("/var/lib/docker/volumes/%s", name)
-	}
-
 	mux.HandleFunc("/Plugin.Activate", func(w http.ResponseWriter, r *http.Request) {
+		s.ec.activations++
+
 		w.Header().Set("Content-Type", "appplication/vnd.docker.plugins.v1+json")
 		fmt.Fprintln(w, `{"Implements": ["VolumeDriver"]}`)
 	})
 
 	mux.HandleFunc("/VolumeDriver.Create", func(w http.ResponseWriter, r *http.Request) {
+		s.ec.creations++
+
 		w.Header().Set("Content-Type", "appplication/vnd.docker.plugins.v1+json")
 		fmt.Fprintln(w, `{}`)
 	})
 
 	mux.HandleFunc("/VolumeDriver.Remove", func(w http.ResponseWriter, r *http.Request) {
+		s.ec.removals++
+
 		w.Header().Set("Content-Type", "appplication/vnd.docker.plugins.v1+json")
 		fmt.Fprintln(w, `{}`)
 	})
 
 	mux.HandleFunc("/VolumeDriver.Path", func(w http.ResponseWriter, r *http.Request) {
+		s.ec.paths++
+
 		var pr pluginRequest
 		if err := json.NewDecoder(r.Body).Decode(&pr); err != nil {
 			http.Error(w, err.Error(), 500)
@@ -76,6 +94,8 @@ func (s *ExternalVolumeSuite) SetUpSuite(c *check.C) {
 	})
 
 	mux.HandleFunc("/VolumeDriver.Mount", func(w http.ResponseWriter, r *http.Request) {
+		s.ec.mounts++
+
 		var pr pluginRequest
 		if err := json.NewDecoder(r.Body).Decode(&pr); err != nil {
 			http.Error(w, err.Error(), 500)
@@ -94,7 +114,9 @@ func (s *ExternalVolumeSuite) SetUpSuite(c *check.C) {
 		fmt.Fprintln(w, fmt.Sprintf("{\"Mountpoint\": \"%s\"}", p))
 	})
 
-	mux.HandleFunc("/VolumeDriver.Umount", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/VolumeDriver.Unmount", func(w http.ResponseWriter, r *http.Request) {
+		s.ec.unmounts++
+
 		var pr pluginRequest
 		if err := json.NewDecoder(r.Body).Decode(&pr); err != nil {
 			http.Error(w, err.Error(), 500)
@@ -118,7 +140,7 @@ func (s *ExternalVolumeSuite) SetUpSuite(c *check.C) {
 	}
 }
 
-func (s *ExternalVolumeSuite) TearDownSuite(c *check.C) {
+func (s *DockerExternalVolumeSuite) TearDownSuite(c *check.C) {
 	s.server.Close()
 
 	if err := os.RemoveAll("/usr/share/docker/plugins"); err != nil {
@@ -126,14 +148,102 @@ func (s *ExternalVolumeSuite) TearDownSuite(c *check.C) {
 	}
 }
 
-func (s *ExternalVolumeSuite) TestStartExternalVolumeDriver(c *check.C) {
-	runCmd := exec.Command(dockerBinary, "run", "--name", "test-data", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", "busybox:latest", "cat", "/tmp/external-volume-test/test")
-	out, stderr, exitCode, err := runCommandWithStdoutStderr(runCmd)
-	if err != nil && exitCode != 0 {
-		c.Fatal(out, stderr, err)
+func (s *DockerExternalVolumeSuite) TestStartExternalNamedVolumeDriver(c *check.C) {
+	if err := s.d.StartWithBusybox(); err != nil {
+		c.Fatal(err)
+	}
+
+	out, err := s.d.Cmd("run", "--rm", "--name", "test-data", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", "busybox:latest", "cat", "/tmp/external-volume-test/test")
+	if err != nil {
+		c.Fatal(err)
 	}
 
 	if !strings.Contains(out, s.server.URL) {
 		c.Fatalf("External volume mount failed. Output: %s\n", out)
 	}
+
+	p := hostVolumePath("external-volume-test")
+	_, err = os.Lstat(p)
+	if err == nil {
+		c.Fatalf("Expected error checking volume path in host: %s\n", p)
+	}
+
+	if !os.IsNotExist(err) {
+		c.Fatalf("Expected volume path in host to not exist: %s, %v\n", p, err)
+	}
+
+	c.Assert(s.ec.activations, check.Equals, 1)
+	c.Assert(s.ec.creations, check.Equals, 1)
+	c.Assert(s.ec.removals, check.Equals, 1)
+	c.Assert(s.ec.mounts, check.Equals, 1)
+	c.Assert(s.ec.unmounts, check.Equals, 1)
+}
+
+func (s *DockerExternalVolumeSuite) TestStartExternalVolumeUnnamedDriver(c *check.C) {
+	if err := s.d.StartWithBusybox(); err != nil {
+		c.Fatal(err)
+	}
+
+	out, err := s.d.Cmd("run", "--rm", "--name", "test-data", "-v", "/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", "busybox:latest", "cat", "/tmp/external-volume-test/test")
+	if err != nil {
+		c.Fatal(err)
+	}
+
+	if !strings.Contains(out, s.server.URL) {
+		c.Fatalf("External volume mount failed. Output: %s\n", out)
+	}
+
+	c.Assert(s.ec.activations, check.Equals, 1)
+	c.Assert(s.ec.creations, check.Equals, 1)
+	c.Assert(s.ec.removals, check.Equals, 1)
+	c.Assert(s.ec.mounts, check.Equals, 1)
+	c.Assert(s.ec.unmounts, check.Equals, 1)
+}
+
+func (s DockerExternalVolumeSuite) TestStartExternalVolumeDriverVolumesFrom(c *check.C) {
+	if err := s.d.StartWithBusybox(); err != nil {
+		c.Fatal(err)
+	}
+
+	if _, err := s.d.Cmd("run", "-d", "--name", "vol-test1", "-v", "/foo", "--volume-driver", "test-external-volume-driver", "busybox:latest"); err != nil {
+		c.Fatal(err)
+	}
+
+	if _, err := s.d.Cmd("run", "--rm", "--volumes-from", "vol-test1", "--name", "vol-test2", "busybox", "ls", "/tmp"); err != nil {
+		c.Fatal(err)
+	}
+
+	if _, err := s.d.Cmd("rm", "-f", "vol-test1"); err != nil {
+		c.Fatal(err)
+	}
+
+	c.Assert(s.ec.activations, check.Equals, 1)
+	c.Assert(s.ec.creations, check.Equals, 2)
+	c.Assert(s.ec.removals, check.Equals, 1)
+	c.Assert(s.ec.mounts, check.Equals, 2)
+	c.Assert(s.ec.unmounts, check.Equals, 2)
+}
+
+func (s DockerExternalVolumeSuite) TestStartExternalVolumeDriverDeleteContainer(c *check.C) {
+	if err := s.d.StartWithBusybox(); err != nil {
+		c.Fatal(err)
+	}
+
+	if _, err := s.d.Cmd("run", "-d", "--name", "vol-test1", "-v", "/foo", "--volume-driver", "test-external-volume-driver", "busybox:latest"); err != nil {
+		c.Fatal(err)
+	}
+
+	if _, err := s.d.Cmd("rm", "-fv", "vol-test1"); err != nil {
+		c.Fatal(err)
+	}
+
+	c.Assert(s.ec.activations, check.Equals, 1)
+	c.Assert(s.ec.creations, check.Equals, 1)
+	c.Assert(s.ec.removals, check.Equals, 1)
+	c.Assert(s.ec.mounts, check.Equals, 1)
+	c.Assert(s.ec.unmounts, check.Equals, 1)
+}
+
+func hostVolumePath(name string) string {
+	return fmt.Sprintf("/var/lib/docker/volumes/%s", name)
 }

volume/drivers/adapter.go

@@ -16,7 +16,10 @@ func (a *volumeDriverAdapter) Create(name string) (volume.Volume, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &volumeAdapter{a.proxy, name, a.name}, nil
+	return &volumeAdapter{
+		proxy:      a.proxy,
+		name:       name,
+		driverName: a.name}, nil
 }
 
 func (a *volumeDriverAdapter) Remove(v volume.Volume) error {
@@ -27,6 +30,7 @@ type volumeAdapter struct {
 	proxy      *volumeDriverProxy
 	name       string
 	driverName string
+	eMount     string // ephemeral host volume path
 }
 
 func (a *volumeAdapter) Name() string {
@@ -38,12 +42,17 @@ func (a *volumeAdapter) DriverName() string {
 }
 
 func (a *volumeAdapter) Path() string {
+	if len(a.eMount) > 0 {
+		return a.eMount
+	}
 	m, _ := a.proxy.Path(a.name)
 	return m
 }
 
 func (a *volumeAdapter) Mount() (string, error) {
-	return a.proxy.Mount(a.name)
+	var err error
+	a.eMount, err = a.proxy.Mount(a.name)
+	return a.eMount, err
 }
 
 func (a *volumeAdapter) Unmount() error {

volume/drivers/extpoint.go

@@ -1,9 +1,9 @@
 package volumedrivers
 
 import (
+	"fmt"
 	"sync"
 
-	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/plugins"
 	"github.com/docker/docker/volume"
 )
@@ -52,9 +52,9 @@ func Lookup(name string) (volume.Driver, error) {
 	}
 	pl, err := plugins.Get(name, "VolumeDriver")
 	if err != nil {
-		logrus.Errorf("Error: %v", err)
-		return nil, err
+		return nil, fmt.Errorf("Error looking up volume plugin %s: %v", name, err)
 	}
+
 	d := NewVolumeDriver(name, pl.Client)
 	drivers.extensions[name] = d
 	return d, nil

volume/drivers/proxy.go

@@ -1,5 +1,7 @@
 package volumedrivers
 
+import "fmt"
+
 // currently created by hand. generation tool would generate this like:
 // $ rpc-gen volume/drivers/api.go VolumeDriver > volume/drivers/proxy.go
 
@@ -21,9 +23,9 @@ func (pp *volumeDriverProxy) Create(name string) error {
 	var ret volumeDriverResponse
 	err := pp.c.Call("VolumeDriver.Create", args, &ret)
 	if err != nil {
-		return err
+		return pp.fmtError(name, err)
 	}
-	return ret.Err
+	return pp.fmtError(name, ret.Err)
 }
 
 func (pp *volumeDriverProxy) Remove(name string) error {
@@ -31,27 +33,27 @@ func (pp *volumeDriverProxy) Remove(name string) error {
 	var ret volumeDriverResponse
 	err := pp.c.Call("VolumeDriver.Remove", args, &ret)
 	if err != nil {
-		return err
+		return pp.fmtError(name, err)
 	}
-	return ret.Err
+	return pp.fmtError(name, ret.Err)
 }
 
 func (pp *volumeDriverProxy) Path(name string) (string, error) {
 	args := volumeDriverRequest{name}
 	var ret volumeDriverResponse
 	if err := pp.c.Call("VolumeDriver.Path", args, &ret); err != nil {
-		return "", err
+		return "", pp.fmtError(name, err)
 	}
-	return ret.Mountpoint, ret.Err
+	return ret.Mountpoint, pp.fmtError(name, ret.Err)
 }
 
 func (pp *volumeDriverProxy) Mount(name string) (string, error) {
 	args := volumeDriverRequest{name}
 	var ret volumeDriverResponse
 	if err := pp.c.Call("VolumeDriver.Mount", args, &ret); err != nil {
-		return "", err
+		return "", pp.fmtError(name, err)
 	}
-	return ret.Mountpoint, ret.Err
+	return ret.Mountpoint, pp.fmtError(name, ret.Err)
 }
 
 func (pp *volumeDriverProxy) Unmount(name string) error {
@@ -59,7 +61,14 @@ func (pp *volumeDriverProxy) Unmount(name string) error {
 	var ret volumeDriverResponse
 	err := pp.c.Call("VolumeDriver.Unmount", args, &ret)
 	if err != nil {
-		return err
+		return pp.fmtError(name, err)
+	}
+	return pp.fmtError(name, ret.Err)
+}
+
+func (pp *volumeDriverProxy) fmtError(name string, err error) error {
+	if err == nil {
+		return nil
 	}
-	return ret.Err
+	return fmt.Errorf("External volume driver request failed for %s: %v", name, err)
 }