From d592778f4a75d36745aaffaf73c0775ecd420545 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Fri, 22 May 2015 10:37:00 -0700 Subject: [PATCH] Propagate unmount events to the external volume drivers. Signed-off-by: David Calavera --- daemon/container.go | 59 +++++-- daemon/create.go | 14 +- daemon/delete.go | 1 + integration-cli/docker_cli_run_test.go | 8 - ...ocker_cli_start_volume_driver_unix_test.go | 144 +++++++++++++++--- volume/drivers/adapter.go | 13 +- volume/drivers/extpoint.go | 6 +- volume/drivers/proxy.go | 29 ++-- 8 files changed, 217 insertions(+), 57 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index 468dc23044..1f65386fa9 100644 --- a/daemon/container.go +++ b/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() +} diff --git a/daemon/create.go b/daemon/create.go index 0020226dd3..5d67ed55f1 100644 --- a/daemon/create.go +++ b/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 { + + if err := container.copyImagePathContent(v, destination); err != nil { return nil, nil, err } - if path, err = v.Mount(); err != nil { - return nil, nil, err - } - copyExistingContents(rootfs, path) container.addMountPointWithVolume(destination, v, true) } diff --git a/daemon/delete.go b/daemon/delete.go index 02e6c9814e..b323f1b1db 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -70,6 +70,7 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error } } container.LogEvent("destroy") + if config.RemoveVolume { container.removeMountPoints() } diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 7e60675045..821ea4a995 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/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 { diff --git a/integration-cli/docker_cli_start_volume_driver_unix_test.go b/integration-cli/docker_cli_start_volume_driver_unix_test.go index 5b4223dd8a..1cc90801d6 100644 --- a/integration-cli/docker_cli_start_volume_driver_unix_test.go +++ b/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) } diff --git a/volume/drivers/adapter.go b/volume/drivers/adapter.go index e849fb8083..6846a3a8e8 100644 --- a/volume/drivers/adapter.go +++ b/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 { diff --git a/volume/drivers/extpoint.go b/volume/drivers/extpoint.go index 0e90843517..b002a0ffda 100644 --- a/volume/drivers/extpoint.go +++ b/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 diff --git a/volume/drivers/proxy.go b/volume/drivers/proxy.go index 1bc1586791..545e49007a 100644 --- a/volume/drivers/proxy.go +++ b/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 ret.Err + return pp.fmtError(name, ret.Err) +} + +func (pp *volumeDriverProxy) fmtError(name string, err error) error { + if err == nil { + return nil + } + return fmt.Errorf("External volume driver request failed for %s: %v", name, err) }