From 2b6bc294fc7f9e08a9091833b021b7d2a01ad2a6 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 7 Mar 2016 21:41:44 -0500 Subject: [PATCH] When calling volume driver Mount, send opaque ID This generates an ID string for calls to Mount/Unmount, allowing drivers to differentiate between two callers of `Mount` and `Unmount`. Signed-off-by: Brian Goff --- container/container_unix.go | 14 +++++++++++--- docs/extend/plugins_volume.md | 10 ++++++++-- ...docker_cli_external_volume_driver_unix_test.go | 15 +++++++++++++++ pkg/plugins/pluginrpc-gen/template.go | 9 ++++++++- volume/drivers/adapter.go | 8 ++++---- volume/drivers/extpoint.go | 4 ++-- volume/drivers/proxy.go | 8 ++++++-- volume/drivers/proxy_test.go | 4 ++-- volume/local/local.go | 4 ++-- volume/local/local_test.go | 8 ++++---- volume/testutils/testutils.go | 8 ++++---- volume/volume.go | 14 +++++++++++--- 12 files changed, 77 insertions(+), 29 deletions(-) diff --git a/container/container_unix.go b/container/container_unix.go index 8273bdb025..5314d7e382 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -12,6 +12,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/chrootarchive" + "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/docker/docker/utils" @@ -181,11 +182,17 @@ func (container *Container) CopyImagePathContent(v volume.Volume, destination st return err } - path, err := v.Mount() + id := stringid.GenerateNonCryptoID() + path, err := v.Mount(id) if err != nil { return err } - defer v.Unmount() + + defer func() { + if err := v.Unmount(id); err != nil { + logrus.Warnf("error while unmounting volume %s: %v", v.Name(), err) + } + }() return copyExistingContents(rootfs, path) } @@ -328,9 +335,10 @@ func (container *Container) UnmountVolumes(forceSyscall bool, volumeEventLog fun } if volumeMount.Volume != nil { - if err := volumeMount.Volume.Unmount(); err != nil { + if err := volumeMount.Volume.Unmount(volumeMount.ID); err != nil { return err } + volumeMount.ID = "" attributes := map[string]string{ "driver": volumeMount.Volume.DriverName(), diff --git a/docs/extend/plugins_volume.md b/docs/extend/plugins_volume.md index 88798497c3..5c2ebb49bc 100644 --- a/docs/extend/plugins_volume.md +++ b/docs/extend/plugins_volume.md @@ -115,7 +115,8 @@ Respond with a string error if an error occurred. **Request**: ```json { - "Name": "volume_name" + "Name": "volume_name", + "ID": "b87d7442095999a92b65b3d9691e697b61713829cc0ffd1bb72e4ccd51aa4d6c" } ``` @@ -124,6 +125,8 @@ name. This is called once per container start. If the same volume_name is reques more than once, the plugin may need to keep track of each new mount request and provision at the first mount request and deprovision at the last corresponding unmount request. +`ID` is a unqiue ID for the caller that is requesting the mount. + **Response**: ```json { @@ -162,7 +165,8 @@ available, and/or a string error if an error occurred. **Request**: ```json { - "Name": "volume_name" + "Name": "volume_name", + "ID": "b87d7442095999a92b65b3d9691e697b61713829cc0ffd1bb72e4ccd51aa4d6c" } ``` @@ -170,6 +174,8 @@ Indication that Docker no longer is using the named volume. This is called once per container stop. Plugin may deduce that it is safe to deprovision it at this point. +`ID` is a unqiue ID for the caller that is requesting the mount. + **Response**: ```json { diff --git a/integration-cli/docker_cli_external_volume_driver_unix_test.go b/integration-cli/docker_cli_external_volume_driver_unix_test.go index 730cf59b34..e6c6d9a843 100644 --- a/integration-cli/docker_cli_external_volume_driver_unix_test.go +++ b/integration-cli/docker_cli_external_volume_driver_unix_test.go @@ -61,6 +61,7 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) { type pluginRequest struct { Name string Opts map[string]string + ID string } type pluginResp struct { @@ -204,6 +205,11 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) { return } + if err := ioutil.WriteFile(filepath.Join(p, "mountID"), []byte(pr.ID), 0644); err != nil { + send(w, err) + return + } + send(w, &pluginResp{Mountpoint: p}) }) @@ -476,3 +482,12 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverPathCalls(c *check.C c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") c.Assert(s.ec.paths, checker.Equals, 1) } + +func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverMountID(c *check.C) { + err := s.d.StartWithBusybox() + c.Assert(err, checker.IsNil) + + out, err := s.d.Cmd("run", "--rm", "-v", "external-volume-test:/tmp/external-volume-test", "--volume-driver", "test-external-volume-driver", "busybox:latest", "cat", "/tmp/external-volume-test/test") + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") +} diff --git a/pkg/plugins/pluginrpc-gen/template.go b/pkg/plugins/pluginrpc-gen/template.go index 704030cf0f..d3dc494dd4 100644 --- a/pkg/plugins/pluginrpc-gen/template.go +++ b/pkg/plugins/pluginrpc-gen/template.go @@ -42,10 +42,17 @@ var templFuncs = template.FuncMap{ "marshalType": marshalType, "isErr": isErr, "lower": strings.ToLower, - "title": strings.Title, + "title": title, "tag": buildTag, } +func title(s string) string { + if strings.ToLower(s) == "id" { + return "ID" + } + return strings.Title(s) +} + var generatedTempl = template.Must(template.New("rpc_cient").Funcs(templFuncs).Parse(` // generated code - DO NOT EDIT {{ range $k, $v := .BuildTags }} diff --git a/volume/drivers/adapter.go b/volume/drivers/adapter.go index fcbf01f5d6..1593d01669 100644 --- a/volume/drivers/adapter.go +++ b/volume/drivers/adapter.go @@ -101,14 +101,14 @@ func (a *volumeAdapter) CachedPath() string { return a.eMount } -func (a *volumeAdapter) Mount() (string, error) { +func (a *volumeAdapter) Mount(id string) (string, error) { var err error - a.eMount, err = a.proxy.Mount(a.name) + a.eMount, err = a.proxy.Mount(a.name, id) return a.eMount, err } -func (a *volumeAdapter) Unmount() error { - err := a.proxy.Unmount(a.name) +func (a *volumeAdapter) Unmount(id string) error { + err := a.proxy.Unmount(a.name, id) if err == nil { a.eMount = "" } diff --git a/volume/drivers/extpoint.go b/volume/drivers/extpoint.go index a55da5aff9..c792f5e31d 100644 --- a/volume/drivers/extpoint.go +++ b/volume/drivers/extpoint.go @@ -38,9 +38,9 @@ type volumeDriver interface { // Get the mountpoint of the given volume Path(name string) (mountpoint string, err error) // Mount the given volume and return the mountpoint - Mount(name string) (mountpoint string, err error) + Mount(name, id string) (mountpoint string, err error) // Unmount the given volume - Unmount(name string) (err error) + Unmount(name, id string) (err error) // List lists all the volumes known to the driver List() (volumes list, err error) // Get retrieves the volume with the requested name diff --git a/volume/drivers/proxy.go b/volume/drivers/proxy.go index 5c7cdcb7c7..f55e0dbeaf 100644 --- a/volume/drivers/proxy.go +++ b/volume/drivers/proxy.go @@ -97,6 +97,7 @@ func (pp *volumeDriverProxy) Path(name string) (mountpoint string, err error) { type volumeDriverProxyMountRequest struct { Name string + ID string } type volumeDriverProxyMountResponse struct { @@ -104,13 +105,14 @@ type volumeDriverProxyMountResponse struct { Err string } -func (pp *volumeDriverProxy) Mount(name string) (mountpoint string, err error) { +func (pp *volumeDriverProxy) Mount(name string, id string) (mountpoint string, err error) { var ( req volumeDriverProxyMountRequest ret volumeDriverProxyMountResponse ) req.Name = name + req.ID = id if err = pp.Call("VolumeDriver.Mount", req, &ret); err != nil { return } @@ -126,19 +128,21 @@ func (pp *volumeDriverProxy) Mount(name string) (mountpoint string, err error) { type volumeDriverProxyUnmountRequest struct { Name string + ID string } type volumeDriverProxyUnmountResponse struct { Err string } -func (pp *volumeDriverProxy) Unmount(name string) (err error) { +func (pp *volumeDriverProxy) Unmount(name string, id string) (err error) { var ( req volumeDriverProxyUnmountRequest ret volumeDriverProxyUnmountResponse ) req.Name = name + req.ID = id if err = pp.Call("VolumeDriver.Unmount", req, &ret); err != nil { return } diff --git a/volume/drivers/proxy_test.go b/volume/drivers/proxy_test.go index 6b26f9dc5b..62a217baa9 100644 --- a/volume/drivers/proxy_test.go +++ b/volume/drivers/proxy_test.go @@ -68,7 +68,7 @@ func TestVolumeRequestError(t *testing.T) { t.Fatalf("Unexpected error: %v\n", err) } - _, err = driver.Mount("volume") + _, err = driver.Mount("volume", "123") if err == nil { t.Fatal("Expected error, was nil") } @@ -77,7 +77,7 @@ func TestVolumeRequestError(t *testing.T) { t.Fatalf("Unexpected error: %v\n", err) } - err = driver.Unmount("volume") + err = driver.Unmount("volume", "123") if err == nil { t.Fatal("Expected error, was nil") } diff --git a/volume/local/local.go b/volume/local/local.go index 0c615deb81..b0046b6f55 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -287,7 +287,7 @@ func (v *localVolume) Path() string { } // Mount implements the localVolume interface, returning the data location. -func (v *localVolume) Mount() (string, error) { +func (v *localVolume) Mount(id string) (string, error) { v.m.Lock() defer v.m.Unlock() if v.opts != nil { @@ -303,7 +303,7 @@ func (v *localVolume) Mount() (string, error) { } // Umount is for satisfying the localVolume interface and does not do anything in this driver. -func (v *localVolume) Unmount() error { +func (v *localVolume) Unmount(id string) error { v.m.Lock() defer v.m.Unlock() if v.opts != nil { diff --git a/volume/local/local_test.go b/volume/local/local_test.go index 1baa085457..fd8c4f8184 100644 --- a/volume/local/local_test.go +++ b/volume/local/local_test.go @@ -181,12 +181,12 @@ func TestCreateWithOpts(t *testing.T) { } v := vol.(*localVolume) - dir, err := v.Mount() + dir, err := v.Mount("1234") if err != nil { t.Fatal(err) } defer func() { - if err := v.Unmount(); err != nil { + if err := v.Unmount("1234"); err != nil { t.Fatal(err) } }() @@ -225,14 +225,14 @@ func TestCreateWithOpts(t *testing.T) { } // test double mount - if _, err := v.Mount(); err != nil { + if _, err := v.Mount("1234"); err != nil { t.Fatal(err) } if v.active.count != 2 { t.Fatalf("Expected active mount count to be 2, got %d", v.active.count) } - if err := v.Unmount(); err != nil { + if err := v.Unmount("1234"); err != nil { t.Fatal(err) } if v.active.count != 1 { diff --git a/volume/testutils/testutils.go b/volume/testutils/testutils.go index 96d89258d7..91cdcded00 100644 --- a/volume/testutils/testutils.go +++ b/volume/testutils/testutils.go @@ -19,10 +19,10 @@ func (NoopVolume) DriverName() string { return "noop" } func (NoopVolume) Path() string { return "noop" } // Mount mounts the volume in the container -func (NoopVolume) Mount() (string, error) { return "noop", nil } +func (NoopVolume) Mount(_ string) (string, error) { return "noop", nil } // Unmount unmounts the volume from the container -func (NoopVolume) Unmount() error { return nil } +func (NoopVolume) Unmount(_ string) error { return nil } // Status proivdes low-level details about the volume func (NoopVolume) Status() map[string]interface{} { return nil } @@ -48,10 +48,10 @@ func (f FakeVolume) DriverName() string { return f.driverName } func (FakeVolume) Path() string { return "fake" } // Mount mounts the volume in the container -func (FakeVolume) Mount() (string, error) { return "fake", nil } +func (FakeVolume) Mount(_ string) (string, error) { return "fake", nil } // Unmount unmounts the volume from the container -func (FakeVolume) Unmount() error { return nil } +func (FakeVolume) Unmount(_ string) error { return nil } // Status proivdes low-level details about the volume func (FakeVolume) Status() map[string]interface{} { return nil } diff --git a/volume/volume.go b/volume/volume.go index db0105556a..f288ce97da 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -5,6 +5,8 @@ import ( "os" "runtime" "strings" + + "github.com/docker/docker/pkg/stringid" ) // DefaultDriverName is the driver name used for the driver @@ -35,9 +37,9 @@ type Volume interface { Path() string // Mount mounts the volume and returns the absolute path to // where it can be consumed. - Mount() (string, error) + Mount(id string) (string, error) // Unmount unmounts the volume when it is no longer in use. - Unmount() error + Unmount(id string) error // Status returns low-level status information about a volume Status() map[string]interface{} } @@ -64,13 +66,19 @@ type MountPoint struct { // Use a pointer here so we can tell if the user set this value explicitly // This allows us to error out when the user explicitly enabled copy but we can't copy due to the volume being populated CopyData bool `json:"-"` + // ID is the opaque ID used to pass to the volume driver. + // This should be set by calls to `Mount` and unset by calls to `Unmount` + ID string } // Setup sets up a mount point by either mounting the volume if it is // configured, or creating the source directory if supplied. func (m *MountPoint) Setup() (string, error) { if m.Volume != nil { - return m.Volume.Mount() + if m.ID == "" { + m.ID = stringid.GenerateNonCryptoID() + } + return m.Volume.Mount(m.ID) } if len(m.Source) > 0 { if _, err := os.Stat(m.Source); err != nil {