diff --git a/container/container_unix.go b/container/container_unix.go index 8273bdb02..5314d7e38 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 88798497c..5c2ebb49b 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 730cf59b3..e6c6d9a84 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 704030cf0..d3dc494dd 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 fcbf01f5d..1593d0166 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 a55da5aff..c792f5e31 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 5c7cdcb7c..f55e0dbea 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 6b26f9dc5..62a217baa 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 0c615deb8..b0046b6f5 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 1baa08545..fd8c4f818 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 96d89258d..91cdcded0 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 db0105556..f288ce97d 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 {