diff --git a/api/client/volume.go b/api/client/volume.go index a7c686348a..53ad3da780 100644 --- a/api/client/volume.go +++ b/api/client/volume.go @@ -65,6 +65,9 @@ func (cli *DockerCli) CmdVolumeLs(args ...string) error { w := tabwriter.NewWriter(cli.out, 20, 1, 3, ' ', 0) if !*quiet { + for _, warn := range volumes.Warnings { + fmt.Fprintln(cli.err, warn) + } fmt.Fprintf(w, "DRIVER \tVOLUME NAME") fmt.Fprintf(w, "\n") } @@ -102,7 +105,7 @@ func (cli *DockerCli) CmdVolumeInspect(args ...string) error { return cli.inspectElements(*tmplStr, cmd.Args(), inspectSearcher) } -// CmdVolumeCreate creates a new container from a given image. +// CmdVolumeCreate creates a new volume. // // Usage: docker volume create [OPTIONS] func (cli *DockerCli) CmdVolumeCreate(args ...string) error { @@ -131,7 +134,7 @@ func (cli *DockerCli) CmdVolumeCreate(args ...string) error { return nil } -// CmdVolumeRm removes one or more containers. +// CmdVolumeRm removes one or more volumes. // // Usage: docker volume rm VOLUME [VOLUME...] func (cli *DockerCli) CmdVolumeRm(args ...string) error { @@ -140,6 +143,7 @@ func (cli *DockerCli) CmdVolumeRm(args ...string) error { cmd.ParseFlags(args, true) var status = 0 + for _, name := range cmd.Args() { if err := cli.client.VolumeRemove(name); err != nil { fmt.Fprintf(cli.err, "%s\n", err) diff --git a/api/server/router/volume/backend.go b/api/server/router/volume/backend.go index aa69972cfc..0c09b073e2 100644 --- a/api/server/router/volume/backend.go +++ b/api/server/router/volume/backend.go @@ -8,7 +8,7 @@ import ( // Backend is the methods that need to be implemented to provide // volume specific functionality type Backend interface { - Volumes(filter string) ([]*types.Volume, error) + Volumes(filter string) ([]*types.Volume, []string, error) VolumeInspect(name string) (*types.Volume, error) VolumeCreate(name, driverName string, opts map[string]string) (*types.Volume, error) diff --git a/api/server/router/volume/volume_routes.go b/api/server/router/volume/volume_routes.go index 5b0787c57a..882de3a60b 100644 --- a/api/server/router/volume/volume_routes.go +++ b/api/server/router/volume/volume_routes.go @@ -14,11 +14,11 @@ func (v *volumeRouter) getVolumesList(ctx context.Context, w http.ResponseWriter return err } - volumes, err := v.backend.Volumes(r.Form.Get("filters")) + volumes, warnings, err := v.backend.Volumes(r.Form.Get("filters")) if err != nil { return err } - return httputils.WriteJSON(w, http.StatusOK, &types.VolumesListResponse{Volumes: volumes}) + return httputils.WriteJSON(w, http.StatusOK, &types.VolumesListResponse{Volumes: volumes, Warnings: warnings}) } func (v *volumeRouter) getVolumeByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { diff --git a/api/types/types.go b/api/types/types.go index 11c447116f..80780ca9b0 100644 --- a/api/types/types.go +++ b/api/types/types.go @@ -366,7 +366,8 @@ type Volume struct { // VolumesListResponse contains the response for the remote API: // GET "/volumes" type VolumesListResponse struct { - Volumes []*Volume // Volumes is the list of volumes being returned + Volumes []*Volume // Volumes is the list of volumes being returned + Warnings []string // Warnings is a list of warnings that occurred when getting the list from the volume drivers } // VolumeCreateRequest contains the response for the remote API: diff --git a/daemon/create.go b/daemon/create.go index 42002ce98e..7456a99797 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -10,7 +10,7 @@ import ( "github.com/docker/docker/layer" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/volume" + volumestore "github.com/docker/docker/volume/store" "github.com/opencontainers/runc/libcontainer/label" ) @@ -162,17 +162,12 @@ func (daemon *Daemon) VolumeCreate(name, driverName string, opts map[string]stri v, err := daemon.volumes.Create(name, driverName, opts) if err != nil { + if volumestore.IsNameConflict(err) { + return nil, derr.ErrorVolumeNameTaken.WithArgs(name) + } return nil, err } - // keep "docker run -v existing_volume:/foo --volume-driver other_driver" work - if (driverName != "" && v.DriverName() != driverName) || (driverName == "" && v.DriverName() != volume.DefaultDriverName) { - return nil, derr.ErrorVolumeNameTaken.WithArgs(name, v.DriverName()) - } - - if driverName == "" { - driverName = volume.DefaultDriverName - } - daemon.LogVolumeEvent(name, "create", map[string]string{"driver": driverName}) + daemon.LogVolumeEvent(v.Name(), "create", map[string]string{"driver": v.DriverName()}) return volumeToAPIType(v), nil } diff --git a/daemon/create_unix.go b/daemon/create_unix.go index 25548d231e..865400cacc 100644 --- a/daemon/create_unix.go +++ b/daemon/create_unix.go @@ -51,7 +51,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain } } - v, err := daemon.createVolume(name, volumeDriver, nil) + v, err := daemon.volumes.CreateWithRef(name, volumeDriver, container.ID, nil) if err != nil { return err } diff --git a/daemon/create_windows.go b/daemon/create_windows.go index 33e54141e2..6e6ec9c12e 100644 --- a/daemon/create_windows.go +++ b/daemon/create_windows.go @@ -42,7 +42,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain // Create the volume in the volume driver. If it doesn't exist, // a new one will be created. - v, err := daemon.createVolume(mp.Name, volumeDriver, nil) + v, err := daemon.volumes.CreateWithRef(mp.Name, volumeDriver, container.ID, nil) if err != nil { return err } diff --git a/daemon/daemon.go b/daemon/daemon.go index d12d3e4867..849a97b9df 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1487,10 +1487,7 @@ func configureVolumes(config *Config, rootUID, rootGID int) (*store.VolumeStore, } volumedrivers.Register(volumesDriver, volumesDriver.Name()) - s := store.New() - s.AddAll(volumesDriver.List()) - - return s, nil + return store.New(), nil } // AuthenticateToRegistry checks the validity of credentials in authConfig diff --git a/daemon/delete.go b/daemon/delete.go index 6aaecc1289..c513cd8ddd 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -151,6 +151,7 @@ func (daemon *Daemon) VolumeRm(name string) error { if err != nil { return err } + if err := daemon.volumes.Remove(v); err != nil { if volumestore.IsInUse(err) { return derr.ErrorCodeRmVolumeInUse.WithArgs(err) diff --git a/daemon/list.go b/daemon/list.go index 68944d6598..2e9ff83310 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -392,24 +392,27 @@ func (daemon *Daemon) transformContainer(container *container.Container, ctx *li // Volumes lists known volumes, using the filter to restrict the range // of volumes returned. -func (daemon *Daemon) Volumes(filter string) ([]*types.Volume, error) { +func (daemon *Daemon) Volumes(filter string) ([]*types.Volume, []string, error) { var volumesOut []*types.Volume volFilters, err := filters.FromParam(filter) if err != nil { - return nil, err + return nil, nil, err } filterUsed := volFilters.Include("dangling") && (volFilters.ExactMatch("dangling", "true") || volFilters.ExactMatch("dangling", "1")) - volumes := daemon.volumes.List() + volumes, warnings, err := daemon.volumes.List() + if err != nil { + return nil, nil, err + } + if filterUsed { + volumes = daemon.volumes.FilterByUsed(volumes) + } for _, v := range volumes { - if filterUsed && daemon.volumes.Count(v) > 0 { - continue - } volumesOut = append(volumesOut, volumeToAPIType(v)) } - return volumesOut, nil + return volumesOut, warnings, nil } func populateImageFilterByParents(ancestorMap map[image.ID]bool, imageID image.ID, getChildren func(image.ID) []image.ID) { diff --git a/daemon/mounts.go b/daemon/mounts.go index 6b47575697..4f3669d37c 100644 --- a/daemon/mounts.go +++ b/daemon/mounts.go @@ -11,10 +11,11 @@ import ( func (daemon *Daemon) prepareMountPoints(container *container.Container) error { for _, config := range container.MountPoints { if len(config.Driver) > 0 { - v, err := daemon.createVolume(config.Name, config.Driver, nil) + v, err := daemon.volumes.GetWithRef(config.Name, config.Driver, container.ID) if err != nil { return err } + config.Volume = v } } @@ -27,10 +28,10 @@ func (daemon *Daemon) removeMountPoints(container *container.Container, rm bool) if m.Volume == nil { continue } - daemon.volumes.Decrement(m.Volume) + daemon.volumes.Dereference(m.Volume, container.ID) if rm { err := daemon.volumes.Remove(m.Volume) - // ErrVolumeInUse is ignored because having this + // Ignore volume in use errors because having this // volume being referenced by other container is // not an error, but an implementation detail. // This prevents docker from logging "ERROR: Volume in use" diff --git a/daemon/volumes.go b/daemon/volumes.go index 4b5522d66a..df95af79aa 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -32,16 +32,6 @@ func volumeToAPIType(v volume.Volume) *types.Volume { } } -// createVolume creates a volume. -func (daemon *Daemon) createVolume(name, driverName string, opts map[string]string) (volume.Volume, error) { - v, err := daemon.volumes.Create(name, driverName, opts) - if err != nil { - return nil, err - } - daemon.volumes.Increment(v) - return v, nil -} - // Len returns the number of mounts. Used in sorting. func (m mounts) Len() int { return len(m) @@ -103,7 +93,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo } if len(cp.Source) == 0 { - v, err := daemon.createVolume(cp.Name, cp.Driver, nil) + v, err := daemon.volumes.GetWithRef(cp.Name, cp.Driver, container.ID) if err != nil { return err } @@ -128,7 +118,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo if len(bind.Name) > 0 && len(bind.Driver) > 0 { // create the volume - v, err := daemon.createVolume(bind.Name, bind.Driver, nil) + v, err := daemon.volumes.CreateWithRef(bind.Name, bind.Driver, container.ID, nil) if err != nil { return err } @@ -153,7 +143,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo for _, m := range mountPoints { if m.BackwardsCompatible() { if mp, exists := container.MountPoints[m.Destination]; exists && mp.Volume != nil { - daemon.volumes.Decrement(mp.Volume) + daemon.volumes.Dereference(mp.Volume, container.ID) } } } diff --git a/errors/daemon.go b/errors/daemon.go index 0a1bd7d477..1cb0da562f 100644 --- a/errors/daemon.go +++ b/errors/daemon.go @@ -908,7 +908,7 @@ var ( // trying to create a volume that has existed using different driver. ErrorVolumeNameTaken = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "VOLUME_NAME_TAKEN", - Message: "A volume named %q already exists with the %q driver. Choose a different volume name.", + Message: "A volume named %s already exists. Choose a different volume name.", Description: "An attempt to create a volume using a driver but the volume already exists with a different driver", HTTPStatusCode: http.StatusInternalServerError, }) diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index cb14a190e0..5dee35f839 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -1733,8 +1733,8 @@ func (s *DockerDaemonSuite) TestDaemonRestartRmVolumeInUse(c *check.C) { c.Assert(s.d.Restart(), check.IsNil) out, err = s.d.Cmd("volume", "rm", "test") - c.Assert(err, check.Not(check.IsNil), check.Commentf("should not be able to remove in use volume after daemon restart")) - c.Assert(strings.Contains(out, "in use"), check.Equals, true) + c.Assert(err, check.NotNil, check.Commentf("should not be able to remove in use volume after daemon restart")) + c.Assert(out, checker.Contains, "in use") } func (s *DockerDaemonSuite) TestDaemonRestartLocalVolumes(c *check.C) { 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 13c525b84e..a8ad58f233 100644 --- a/integration-cli/docker_cli_start_volume_driver_unix_test.go +++ b/integration-cli/docker_cli_start_volume_driver_unix_test.go @@ -32,6 +32,8 @@ type eventCounter struct { mounts int unmounts int paths int + lists int + gets int } type DockerExternalVolumeSuite struct { @@ -64,6 +66,12 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) { Err string `json:",omitempty"` } + type vol struct { + Name string + Mountpoint string + } + var volList []vol + read := func(b io.ReadCloser) (pluginRequest, error) { defer b.Close() var pr pluginRequest @@ -93,29 +101,61 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) { mux.HandleFunc("/VolumeDriver.Create", func(w http.ResponseWriter, r *http.Request) { s.ec.creations++ - - _, err := read(r.Body) - if err != nil { - send(w, err) - return - } - - send(w, nil) - }) - - mux.HandleFunc("/VolumeDriver.Remove", func(w http.ResponseWriter, r *http.Request) { - s.ec.removals++ - pr, err := read(r.Body) if err != nil { send(w, err) return } + volList = append(volList, vol{Name: pr.Name}) + send(w, nil) + }) + + mux.HandleFunc("/VolumeDriver.List", func(w http.ResponseWriter, r *http.Request) { + s.ec.lists++ + send(w, map[string][]vol{"Volumes": volList}) + }) + + mux.HandleFunc("/VolumeDriver.Get", func(w http.ResponseWriter, r *http.Request) { + s.ec.gets++ + pr, err := read(r.Body) + if err != nil { + send(w, err) + return + } + + for _, v := range volList { + if v.Name == pr.Name { + v.Mountpoint = hostVolumePath(pr.Name) + send(w, map[string]vol{"Volume": v}) + return + } + } + send(w, `{"Err": "no such volume"}`) + }) + + mux.HandleFunc("/VolumeDriver.Remove", func(w http.ResponseWriter, r *http.Request) { + s.ec.removals++ + pr, err := read(r.Body) + if err != nil { + send(w, err) + return + } + if err := os.RemoveAll(hostVolumePath(pr.Name)); err != nil { send(w, &pluginResp{Err: err.Error()}) return } + for i, v := range volList { + if v.Name == pr.Name { + if err := os.RemoveAll(hostVolumePath(v.Name)); err != nil { + send(w, fmt.Sprintf(`{"Err": "%v"}`, err)) + return + } + volList = append(volList[:i], volList[i+1:]...) + break + } + } send(w, nil) }) @@ -128,8 +168,7 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) { return } p := hostVolumePath(pr.Name) - - fmt.Fprintln(w, fmt.Sprintf("{\"Mountpoint\": \"%s\"}", p)) + send(w, &pluginResp{Mountpoint: p}) }) mux.HandleFunc("/VolumeDriver.Mount", func(w http.ResponseWriter, r *http.Request) { @@ -164,7 +203,7 @@ func (s *DockerExternalVolumeSuite) SetUpSuite(c *check.C) { return } - fmt.Fprintln(w, nil) + send(w, nil) }) err := os.MkdirAll("/etc/docker/plugins", 0755) @@ -287,8 +326,8 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverNamedCheckBindLocalV // Make sure a request to use a down driver doesn't block other requests func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverLookupNotBlocked(c *check.C) { specPath := "/etc/docker/plugins/down-driver.spec" - err := ioutil.WriteFile("/etc/docker/plugins/down-driver.spec", []byte("tcp://127.0.0.7:9999"), 0644) - c.Assert(err, checker.IsNil) + err := ioutil.WriteFile(specPath, []byte("tcp://127.0.0.7:9999"), 0644) + c.Assert(err, check.IsNil) defer os.RemoveAll(specPath) chCmd1 := make(chan struct{}) @@ -316,10 +355,11 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverLookupNotBlocked(c * case err := <-chCmd2: c.Assert(err, checker.IsNil) case <-time.After(5 * time.Second): - c.Fatal("volume creates are blocked by previous create requests when previous driver is down") cmd2.Process.Kill() + c.Fatal("volume creates are blocked by previous create requests when previous driver is down") } } + func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverRetryNotImmediatelyExists(c *check.C) { err := s.d.StartWithBusybox() c.Assert(err, checker.IsNil) @@ -371,3 +411,24 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverBindExternalVolume(c c.Assert(mounts[0].Name, checker.Equals, "foo") c.Assert(mounts[0].Driver, checker.Equals, "test-external-volume-driver") } + +func (s *DockerExternalVolumeSuite) TestStartExternalVolumeDriverList(c *check.C) { + dockerCmd(c, "volume", "create", "-d", "test-external-volume-driver", "--name", "abc") + out, _ := dockerCmd(c, "volume", "ls") + ls := strings.Split(strings.TrimSpace(out), "\n") + c.Assert(len(ls), check.Equals, 2, check.Commentf("\n%s", out)) + + vol := strings.Fields(ls[len(ls)-1]) + c.Assert(len(vol), check.Equals, 2, check.Commentf("%v", vol)) + c.Assert(vol[0], check.Equals, "test-external-volume-driver") + c.Assert(vol[1], check.Equals, "abc") + + c.Assert(s.ec.lists, check.Equals, 1) +} + +func (s *DockerExternalVolumeSuite) TestStartExternalVolumeDriverGet(c *check.C) { + out, _, err := dockerCmdWithError("volume", "inspect", "dummy") + c.Assert(err, check.NotNil, check.Commentf(out)) + c.Assert(s.ec.gets, check.Equals, 1) + c.Assert(out, checker.Contains, "No such volume") +} diff --git a/integration-cli/docker_cli_volume_test.go b/integration-cli/docker_cli_volume_test.go index 13aade916d..7d1683a7ee 100644 --- a/integration-cli/docker_cli_volume_test.go +++ b/integration-cli/docker_cli_volume_test.go @@ -4,9 +4,7 @@ import ( "os/exec" "strings" - derr "github.com/docker/docker/errors" "github.com/docker/docker/pkg/integration/checker" - "github.com/docker/docker/volume" "github.com/go-check/check" ) @@ -25,8 +23,7 @@ func (s *DockerSuite) TestVolumeCliCreateOptionConflict(c *check.C) { dockerCmd(c, "volume", "create", "--name=test") out, _, err := dockerCmdWithError("volume", "create", "--name", "test", "--driver", "nosuchdriver") c.Assert(err, check.NotNil, check.Commentf("volume create exception name already in use with another driver")) - stderr := derr.ErrorVolumeNameTaken.WithArgs("test", volume.DefaultDriverName).Error() - c.Assert(strings.Contains(out, strings.TrimPrefix(stderr, "volume name taken: ")), check.Equals, true) + c.Assert(out, checker.Contains, "A volume named test already exists") out, _ = dockerCmd(c, "volume", "inspect", "--format='{{ .Driver }}'", "test") _, _, err = dockerCmdWithError("volume", "create", "--name", "test", "--driver", strings.TrimSpace(out)) diff --git a/pkg/plugins/discovery.go b/pkg/plugins/discovery.go index 5ea8db5c0a..3f79661782 100644 --- a/pkg/plugins/discovery.go +++ b/pkg/plugins/discovery.go @@ -25,6 +25,38 @@ func newLocalRegistry() localRegistry { return localRegistry{} } +// Scan scans all the plugin paths and returns all the names it found +func Scan() ([]string, error) { + var names []string + if err := filepath.Walk(socketsPath, func(path string, fi os.FileInfo, err error) error { + if err != nil { + return nil + } + + if fi.Mode()&os.ModeSocket != 0 { + name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name())) + names = append(names, name) + } + return nil + }); err != nil { + return nil, err + } + + for _, path := range specsPaths { + if err := filepath.Walk(path, func(p string, fi os.FileInfo, err error) error { + if err != nil || fi.IsDir() { + return nil + } + name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name())) + names = append(names, name) + return nil + }); err != nil { + return nil, err + } + } + return names, nil +} + // Plugin returns the plugin registered with the given name (or returns an error). func (l *localRegistry) Plugin(name string) (*Plugin, error) { socketpaths := pluginPaths(socketsPath, name, ".sock") diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 6317e4f174..7157107ba3 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -108,6 +108,15 @@ func (p *Plugin) activateWithLock() error { return nil } +func (p *Plugin) implements(kind string) bool { + for _, driver := range p.Manifest.Implements { + if driver == kind { + return true + } + } + return false +} + func load(name string) (*Plugin, error) { return loadWithRetry(name, true) } @@ -166,11 +175,9 @@ func Get(name, imp string) (*Plugin, error) { if err != nil { return nil, err } - for _, driver := range pl.Manifest.Implements { - logrus.Debugf("%s implements: %s", name, driver) - if driver == imp { - return pl, nil - } + if pl.implements(imp) { + logrus.Debugf("%s implements: %s", name, imp) + return pl, nil } return nil, ErrNotImplements } @@ -179,3 +186,37 @@ func Get(name, imp string) (*Plugin, error) { func Handle(iface string, fn func(string, *Client)) { extpointHandlers[iface] = fn } + +// GetAll returns all the plugins for the specified implementation +func GetAll(imp string) ([]*Plugin, error) { + pluginNames, err := Scan() + if err != nil { + return nil, err + } + + type plLoad struct { + pl *Plugin + err error + } + + chPl := make(chan plLoad, len(pluginNames)) + for _, name := range pluginNames { + go func(name string) { + pl, err := loadWithRetry(name, false) + chPl <- plLoad{pl, err} + }(name) + } + + var out []*Plugin + for i := 0; i < len(pluginNames); i++ { + pl := <-chPl + if pl.err != nil { + logrus.Error(err) + continue + } + if pl.pl.implements(imp) { + out = append(out, pl.pl) + } + } + return out, nil +} diff --git a/volume/drivers/adapter.go b/volume/drivers/adapter.go index a5db6991e7..f29cf75748 100644 --- a/volume/drivers/adapter.go +++ b/volume/drivers/adapter.go @@ -26,6 +26,38 @@ func (a *volumeDriverAdapter) Remove(v volume.Volume) error { return a.proxy.Remove(v.Name()) } +func (a *volumeDriverAdapter) List() ([]volume.Volume, error) { + ls, err := a.proxy.List() + if err != nil { + return nil, err + } + + var out []volume.Volume + for _, vp := range ls { + out = append(out, &volumeAdapter{ + proxy: a.proxy, + name: vp.Name, + driverName: a.name, + eMount: vp.Mountpoint, + }) + } + return out, nil +} + +func (a *volumeDriverAdapter) Get(name string) (volume.Volume, error) { + v, err := a.proxy.Get(name) + if err != nil { + return nil, err + } + + return &volumeAdapter{ + proxy: a.proxy, + name: v.Name, + driverName: a.Name(), + eMount: v.Mountpoint, + }, nil +} + type volumeAdapter struct { proxy *volumeDriverProxy name string diff --git a/volume/drivers/extpoint.go b/volume/drivers/extpoint.go index 3783ec6e67..6f894e0ae6 100644 --- a/volume/drivers/extpoint.go +++ b/volume/drivers/extpoint.go @@ -15,6 +15,8 @@ import ( var drivers = &driverExtpoint{extensions: make(map[string]volume.Driver)} +const extName = "VolumeDriver" + // NewVolumeDriver returns a driver has the given name mapped on the given client. func NewVolumeDriver(name string, c client) volume.Driver { proxy := &volumeDriverProxy{c} @@ -22,6 +24,7 @@ func NewVolumeDriver(name string, c client) volume.Driver { } type opts map[string]string +type list []*proxyVolume // volumeDriver defines the available functions that volume plugins must implement. // This interface is only defined to generate the proxy objects. @@ -37,6 +40,10 @@ type volumeDriver interface { Mount(name string) (mountpoint string, err error) // Unmount the given volume Unmount(name string) (err error) + // List lists all the volumes known to the driver + List() (volumes list, err error) + // Get retreives the volume with the requested name + Get(name string) (volume *proxyVolume, err error) } type driverExtpoint struct { @@ -82,7 +89,7 @@ func Lookup(name string) (volume.Driver, error) { if ok { return ext, nil } - pl, err := plugins.Get(name, "VolumeDriver") + pl, err := plugins.Get(name, extName) if err != nil { return nil, fmt.Errorf("Error looking up volume plugin %s: %v", name, err) } @@ -116,3 +123,30 @@ func GetDriverList() []string { } return driverList } + +// GetAllDrivers lists all the registered drivers +func GetAllDrivers() ([]volume.Driver, error) { + plugins, err := plugins.GetAll(extName) + if err != nil { + return nil, err + } + var ds []volume.Driver + + drivers.Lock() + defer drivers.Unlock() + + for _, d := range drivers.extensions { + ds = append(ds, d) + } + + for _, p := range plugins { + ext, ok := drivers.extensions[p.Name] + if ok { + continue + } + ext = NewVolumeDriver(p.Name, p.Client) + drivers.extensions[p.Name] = ext + ds = append(ds, ext) + } + return ds, nil +} diff --git a/volume/drivers/extpoint_test.go b/volume/drivers/extpoint_test.go index 8ab60c95e3..26c06954ee 100644 --- a/volume/drivers/extpoint_test.go +++ b/volume/drivers/extpoint_test.go @@ -11,7 +11,8 @@ func TestGetDriver(t *testing.T) { if err == nil { t.Fatal("Expected error, was nil") } - Register(volumetestutils.FakeDriver{}, "fake") + + Register(volumetestutils.NewFakeDriver("fake"), "fake") d, err := GetDriver("fake") if err != nil { t.Fatal(err) diff --git a/volume/drivers/proxy.go b/volume/drivers/proxy.go index f2e2f04467..5c7cdcb7c7 100644 --- a/volume/drivers/proxy.go +++ b/volume/drivers/proxy.go @@ -149,3 +149,59 @@ func (pp *volumeDriverProxy) Unmount(name string) (err error) { return } + +type volumeDriverProxyListRequest struct { +} + +type volumeDriverProxyListResponse struct { + Volumes list + Err string +} + +func (pp *volumeDriverProxy) List() (volumes list, err error) { + var ( + req volumeDriverProxyListRequest + ret volumeDriverProxyListResponse + ) + + if err = pp.Call("VolumeDriver.List", req, &ret); err != nil { + return + } + + volumes = ret.Volumes + + if ret.Err != "" { + err = errors.New(ret.Err) + } + + return +} + +type volumeDriverProxyGetRequest struct { + Name string +} + +type volumeDriverProxyGetResponse struct { + Volume *proxyVolume + Err string +} + +func (pp *volumeDriverProxy) Get(name string) (volume *proxyVolume, err error) { + var ( + req volumeDriverProxyGetRequest + ret volumeDriverProxyGetResponse + ) + + req.Name = name + if err = pp.Call("VolumeDriver.Get", req, &ret); err != nil { + return + } + + volume = ret.Volume + + if ret.Err != "" { + err = errors.New(ret.Err) + } + + return +} diff --git a/volume/drivers/proxy_test.go b/volume/drivers/proxy_test.go index 5e34b0773b..6b26f9dc5b 100644 --- a/volume/drivers/proxy_test.go +++ b/volume/drivers/proxy_test.go @@ -42,6 +42,16 @@ func TestVolumeRequestError(t *testing.T) { fmt.Fprintln(w, `{"Err": "Unknown volume"}`) }) + mux.HandleFunc("/VolumeDriver.List", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/vnd.docker.plugins.v1+json") + fmt.Fprintln(w, `{"Err": "Cannot list volumes"}`) + }) + + mux.HandleFunc("/VolumeDriver.Get", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/vnd.docker.plugins.v1+json") + fmt.Fprintln(w, `{"Err": "Cannot get volume"}`) + }) + u, _ := url.Parse(server.URL) client, err := plugins.NewClient("tcp://"+u.Host, tlsconfig.Options{InsecureSkipVerify: true}) if err != nil { @@ -93,4 +103,20 @@ func TestVolumeRequestError(t *testing.T) { if !strings.Contains(err.Error(), "Unknown volume") { t.Fatalf("Unexpected error: %v\n", err) } + + _, err = driver.List() + if err == nil { + t.Fatal("Expected error, was nil") + } + if !strings.Contains(err.Error(), "Cannot list volumes") { + t.Fatalf("Unexpected error: %v\n", err) + } + + _, err = driver.Get("volume") + if err == nil { + t.Fatal("Expected error, was nil") + } + if !strings.Contains(err.Error(), "Cannot get volume") { + t.Fatalf("Unexpected error: %v\n", err) + } } diff --git a/volume/local/local.go b/volume/local/local.go index 44fd3e57e5..1a44c05858 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -82,12 +82,12 @@ type Root struct { } // List lists all the volumes -func (r *Root) List() []volume.Volume { +func (r *Root) List() ([]volume.Volume, error) { var ls []volume.Volume for _, v := range r.volumes { ls = append(ls, v) } - return ls + return ls, nil } // DataPath returns the constructed path of this volume. diff --git a/volume/local/local_test.go b/volume/local/local_test.go index 97d0f082ae..decf07cba1 100644 --- a/volume/local/local_test.go +++ b/volume/local/local_test.go @@ -43,7 +43,7 @@ func TestRemove(t *testing.T) { t.Fatal("volume dir not removed") } - if len(r.List()) != 0 { + if l, _ := r.List(); len(l) != 0 { t.Fatal("expected there to be no volumes") } } diff --git a/volume/store/errors.go b/volume/store/errors.go index 0cc59cdea8..7bdfa12b95 100644 --- a/volume/store/errors.go +++ b/volume/store/errors.go @@ -1,6 +1,9 @@ package store -import "errors" +import ( + "errors" + "strings" +) var ( // errVolumeInUse is a typed error returned when trying to remove a volume that is currently in use by a container @@ -9,6 +12,8 @@ var ( errNoSuchVolume = errors.New("no such volume") // errInvalidName is a typed error returned when creating a volume with a name that is not valid on the platform errInvalidName = errors.New("volume name is not valid on this platform") + // errNameConflict is a typed error returned on create when a volume exists with the given name, but for a different driver + errNameConflict = errors.New("conflict: volume name must be unique") ) // OpErr is the error type returned by functions in the store package. It describes @@ -20,6 +25,8 @@ type OpErr struct { Op string // Name is the name of the resource being requested for this op, typically the volume name or the driver name. Name string + // Refs is the list of references associated with the resource. + Refs []string } // Error satisfies the built-in error interface type. @@ -33,6 +40,9 @@ func (e *OpErr) Error() string { } s = s + ": " + e.Err.Error() + if len(e.Refs) > 0 { + s = s + " - " + "[" + strings.Join(e.Refs, ", ") + "]" + } return s } @@ -47,6 +57,12 @@ func IsNotExist(err error) bool { return isErr(err, errNoSuchVolume) } +// IsNameConflict returns a boolean indicating whether the error indicates that a +// volume name is already taken +func IsNameConflict(err error) bool { + return isErr(err, errNameConflict) +} + func isErr(err error, expected error) bool { switch pe := err.(type) { case nil: diff --git a/volume/store/store.go b/volume/store/store.go index ebbb512efb..e85f542bff 100644 --- a/volume/store/store.go +++ b/volume/store/store.go @@ -13,66 +13,153 @@ import ( // reference counting of volumes in the system. func New() *VolumeStore { return &VolumeStore{ - vols: make(map[string]*volumeCounter), locks: &locker.Locker{}, + names: make(map[string]string), + refs: make(map[string][]string), } } -func (s *VolumeStore) get(name string) (*volumeCounter, bool) { +func (s *VolumeStore) getNamed(name string) (string, bool) { s.globalLock.Lock() - vc, exists := s.vols[name] + driverName, exists := s.names[name] s.globalLock.Unlock() - return vc, exists + return driverName, exists } -func (s *VolumeStore) set(name string, vc *volumeCounter) { +func (s *VolumeStore) setNamed(name, driver, ref string) { s.globalLock.Lock() - s.vols[name] = vc + s.names[name] = driver + if len(ref) > 0 { + s.refs[name] = append(s.refs[name], ref) + } s.globalLock.Unlock() } -func (s *VolumeStore) remove(name string) { +func (s *VolumeStore) purge(name string) { s.globalLock.Lock() - delete(s.vols, name) + delete(s.names, name) + delete(s.refs, name) s.globalLock.Unlock() } // VolumeStore is a struct that stores the list of volumes available and keeps track of their usage counts type VolumeStore struct { - vols map[string]*volumeCounter locks *locker.Locker globalLock sync.Mutex + // names stores the volume name -> driver name relationship. + // This is used for making lookups faster so we don't have to probe all drivers + names map[string]string + // refs stores the volume name and the list of things referencing it + refs map[string][]string } -// volumeCounter keeps track of references to a volume -type volumeCounter struct { - volume.Volume - count uint -} - -// AddAll adds a list of volumes to the store -func (s *VolumeStore) AddAll(vols []volume.Volume) { - for _, v := range vols { - s.vols[normaliseVolumeName(v.Name())] = &volumeCounter{v, 0} +// List proxies to all registered volume drivers to get the full list of volumes +// If a driver returns a volume that has name which conflicts with a another volume from a different driver, +// the first volume is chosen and the conflicting volume is dropped. +func (s *VolumeStore) List() ([]volume.Volume, []string, error) { + vols, warnings, err := s.list() + if err != nil { + return nil, nil, &OpErr{Err: err, Op: "list"} } + var out []volume.Volume + + for _, v := range vols { + name := normaliseVolumeName(v.Name()) + + s.locks.Lock(name) + driverName, exists := s.getNamed(name) + if !exists { + s.setNamed(name, v.DriverName(), "") + } + if exists && driverName != v.DriverName() { + logrus.Warnf("Volume name %s already exists for driver %s, not including volume returned by %s", v.Name(), driverName, v.DriverName()) + s.locks.Unlock(v.Name()) + continue + } + + out = append(out, v) + s.locks.Unlock(v.Name()) + } + return out, warnings, nil } -// Create tries to find an existing volume with the given name or create a new one from the passed in driver +// list goes through each volume driver and asks for its list of volumes. +func (s *VolumeStore) list() ([]volume.Volume, []string, error) { + drivers, err := volumedrivers.GetAllDrivers() + if err != nil { + return nil, nil, err + } + var ( + ls []volume.Volume + warnings []string + ) + + type vols struct { + vols []volume.Volume + err error + } + chVols := make(chan vols, len(drivers)) + + for _, vd := range drivers { + go func(d volume.Driver) { + vs, err := d.List() + if err != nil { + chVols <- vols{err: &OpErr{Err: err, Name: d.Name(), Op: "list"}} + return + } + chVols <- vols{vols: vs} + }(vd) + } + + for i := 0; i < len(drivers); i++ { + vs := <-chVols + + if vs.err != nil { + warnings = append(warnings, vs.err.Error()) + logrus.Warn(vs.err) + continue + } + ls = append(ls, vs.vols...) + } + return ls, warnings, nil +} + +// CreateWithRef creates a volume with the given name and driver and stores the ref +// This is just like Create() except we store the reference while holding the lock. +// This ensures there's no race between creating a volume and then storing a reference. +func (s *VolumeStore) CreateWithRef(name, driverName, ref string, opts map[string]string) (volume.Volume, error) { + name = normaliseVolumeName(name) + s.locks.Lock(name) + defer s.locks.Unlock(name) + + v, err := s.create(name, driverName, opts) + if err != nil { + return nil, &OpErr{Err: err, Name: name, Op: "create"} + } + + s.setNamed(name, v.DriverName(), ref) + return v, nil +} + +// Create creates a volume with the given name and driver. func (s *VolumeStore) Create(name, driverName string, opts map[string]string) (volume.Volume, error) { name = normaliseVolumeName(name) s.locks.Lock(name) defer s.locks.Unlock(name) - if vc, exists := s.get(name); exists { - v := vc.Volume - return v, nil - } - - vd, err := volumedrivers.GetDriver(driverName) + v, err := s.create(name, driverName, opts) if err != nil { - return nil, &OpErr{Err: err, Name: driverName, Op: "create"} + return nil, &OpErr{Err: err, Name: name, Op: "create"} } + s.setNamed(name, v.DriverName(), "") + return v, nil +} +// create asks the given driver to create a volume with the name/opts. +// If a volume with the name is already known, it will ask the stored driver for the volume. +// If the passed in driver name does not match the driver name which is stored for the given volume name, an error is returned. +// It is expected that callers of this function hold any neccessary locks. +func (s *VolumeStore) create(name, driverName string, opts map[string]string) (volume.Volume, error) { // Validate the name in a platform-specific manner valid, err := volume.IsVolumeNameValid(name) if err != nil { @@ -82,12 +169,45 @@ func (s *VolumeStore) Create(name, driverName string, opts map[string]string) (v return nil, &OpErr{Err: errInvalidName, Name: name, Op: "create"} } - v, err := vd.Create(name, opts) + vdName, exists := s.getNamed(name) + if exists { + if vdName != driverName && driverName != "" && driverName != volume.DefaultDriverName { + return nil, errNameConflict + } + driverName = vdName + } + + logrus.Debugf("Registering new volume reference: driver %s, name %s", driverName, name) + vd, err := volumedrivers.GetDriver(driverName) if err != nil { return nil, &OpErr{Op: "create", Name: name, Err: err} } - s.set(name, &volumeCounter{v, 0}) + if v, err := vd.Get(name); err == nil { + return v, nil + } + return vd.Create(name, opts) +} + +// GetWithRef gets a volume with the given name from the passed in driver and stores the ref +// This is just like Get(), but we store the reference while holding the lock. +// This makes sure there are no races between checking for the existance of a volume and adding a reference for it +func (s *VolumeStore) GetWithRef(name, driverName, ref string) (volume.Volume, error) { + name = normaliseVolumeName(name) + s.locks.Lock(name) + defer s.locks.Unlock(name) + + vd, err := volumedrivers.GetDriver(driverName) + if err != nil { + return nil, &OpErr{Err: err, Name: name, Op: "get"} + } + + v, err := vd.Get(name) + if err != nil { + return nil, &OpErr{Err: err, Name: name, Op: "get"} + } + + s.setNamed(name, v.DriverName(), ref) return v, nil } @@ -97,120 +217,116 @@ func (s *VolumeStore) Get(name string) (volume.Volume, error) { s.locks.Lock(name) defer s.locks.Unlock(name) - vc, exists := s.get(name) - if !exists { - return nil, &OpErr{Err: errNoSuchVolume, Name: name, Op: "get"} + v, err := s.getVolume(name) + if err != nil { + return nil, &OpErr{Err: err, Name: name, Op: "get"} } - return vc.Volume, nil + return v, nil } -// Remove removes the requested volume. A volume is not removed if the usage count is > 0 +// get requests the volume, if the driver info is stored it just access that driver, +// if the driver is unknown it probes all drivers until it finds the first volume with that name. +// it is expected that callers of this function hold any neccessary locks +func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { + logrus.Debugf("Getting volume reference for name: %s", name) + if vdName, exists := s.names[name]; exists { + vd, err := volumedrivers.GetDriver(vdName) + if err != nil { + return nil, err + } + return vd.Get(name) + } + + logrus.Debugf("Probing all drivers for volume with name: %s", name) + drivers, err := volumedrivers.GetAllDrivers() + if err != nil { + return nil, err + } + + for _, d := range drivers { + v, err := d.Get(name) + if err != nil { + continue + } + return v, nil + } + return nil, errNoSuchVolume +} + +// Remove removes the requested volume. A volume is not removed if it has any refs func (s *VolumeStore) Remove(v volume.Volume) error { name := normaliseVolumeName(v.Name()) s.locks.Lock(name) defer s.locks.Unlock(name) - logrus.Debugf("Removing volume reference: driver %s, name %s", v.DriverName(), name) - vc, exists := s.get(name) - if !exists { - return &OpErr{Err: errNoSuchVolume, Name: name, Op: "remove"} + if refs, exists := s.refs[name]; exists && len(refs) > 0 { + return &OpErr{Err: errVolumeInUse, Name: v.Name(), Op: "remove", Refs: refs} } - if vc.count > 0 { - return &OpErr{Err: errVolumeInUse, Name: name, Op: "remove"} - } - - vd, err := volumedrivers.GetDriver(vc.DriverName()) + vd, err := volumedrivers.GetDriver(v.DriverName()) if err != nil { - return &OpErr{Err: err, Name: vc.DriverName(), Op: "remove"} + return &OpErr{Err: err, Name: vd.Name(), Op: "remove"} } - if err := vd.Remove(vc.Volume); err != nil { + + logrus.Debugf("Removing volume reference: driver %s, name %s", v.DriverName(), name) + if err := vd.Remove(v); err != nil { return &OpErr{Err: err, Name: name, Op: "remove"} } - s.remove(name) + s.purge(name) return nil } -// Increment increments the usage count of the passed in volume by 1 -func (s *VolumeStore) Increment(v volume.Volume) { - name := normaliseVolumeName(v.Name()) - s.locks.Lock(name) - defer s.locks.Unlock(name) +// Dereference removes the specified reference to the volume +func (s *VolumeStore) Dereference(v volume.Volume, ref string) { + s.locks.Lock(v.Name()) + defer s.locks.Unlock(v.Name()) - logrus.Debugf("Incrementing volume reference: driver %s, name %s", v.DriverName(), v.Name()) - vc, exists := s.get(name) - if !exists { - s.set(name, &volumeCounter{v, 1}) - return - } - vc.count++ -} - -// Decrement decrements the usage count of the passed in volume by 1 -func (s *VolumeStore) Decrement(v volume.Volume) { - name := normaliseVolumeName(v.Name()) - s.locks.Lock(name) - defer s.locks.Unlock(name) - logrus.Debugf("Decrementing volume reference: driver %s, name %s", v.DriverName(), v.Name()) - - vc, exists := s.get(name) - if !exists { - return - } - if vc.count == 0 { - return - } - vc.count-- -} - -// Count returns the usage count of the passed in volume -func (s *VolumeStore) Count(v volume.Volume) uint { - name := normaliseVolumeName(v.Name()) - s.locks.Lock(name) - defer s.locks.Unlock(name) - - vc, exists := s.get(name) - if !exists { - return 0 - } - return vc.count -} - -// List returns all the available volumes -func (s *VolumeStore) List() []volume.Volume { s.globalLock.Lock() - defer s.globalLock.Unlock() - var ls []volume.Volume - for _, vc := range s.vols { - ls = append(ls, vc.Volume) + refs, exists := s.refs[v.Name()] + if !exists { + return } - return ls + + for i, r := range refs { + if r == ref { + s.refs[v.Name()] = append(s.refs[v.Name()][:i], s.refs[v.Name()][i+1:]...) + } + } + s.globalLock.Unlock() } // FilterByDriver returns the available volumes filtered by driver name -func (s *VolumeStore) FilterByDriver(name string) []volume.Volume { - return s.filter(byDriver(name)) +func (s *VolumeStore) FilterByDriver(name string) ([]volume.Volume, error) { + vd, err := volumedrivers.GetDriver(name) + if err != nil { + return nil, &OpErr{Err: err, Name: name, Op: "list"} + } + ls, err := vd.List() + if err != nil { + return nil, &OpErr{Err: err, Name: name, Op: "list"} + } + return ls, nil +} + +// FilterByUsed returns the available volumes filtered by if they are not in use +func (s *VolumeStore) FilterByUsed(vols []volume.Volume) []volume.Volume { + return s.filter(vols, func(v volume.Volume) bool { + s.locks.Lock(v.Name()) + defer s.locks.Unlock(v.Name()) + return len(s.refs[v.Name()]) == 0 + }) } // filterFunc defines a function to allow filter volumes in the store type filterFunc func(vol volume.Volume) bool -// byDriver generates a filterFunc to filter volumes by their driver name -func byDriver(name string) filterFunc { - return func(vol volume.Volume) bool { - return vol.DriverName() == name - } -} - // filter returns the available volumes filtered by a filterFunc function -func (s *VolumeStore) filter(f filterFunc) []volume.Volume { - s.globalLock.Lock() - defer s.globalLock.Unlock() +func (s *VolumeStore) filter(vols []volume.Volume, f filterFunc) []volume.Volume { var ls []volume.Volume - for _, vc := range s.vols { - if f(vc.Volume) { - ls = append(ls, vc.Volume) + for _, v := range vols { + if f(v) { + ls = append(ls, v) } } return ls diff --git a/volume/store/store_test.go b/volume/store/store_test.go index e06961d453..652feaa594 100644 --- a/volume/store/store_test.go +++ b/volume/store/store_test.go @@ -2,42 +2,16 @@ package store import ( "errors" + "strings" "testing" - "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" vt "github.com/docker/docker/volume/testutils" ) -func TestList(t *testing.T) { - volumedrivers.Register(vt.FakeDriver{}, "fake") - s := New() - s.AddAll([]volume.Volume{vt.NewFakeVolume("fake1"), vt.NewFakeVolume("fake2")}) - l := s.List() - if len(l) != 2 { - t.Fatalf("Expected 2 volumes in the store, got %v: %v", len(l), l) - } -} - -func TestGet(t *testing.T) { - volumedrivers.Register(vt.FakeDriver{}, "fake") - s := New() - s.AddAll([]volume.Volume{vt.NewFakeVolume("fake1"), vt.NewFakeVolume("fake2")}) - v, err := s.Get("fake1") - if err != nil { - t.Fatal(err) - } - if v.Name() != "fake1" { - t.Fatalf("Expected fake1 volume, got %v", v) - } - - if _, err := s.Get("fake4"); !IsNotExist(err) { - t.Fatalf("Expected IsNotExist error, got %v", err) - } -} - func TestCreate(t *testing.T) { - volumedrivers.Register(vt.FakeDriver{}, "fake") + volumedrivers.Register(vt.NewFakeDriver("fake"), "fake") + defer volumedrivers.Unregister("fake") s := New() v, err := s.Create("fake1", "fake", nil) if err != nil { @@ -46,7 +20,7 @@ func TestCreate(t *testing.T) { if v.Name() != "fake1" { t.Fatalf("Expected fake1 volume, got %v", v) } - if l := s.List(); len(l) != 1 { + if l, _, _ := s.List(); len(l) != 1 { t.Fatalf("Expected 1 volume in the store, got %v: %v", len(l), l) } @@ -62,93 +36,90 @@ func TestCreate(t *testing.T) { } func TestRemove(t *testing.T) { - volumedrivers.Register(vt.FakeDriver{}, "fake") + volumedrivers.Register(vt.NewFakeDriver("fake"), "fake") + volumedrivers.Register(vt.NewFakeDriver("noop"), "noop") + defer volumedrivers.Unregister("fake") + defer volumedrivers.Unregister("noop") s := New() - if err := s.Remove(vt.NoopVolume{}); !IsNotExist(err) { - t.Fatalf("Expected IsNotExist error, got %v", err) + + // doing string compare here since this error comes directly from the driver + expected := "no such volume" + if err := s.Remove(vt.NoopVolume{}); err == nil || !strings.Contains(err.Error(), expected) { + t.Fatalf("Expected error %q, got %v", expected, err) } - v, err := s.Create("fake1", "fake", nil) + + v, err := s.CreateWithRef("fake1", "fake", "fake", nil) if err != nil { t.Fatal(err) } - s.Increment(v) + if err := s.Remove(v); !IsInUse(err) { - t.Fatalf("Expected IsInUse error, got %v", err) + t.Fatalf("Expected ErrVolumeInUse error, got %v", err) } - s.Decrement(v) + s.Dereference(v, "fake") if err := s.Remove(v); err != nil { t.Fatal(err) } - if l := s.List(); len(l) != 0 { + if l, _, _ := s.List(); len(l) != 0 { t.Fatalf("Expected 0 volumes in the store, got %v, %v", len(l), l) } } -func TestIncrement(t *testing.T) { +func TestList(t *testing.T) { + volumedrivers.Register(vt.NewFakeDriver("fake"), "fake") + volumedrivers.Register(vt.NewFakeDriver("fake2"), "fake2") + defer volumedrivers.Unregister("fake") + defer volumedrivers.Unregister("fake2") + s := New() - v := vt.NewFakeVolume("fake1") - s.Increment(v) - if l := s.List(); len(l) != 1 { - t.Fatalf("Expected 1 volume, got %v, %v", len(l), l) + if _, err := s.Create("test", "fake", nil); err != nil { + t.Fatal(err) } - if c := s.Count(v); c != 1 { - t.Fatalf("Expected 1 counter, got %v", c) + if _, err := s.Create("test2", "fake2", nil); err != nil { + t.Fatal(err) } - s.Increment(v) - if l := s.List(); len(l) != 1 { - t.Fatalf("Expected 1 volume, got %v, %v", len(l), l) + ls, _, err := s.List() + if err != nil { + t.Fatal(err) } - if c := s.Count(v); c != 2 { - t.Fatalf("Expected 2 counter, got %v", c) + if len(ls) != 2 { + t.Fatalf("expected 2 volumes, got: %d", len(ls)) } - v2 := vt.NewFakeVolume("fake2") - s.Increment(v2) - if l := s.List(); len(l) != 2 { - t.Fatalf("Expected 2 volume, got %v, %v", len(l), l) + // and again with a new store + s = New() + ls, _, err = s.List() + if err != nil { + t.Fatal(err) } -} - -func TestDecrement(t *testing.T) { - s := New() - v := vt.NoopVolume{} - s.Decrement(v) - if c := s.Count(v); c != 0 { - t.Fatalf("Expected 0 volumes, got %v", c) - } - - s.Increment(v) - s.Increment(v) - s.Decrement(v) - if c := s.Count(v); c != 1 { - t.Fatalf("Expected 1 volume, got %v", c) - } - - s.Decrement(v) - if c := s.Count(v); c != 0 { - t.Fatalf("Expected 0 volumes, got %v", c) - } - - // Test counter cannot be negative. - s.Decrement(v) - if c := s.Count(v); c != 0 { - t.Fatalf("Expected 0 volumes, got %v", c) + if len(ls) != 2 { + t.Fatalf("expected 2 volumes, got: %d", len(ls)) } } func TestFilterByDriver(t *testing.T) { + volumedrivers.Register(vt.NewFakeDriver("fake"), "fake") + volumedrivers.Register(vt.NewFakeDriver("noop"), "noop") + defer volumedrivers.Unregister("fake") + defer volumedrivers.Unregister("noop") s := New() - s.Increment(vt.NewFakeVolume("fake1")) - s.Increment(vt.NewFakeVolume("fake2")) - s.Increment(vt.NoopVolume{}) + if _, err := s.Create("fake1", "fake", nil); err != nil { + t.Fatal(err) + } + if _, err := s.Create("fake2", "fake", nil); err != nil { + t.Fatal(err) + } + if _, err := s.Create("fake3", "noop", nil); err != nil { + t.Fatal(err) + } - if l := s.FilterByDriver("fake"); len(l) != 2 { + if l, _ := s.FilterByDriver("fake"); len(l) != 2 { t.Fatalf("Expected 2 volumes, got %v, %v", len(l), l) } - if l := s.FilterByDriver("noop"); len(l) != 1 { + if l, _ := s.FilterByDriver("noop"); len(l) != 1 { t.Fatalf("Expected 1 volume, got %v, %v", len(l), l) } } diff --git a/volume/testutils/testutils.go b/volume/testutils/testutils.go index e1c75e918a..46ada378c7 100644 --- a/volume/testutils/testutils.go +++ b/volume/testutils/testutils.go @@ -50,19 +50,55 @@ func (FakeVolume) Mount() (string, error) { return "fake", nil } func (FakeVolume) Unmount() error { return nil } // FakeDriver is a driver that generates fake volumes -type FakeDriver struct{} +type FakeDriver struct { + name string + vols map[string]volume.Volume +} + +// NewFakeDriver creates a new FakeDriver with the specified name +func NewFakeDriver(name string) volume.Driver { + return &FakeDriver{ + name: name, + vols: make(map[string]volume.Volume), + } +} // Name is the name of the driver -func (FakeDriver) Name() string { return "fake" } +func (d *FakeDriver) Name() string { return d.name } // Create initializes a fake volume. // It returns an error if the options include an "error" key with a message -func (FakeDriver) Create(name string, opts map[string]string) (volume.Volume, error) { +func (d *FakeDriver) Create(name string, opts map[string]string) (volume.Volume, error) { if opts != nil && opts["error"] != "" { return nil, fmt.Errorf(opts["error"]) } - return NewFakeVolume(name), nil + v := NewFakeVolume(name) + d.vols[name] = v + return v, nil } // Remove deletes a volume. -func (FakeDriver) Remove(v volume.Volume) error { return nil } +func (d *FakeDriver) Remove(v volume.Volume) error { + if _, exists := d.vols[v.Name()]; !exists { + return fmt.Errorf("no such volume") + } + delete(d.vols, v.Name()) + return nil +} + +// List lists the volumes +func (d *FakeDriver) List() ([]volume.Volume, error) { + var vols []volume.Volume + for _, v := range d.vols { + vols = append(vols, v) + } + return vols, nil +} + +// Get gets the volume +func (d *FakeDriver) Get(name string) (volume.Volume, error) { + if v, exists := d.vols[name]; exists { + return v, nil + } + return nil, fmt.Errorf("no such volume") +} diff --git a/volume/volume.go b/volume/volume.go index edd8160fae..0044430a64 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -21,7 +21,11 @@ type Driver interface { // Create makes a new volume with the given id. Create(name string, opts map[string]string) (Volume, error) // Remove deletes the volume. - Remove(Volume) error + Remove(vol Volume) (err error) + // List lists all the volumes the driver has + List() ([]Volume, error) + // Get retreives the volume with the requested name + Get(name string) (Volume, error) } // Volume is a place to store data. It is backed by a specific driver, and can be mounted.