From 3bf0ca31cf61e445554326ba405cc32c7ef64189 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Thu, 21 May 2015 10:57:59 -0700 Subject: [PATCH] Allow named volumes for external drivers. Signed-off-by: David Calavera --- daemon/volumes.go | 20 ++++++-- daemon/volumes_experimental.go | 11 +++++ daemon/volumes_experimental_unit_test.go | 54 +++++++++++++++++++++ daemon/volumes_stubs.go | 12 +++++ daemon/volumes_stubs_unit_test.go | 46 ++++++++++++++++++ daemon/volumes_unit_test.go | 62 +----------------------- 6 files changed, 140 insertions(+), 65 deletions(-) diff --git a/daemon/volumes.go b/daemon/volumes.go index bc96b345ff..2fdcd8311d 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -70,11 +70,21 @@ func parseBindMount(spec string, config *runconfig.Config) (*mountPoint, error) return nil, fmt.Errorf("Invalid volume specification: %s", spec) } - if !filepath.IsAbs(arr[0]) { - return nil, fmt.Errorf("cannot bind mount volume: %s volume paths must be absolute.", spec) + name, source, err := parseVolumeSource(arr[0], config) + if err != nil { + return nil, err } - bind.Source = filepath.Clean(arr[0]) + if len(source) == 0 { + bind.Driver = config.VolumeDriver + if len(bind.Driver) == 0 { + bind.Driver = volume.DefaultDriverName + } + } else { + bind.Source = filepath.Clean(source) + } + + bind.Name = name bind.Destination = filepath.Clean(bind.Destination) return bind, nil } @@ -245,7 +255,8 @@ func (daemon *Daemon) verifyOldVolumesInfo(container *Container) error { if strings.HasPrefix(hostPath, vfsPath) { id := filepath.Base(hostPath) - container.addLocalMountPoint(id, destination, vols.VolumesRW[destination]) + rw := vols.VolumesRW != nil && vols.VolumesRW[destination] + container.addLocalMountPoint(id, destination, rw) } } @@ -254,6 +265,7 @@ func (daemon *Daemon) verifyOldVolumesInfo(container *Container) error { func createVolume(name, driverName string) (volume.Volume, error) { vd, err := getVolumeDriver(driverName) + if err != nil { return nil, err } diff --git a/daemon/volumes_experimental.go b/daemon/volumes_experimental.go index a4f5a9cbe8..c39b7907b0 100644 --- a/daemon/volumes_experimental.go +++ b/daemon/volumes_experimental.go @@ -3,6 +3,9 @@ package daemon import ( + "path/filepath" + + "github.com/docker/docker/runconfig" "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" ) @@ -13,3 +16,11 @@ func getVolumeDriver(name string) (volume.Driver, error) { } return volumedrivers.Lookup(name) } + +func parseVolumeSource(spec string, config *runconfig.Config) (string, string, error) { + if !filepath.IsAbs(spec) { + return spec, "", nil + } + + return "", spec, nil +} diff --git a/daemon/volumes_experimental_unit_test.go b/daemon/volumes_experimental_unit_test.go index 89ef5e872d..1201f5154e 100644 --- a/daemon/volumes_experimental_unit_test.go +++ b/daemon/volumes_experimental_unit_test.go @@ -5,6 +5,7 @@ package daemon import ( "testing" + "github.com/docker/docker/runconfig" "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" ) @@ -30,3 +31,56 @@ func TestGetVolumeDriver(t *testing.T) { t.Fatalf("Expected fake driver, got %s\n", d.Name()) } } + +func TestParseBindMount(t *testing.T) { + cases := []struct { + bind string + driver string + expDest string + expSource string + expName string + expDriver string + expRW bool + fail bool + }{ + {"/tmp:/tmp", "", "/tmp", "/tmp", "", "", true, false}, + {"/tmp:/tmp:ro", "", "/tmp", "/tmp", "", "", false, false}, + {"/tmp:/tmp:rw", "", "/tmp", "/tmp", "", "", true, false}, + {"/tmp:/tmp:foo", "", "/tmp", "/tmp", "", "", false, true}, + {"name:/tmp", "", "/tmp", "", "name", "local", true, false}, + {"name:/tmp", "external", "/tmp", "", "name", "external", true, false}, + {"name:/tmp:ro", "local", "/tmp", "", "name", "local", false, false}, + {"local/name:/tmp:rw", "", "/tmp", "", "local/name", "local", true, false}, + } + + for _, c := range cases { + conf := &runconfig.Config{VolumeDriver: c.driver} + m, err := parseBindMount(c.bind, conf) + if c.fail { + if err == nil { + t.Fatalf("Expected error, was nil, for spec %s\n", c.bind) + } + continue + } + + if m.Destination != c.expDest { + t.Fatalf("Expected destination %s, was %s, for spec %s\n", c.expDest, m.Destination, c.bind) + } + + if m.Source != c.expSource { + t.Fatalf("Expected source %s, was %s, for spec %s\n", c.expSource, m.Source, c.bind) + } + + if m.Name != c.expName { + t.Fatalf("Expected name %s, was %s for spec %s\n", c.expName, m.Name, c.bind) + } + + if m.Driver != c.expDriver { + t.Fatalf("Expected driver %s, was %s, for spec %s\n", c.expDriver, m.Driver, c.bind) + } + + if m.RW != c.expRW { + t.Fatalf("Expected RW %v, was %v for spec %s\n", c.expRW, m.RW, c.bind) + } + } +} diff --git a/daemon/volumes_stubs.go b/daemon/volumes_stubs.go index 41f8b67343..1d2d873d8e 100644 --- a/daemon/volumes_stubs.go +++ b/daemon/volumes_stubs.go @@ -3,6 +3,10 @@ package daemon import ( + "fmt" + "path/filepath" + + "github.com/docker/docker/runconfig" "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" ) @@ -10,3 +14,11 @@ import ( func getVolumeDriver(_ string) (volume.Driver, error) { return volumedrivers.Lookup(volume.DefaultDriverName) } + +func parseVolumeSource(spec string, _ *runconfig.Config) (string, string, error) { + if !filepath.IsAbs(spec) { + return "", "", fmt.Errorf("cannot bind mount volume: %s volume paths must be absolute.", spec) + } + + return "", spec, nil +} diff --git a/daemon/volumes_stubs_unit_test.go b/daemon/volumes_stubs_unit_test.go index 655f5f4b79..a3cafe6550 100644 --- a/daemon/volumes_stubs_unit_test.go +++ b/daemon/volumes_stubs_unit_test.go @@ -7,6 +7,7 @@ import ( "os" "testing" + "github.com/docker/docker/runconfig" "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" "github.com/docker/docker/volume/local" @@ -33,3 +34,48 @@ func TestGetVolumeDefaultDriver(t *testing.T) { t.Fatalf("Expected local driver, was %s\n", d.Name) } } + +func TestParseBindMount(t *testing.T) { + cases := []struct { + bind string + expDest string + expSource string + expName string + expRW bool + fail bool + }{ + {"/tmp:/tmp", "/tmp", "/tmp", "", true, false}, + {"/tmp:/tmp:ro", "/tmp", "/tmp", "", false, false}, + {"/tmp:/tmp:rw", "/tmp", "/tmp", "", true, false}, + {"/tmp:/tmp:foo", "/tmp", "/tmp", "", false, true}, + {"name:/tmp", "", "", "", false, true}, + {"local/name:/tmp:rw", "", "", "", true, true}, + } + + for _, c := range cases { + conf := &runconfig.Config{} + m, err := parseBindMount(c.bind, conf) + if c.fail { + if err == nil { + t.Fatalf("Expected error, was nil, for spec %s\n", c.bind) + } + continue + } + + if m.Destination != c.expDest { + t.Fatalf("Expected destination %s, was %s, for spec %s\n", c.expDest, m.Destination, c.bind) + } + + if m.Source != c.expSource { + t.Fatalf("Expected source %s, was %s, for spec %s\n", c.expSource, m.Source, c.bind) + } + + if m.Name != c.expName { + t.Fatalf("Expected name %s, was %s for spec %s\n", c.expName, m.Name, c.bind) + } + + if m.RW != c.expRW { + t.Fatalf("Expected RW %v, was %v for spec %s\n", c.expRW, m.RW, c.bind) + } + } +} diff --git a/daemon/volumes_unit_test.go b/daemon/volumes_unit_test.go index 6578aa2043..b1e7f72f89 100644 --- a/daemon/volumes_unit_test.go +++ b/daemon/volumes_unit_test.go @@ -1,66 +1,6 @@ package daemon -import ( - "testing" - - "github.com/docker/docker/runconfig" -) - -func TestParseBindMount(t *testing.T) { - cases := []struct { - bind string - driver string - expDest string - expSource string - expName string - expDriver string - expRW bool - fail bool - }{ - {"/tmp:/tmp", "", "/tmp", "/tmp", "", "", true, false}, - {"/tmp:/tmp:ro", "", "/tmp", "/tmp", "", "", false, false}, - {"/tmp:/tmp:rw", "", "/tmp", "/tmp", "", "", true, false}, - {"/tmp:/tmp:foo", "", "/tmp", "/tmp", "", "", false, true}, - {"name:/tmp", "", "", "", "", "", false, true}, - {"name:/tmp", "external", "/tmp", "", "name", "external", true, true}, - {"external/name:/tmp:rw", "", "/tmp", "", "name", "external", true, true}, - {"external/name:/tmp:ro", "", "/tmp", "", "name", "external", false, true}, - {"external/name:/tmp:foo", "", "/tmp", "", "name", "external", false, true}, - {"name:/tmp", "local", "", "", "", "", false, true}, - {"local/name:/tmp:rw", "", "", "", "", "", true, true}, - } - - for _, c := range cases { - conf := &runconfig.Config{VolumeDriver: c.driver} - m, err := parseBindMount(c.bind, conf) - if c.fail { - if err == nil { - t.Fatalf("Expected error, was nil, for spec %s\n", c.bind) - } - continue - } - - if m.Destination != c.expDest { - t.Fatalf("Expected destination %s, was %s, for spec %s\n", c.expDest, m.Destination, c.bind) - } - - if m.Source != c.expSource { - t.Fatalf("Expected source %s, was %s, for spec %s\n", c.expSource, m.Source, c.bind) - } - - if m.Name != c.expName { - t.Fatalf("Expected name %s, was %s for spec %s\n", c.expName, m.Name, c.bind) - } - - if m.Driver != c.expDriver { - t.Fatalf("Expected driver %s, was %s, for spec %s\n", c.expDriver, m.Driver, c.bind) - } - - if m.RW != c.expRW { - t.Fatalf("Expected RW %v, was %v for spec %s\n", c.expRW, m.RW, c.bind) - } - } -} +import "testing" func TestParseVolumeFrom(t *testing.T) { cases := []struct {