From 977109d808ae94eb3931ae920338b1aa669f627e Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 19 Mar 2018 17:18:52 -0400 Subject: [PATCH] Remove use of global volume driver store Instead of using a global store for volume drivers, scope the driver store to the caller (e.g. the volume store). This makes testing much simpler. Signed-off-by: Brian Goff --- daemon/daemon.go | 10 +- daemon/daemon_test.go | 7 +- daemon/info.go | 3 +- volume/drivers/extpoint.go | 157 ++++++++++++--------------- volume/drivers/extpoint_test.go | 7 +- volume/store/restore.go | 5 +- volume/store/restore_test.go | 8 +- volume/store/store.go | 54 ++++++---- volume/store/store_test.go | 181 ++++++++++++-------------------- 9 files changed, 187 insertions(+), 245 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 13edeb8956..9812c0be6c 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1145,17 +1145,15 @@ func setDefaultMtu(conf *config.Config) { } func (daemon *Daemon) configureVolumes(rootIDs idtools.IDPair) (*store.VolumeStore, error) { - volumesDriver, err := local.New(daemon.configStore.Root, rootIDs) + volumeDriver, err := local.New(daemon.configStore.Root, rootIDs) if err != nil { return nil, err } - - volumedrivers.RegisterPluginGetter(daemon.PluginStore) - - if !volumedrivers.Register(volumesDriver, volumesDriver.Name()) { + drivers := volumedrivers.NewStore(daemon.PluginStore) + if !drivers.Register(volumeDriver, volumeDriver.Name()) { return nil, errors.New("local volume driver could not be registered") } - return store.New(daemon.configStore.Root) + return store.New(daemon.configStore.Root, drivers) } // IsShuttingDown tells whether the daemon is shutting down or not diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 2fb4ff902a..2fe4276d7a 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -13,7 +13,6 @@ import ( _ "github.com/docker/docker/pkg/discovery/memory" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/truncindex" - "github.com/docker/docker/volume" volumedrivers "github.com/docker/docker/volume/drivers" "github.com/docker/docker/volume/local" "github.com/docker/docker/volume/store" @@ -121,7 +120,8 @@ func initDaemonWithVolumeStore(tmp string) (*Daemon, error) { repository: tmp, root: tmp, } - daemon.volumes, err = store.New(tmp) + drivers := volumedrivers.NewStore(nil) + daemon.volumes, err = store.New(tmp, drivers) if err != nil { return nil, err } @@ -130,7 +130,7 @@ func initDaemonWithVolumeStore(tmp string) (*Daemon, error) { if err != nil { return nil, err } - volumedrivers.Register(volumesDriver, volumesDriver.Name()) + drivers.Register(volumesDriver, volumesDriver.Name()) return daemon, nil } @@ -208,7 +208,6 @@ func TestContainerInitDNS(t *testing.T) { if err != nil { t.Fatal(err) } - defer volumedrivers.Unregister(volume.DefaultDriverName) c, err := daemon.load(containerID) if err != nil { diff --git a/daemon/info.go b/daemon/info.go index 6f5ae69896..7b011fe324 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -19,7 +19,6 @@ import ( "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/pkg/system" "github.com/docker/docker/registry" - "github.com/docker/docker/volume/drivers" "github.com/docker/go-connections/sockets" "github.com/sirupsen/logrus" ) @@ -196,7 +195,7 @@ func (daemon *Daemon) SystemVersion() types.Version { func (daemon *Daemon) showPluginsInfo() types.PluginsInfo { var pluginsInfo types.PluginsInfo - pluginsInfo.Volume = volumedrivers.GetDriverList() + pluginsInfo.Volume = daemon.volumes.GetDriverList() pluginsInfo.Network = daemon.GetNetworkDriverList() // The authorization plugins are returned in the order they are // used as they constitute a request/response modification chain. diff --git a/volume/drivers/extpoint.go b/volume/drivers/extpoint.go index 33346e18d6..14e3b4f625 100644 --- a/volume/drivers/extpoint.go +++ b/volume/drivers/extpoint.go @@ -7,6 +7,7 @@ import ( "sort" "sync" + "github.com/docker/docker/errdefs" "github.com/docker/docker/pkg/locker" getter "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/volume" @@ -14,14 +15,6 @@ import ( "github.com/sirupsen/logrus" ) -// currently created by hand. generation tool would generate this like: -// $ extpoint-gen Driver > volume/extpoint.go - -var drivers = &driverExtpoint{ - extensions: make(map[string]volume.Driver), - driverLock: &locker.Locker{}, -} - const extName = "VolumeDriver" // NewVolumeDriver returns a driver has the given name mapped on the given client. @@ -53,53 +46,21 @@ type volumeDriver interface { Capabilities() (capabilities volume.Capability, err error) } -type driverExtpoint struct { - extensions map[string]volume.Driver - sync.Mutex +// Store is an in-memory store for volume drivers +type Store struct { + extensions map[string]volume.Driver + mu sync.Mutex driverLock *locker.Locker - plugingetter getter.PluginGetter + pluginGetter getter.PluginGetter } -// RegisterPluginGetter sets the plugingetter -func RegisterPluginGetter(plugingetter getter.PluginGetter) { - drivers.plugingetter = plugingetter -} - -// Register associates the given driver to the given name, checking if -// the name is already associated -func Register(extension volume.Driver, name string) bool { - if name == "" { - return false +// NewStore creates a new volume driver store +func NewStore(pg getter.PluginGetter) *Store { + return &Store{ + extensions: make(map[string]volume.Driver), + driverLock: locker.New(), + pluginGetter: pg, } - - drivers.Lock() - defer drivers.Unlock() - - _, exists := drivers.extensions[name] - if exists { - return false - } - - if err := validateDriver(extension); err != nil { - return false - } - - drivers.extensions[name] = extension - - return true -} - -// Unregister dissociates the name from its driver, if the association exists. -func Unregister(name string) bool { - drivers.Lock() - defer drivers.Unlock() - - _, exists := drivers.extensions[name] - if !exists { - return false - } - delete(drivers.extensions, name) - return true } type driverNotFoundError string @@ -113,18 +74,21 @@ func (driverNotFoundError) NotFound() {} // lookup returns the driver associated with the given name. If a // driver with the given name has not been registered it checks if // there is a VolumeDriver plugin available with the given name. -func lookup(name string, mode int) (volume.Driver, error) { - drivers.driverLock.Lock(name) - defer drivers.driverLock.Unlock(name) +func (s *Store) lookup(name string, mode int) (volume.Driver, error) { + if name == "" { + return nil, errdefs.InvalidParameter(errors.New("driver name cannot be empty")) + } + s.driverLock.Lock(name) + defer s.driverLock.Unlock(name) - drivers.Lock() - ext, ok := drivers.extensions[name] - drivers.Unlock() + s.mu.Lock() + ext, ok := s.extensions[name] + s.mu.Unlock() if ok { return ext, nil } - if drivers.plugingetter != nil { - p, err := drivers.plugingetter.Get(name, extName, mode) + if s.pluginGetter != nil { + p, err := s.pluginGetter.Get(name, extName, mode) if err != nil { return nil, errors.Wrap(err, "error looking up volume plugin "+name) } @@ -133,7 +97,7 @@ func lookup(name string, mode int) (volume.Driver, error) { if err := validateDriver(d); err != nil { if mode > 0 { // Undo any reference count changes from the initial `Get` - if _, err := drivers.plugingetter.Get(name, extName, mode*-1); err != nil { + if _, err := s.pluginGetter.Get(name, extName, mode*-1); err != nil { logrus.WithError(err).WithField("action", "validate-driver").WithField("plugin", name).Error("error releasing reference to plugin") } } @@ -141,9 +105,9 @@ func lookup(name string, mode int) (volume.Driver, error) { } if p.IsV1() { - drivers.Lock() - drivers.extensions[name] = d - drivers.Unlock() + s.mu.Lock() + s.extensions[name] = d + s.mu.Unlock() } return d, nil } @@ -158,75 +122,88 @@ func validateDriver(vd volume.Driver) error { return nil } +// Register associates the given driver to the given name, checking if +// the name is already associated +func (s *Store) Register(d volume.Driver, name string) bool { + if name == "" { + return false + } + + s.mu.Lock() + defer s.mu.Unlock() + + if _, exists := s.extensions[name]; exists { + return false + } + + if err := validateDriver(d); err != nil { + return false + } + + s.extensions[name] = d + return true +} + // GetDriver returns a volume driver by its name. // If the driver is empty, it looks for the local driver. -func GetDriver(name string) (volume.Driver, error) { - if name == "" { - name = volume.DefaultDriverName - } - return lookup(name, getter.Lookup) +func (s *Store) GetDriver(name string) (volume.Driver, error) { + return s.lookup(name, getter.Lookup) } // CreateDriver returns a volume driver by its name and increments RefCount. // If the driver is empty, it looks for the local driver. -func CreateDriver(name string) (volume.Driver, error) { - if name == "" { - name = volume.DefaultDriverName - } - return lookup(name, getter.Acquire) +func (s *Store) CreateDriver(name string) (volume.Driver, error) { + return s.lookup(name, getter.Acquire) } // ReleaseDriver returns a volume driver by its name and decrements RefCount.. // If the driver is empty, it looks for the local driver. -func ReleaseDriver(name string) (volume.Driver, error) { - if name == "" { - name = volume.DefaultDriverName - } - return lookup(name, getter.Release) +func (s *Store) ReleaseDriver(name string) (volume.Driver, error) { + return s.lookup(name, getter.Release) } // GetDriverList returns list of volume drivers registered. // If no driver is registered, empty string list will be returned. -func GetDriverList() []string { +func (s *Store) GetDriverList() []string { var driverList []string - drivers.Lock() - for driverName := range drivers.extensions { + s.mu.Lock() + for driverName := range s.extensions { driverList = append(driverList, driverName) } - drivers.Unlock() + s.mu.Unlock() sort.Strings(driverList) return driverList } // GetAllDrivers lists all the registered drivers -func GetAllDrivers() ([]volume.Driver, error) { +func (s *Store) GetAllDrivers() ([]volume.Driver, error) { var plugins []getter.CompatPlugin - if drivers.plugingetter != nil { + if s.pluginGetter != nil { var err error - plugins, err = drivers.plugingetter.GetAllByCap(extName) + plugins, err = s.pluginGetter.GetAllByCap(extName) if err != nil { return nil, fmt.Errorf("error listing plugins: %v", err) } } var ds []volume.Driver - drivers.Lock() - defer drivers.Unlock() + s.mu.Lock() + defer s.mu.Unlock() - for _, d := range drivers.extensions { + for _, d := range s.extensions { ds = append(ds, d) } for _, p := range plugins { name := p.Name() - if _, ok := drivers.extensions[name]; ok { + if _, ok := s.extensions[name]; ok { continue } ext := NewVolumeDriver(name, p.ScopedPath, p.Client()) if p.IsV1() { - drivers.extensions[name] = ext + s.extensions[name] = ext } ds = append(ds, ext) } diff --git a/volume/drivers/extpoint_test.go b/volume/drivers/extpoint_test.go index 12b4ee12d9..384742ea00 100644 --- a/volume/drivers/extpoint_test.go +++ b/volume/drivers/extpoint_test.go @@ -7,13 +7,14 @@ import ( ) func TestGetDriver(t *testing.T) { - _, err := GetDriver("missing") + s := NewStore(nil) + _, err := s.GetDriver("missing") if err == nil { t.Fatal("Expected error, was nil") } - Register(volumetestutils.NewFakeDriver("fake"), "fake") + s.Register(volumetestutils.NewFakeDriver("fake"), "fake") - d, err := GetDriver("fake") + d, err := s.GetDriver("fake") if err != nil { t.Fatal(err) } diff --git a/volume/store/restore.go b/volume/store/restore.go index 790a5b65f7..2e072ec087 100644 --- a/volume/store/restore.go +++ b/volume/store/restore.go @@ -5,7 +5,6 @@ import ( "github.com/boltdb/bolt" "github.com/docker/docker/volume" - "github.com/docker/docker/volume/drivers" "github.com/sirupsen/logrus" ) @@ -33,7 +32,7 @@ func (s *VolumeStore) restore() { var v volume.Volume var err error if meta.Driver != "" { - v, err = lookupVolume(meta.Driver, meta.Name) + v, err = lookupVolume(s.drivers, meta.Driver, meta.Name) if err != nil && err != errNoSuchVolume { logrus.WithError(err).WithField("driver", meta.Driver).WithField("volume", meta.Name).Warn("Error restoring volume") return @@ -59,7 +58,7 @@ func (s *VolumeStore) restore() { } // increment driver refcount - volumedrivers.CreateDriver(meta.Driver) + s.drivers.CreateDriver(meta.Driver) // cache the volume s.globalLock.Lock() diff --git a/volume/store/restore_test.go b/volume/store/restore_test.go index 680735a384..5c3c6df72c 100644 --- a/volume/store/restore_test.go +++ b/volume/store/restore_test.go @@ -18,11 +18,11 @@ func TestRestore(t *testing.T) { assert.NilError(t, err) defer os.RemoveAll(dir) + drivers := volumedrivers.NewStore(nil) driverName := "test-restore" - volumedrivers.Register(volumetestutils.NewFakeDriver(driverName), driverName) - defer volumedrivers.Unregister("test-restore") + drivers.Register(volumetestutils.NewFakeDriver(driverName), driverName) - s, err := New(dir) + s, err := New(dir, drivers) assert.NilError(t, err) defer s.Shutdown() @@ -36,7 +36,7 @@ func TestRestore(t *testing.T) { s.Shutdown() - s, err = New(dir) + s, err = New(dir, drivers) assert.NilError(t, err) v, err := s.Get("test1") diff --git a/volume/store/store.go b/volume/store/store.go index 70dd8b2d89..4f4cffafd7 100644 --- a/volume/store/store.go +++ b/volume/store/store.go @@ -66,13 +66,14 @@ func (v volumeWrapper) CachedPath() string { // New initializes a VolumeStore to keep // reference counting of volumes in the system. -func New(rootPath string) (*VolumeStore, error) { +func New(rootPath string, drivers *drivers.Store) (*VolumeStore, error) { vs := &VolumeStore{ locks: &locker.Locker{}, names: make(map[string]volume.Volume), refs: make(map[string]map[string]struct{}), labels: make(map[string]map[string]string), options: make(map[string]map[string]string), + drivers: drivers, } if rootPath != "" { @@ -157,7 +158,7 @@ func (s *VolumeStore) Purge(name string) { v, exists := s.names[name] if exists { driverName := v.DriverName() - if _, err := volumedrivers.ReleaseDriver(driverName); err != nil { + if _, err := s.drivers.ReleaseDriver(driverName); err != nil { logrus.WithError(err).WithField("driver", driverName).Error("Error releasing reference to volume driver") } } @@ -175,7 +176,8 @@ func (s *VolumeStore) Purge(name string) { type VolumeStore struct { // locks ensures that only one action is being performed on a particular volume at a time without locking the entire store // since actions on volumes can be quite slow, this ensures the store is free to handle requests for other volumes. - locks *locker.Locker + locks *locker.Locker + drivers *drivers.Store // globalLock is used to protect access to mutable structures used by the store object globalLock sync.RWMutex // names stores the volume name -> volume relationship. @@ -226,7 +228,7 @@ func (s *VolumeStore) list() ([]volume.Volume, []string, error) { warnings []string ) - drivers, err := volumedrivers.GetAllDrivers() + drivers, err := s.drivers.GetAllDrivers() if err != nil { return nil, nil, err } @@ -329,7 +331,7 @@ func (s *VolumeStore) checkConflict(name, driverName string) (volume.Volume, err if driverName != "" { // Retrieve canonical driver name to avoid inconsistencies (for example // "plugin" vs. "plugin:latest") - vd, err := volumedrivers.GetDriver(driverName) + vd, err := s.drivers.GetDriver(driverName) if err != nil { return nil, err } @@ -341,7 +343,7 @@ func (s *VolumeStore) checkConflict(name, driverName string) (volume.Volume, err // let's check if the found volume ref // is stale by checking with the driver if it still exists - exists, err := volumeExists(v) + exists, err := volumeExists(s.drivers, v) if err != nil { return nil, errors.Wrapf(errNameConflict, "found reference to volume '%s' in driver '%s', but got an error while checking the driver: %v", name, vDriverName, err) } @@ -366,8 +368,8 @@ func (s *VolumeStore) checkConflict(name, driverName string) (volume.Volume, err // volumeExists returns if the volume is still present in the driver. // An error is returned if there was an issue communicating with the driver. -func volumeExists(v volume.Volume) (bool, error) { - exists, err := lookupVolume(v.DriverName(), v.Name()) +func volumeExists(store *drivers.Store, v volume.Volume) (bool, error) { + exists, err := lookupVolume(store, v.DriverName(), v.Name()) if err != nil { return false, err } @@ -412,7 +414,10 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st } } - vd, err := volumedrivers.CreateDriver(driverName) + if driverName == "" { + driverName = volume.DefaultDriverName + } + vd, err := s.drivers.CreateDriver(driverName) if err != nil { return nil, &OpErr{Op: "create", Name: name, Err: err} } @@ -421,7 +426,7 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st if v, _ = vd.Get(name); v == nil { v, err = vd.Create(name, opts) if err != nil { - if _, err := volumedrivers.ReleaseDriver(driverName); err != nil { + if _, err := s.drivers.ReleaseDriver(driverName); err != nil { logrus.WithError(err).WithField("driver", driverName).Error("Error releasing reference to volume driver") } return nil, err @@ -455,7 +460,10 @@ func (s *VolumeStore) GetWithRef(name, driverName, ref string) (volume.Volume, e s.locks.Lock(name) defer s.locks.Unlock(name) - vd, err := volumedrivers.GetDriver(driverName) + if driverName == "" { + driverName = volume.DefaultDriverName + } + vd, err := s.drivers.GetDriver(driverName) if err != nil { return nil, &OpErr{Err: err, Name: name, Op: "get"} } @@ -510,7 +518,7 @@ func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { } if meta.Driver != "" { - vol, err := lookupVolume(meta.Driver, name) + vol, err := lookupVolume(s.drivers, meta.Driver, name) if err != nil { return nil, err } @@ -520,7 +528,7 @@ func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { } var scope string - vd, err := volumedrivers.GetDriver(meta.Driver) + vd, err := s.drivers.GetDriver(meta.Driver) if err == nil { scope = vd.Scope() } @@ -528,7 +536,7 @@ func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { } logrus.Debugf("Probing all drivers for volume with name: %s", name) - drivers, err := volumedrivers.GetAllDrivers() + drivers, err := s.drivers.GetAllDrivers() if err != nil { return nil, err } @@ -552,8 +560,11 @@ func (s *VolumeStore) getVolume(name string) (volume.Volume, error) { // If the driver returns an error that is not communication related the // error is logged but not returned. // If the volume is not found it will return `nil, nil`` -func lookupVolume(driverName, volumeName string) (volume.Volume, error) { - vd, err := volumedrivers.GetDriver(driverName) +func lookupVolume(store *drivers.Store, driverName, volumeName string) (volume.Volume, error) { + if driverName == "" { + driverName = volume.DefaultDriverName + } + vd, err := store.GetDriver(driverName) if err != nil { return nil, errors.Wrapf(err, "error while checking if volume %q exists in driver %q", volumeName, driverName) } @@ -585,7 +596,7 @@ func (s *VolumeStore) Remove(v volume.Volume) error { return &OpErr{Err: errVolumeInUse, Name: v.Name(), Op: "remove", Refs: s.getRefs(name)} } - vd, err := volumedrivers.GetDriver(v.DriverName()) + vd, err := s.drivers.GetDriver(v.DriverName()) if err != nil { return &OpErr{Err: err, Name: v.DriverName(), Op: "remove"} } @@ -627,7 +638,7 @@ func (s *VolumeStore) Refs(v volume.Volume) []string { // FilterByDriver returns the available volumes filtered by driver name func (s *VolumeStore) FilterByDriver(name string) ([]volume.Volume, error) { - vd, err := volumedrivers.GetDriver(name) + vd, err := s.drivers.GetDriver(name) if err != nil { return nil, &OpErr{Err: err, Name: name, Op: "list"} } @@ -686,3 +697,10 @@ func unwrapVolume(v volume.Volume) volume.Volume { func (s *VolumeStore) Shutdown() error { return s.db.Close() } + +// GetDriverList gets the list of volume drivers from the configured volume driver +// store. +// TODO(@cpuguy83): This should be factored out into a separate service. +func (s *VolumeStore) GetDriverList() []string { + return s.drivers.GetDriverList() +} diff --git a/volume/store/store_test.go b/volume/store/store_test.go index faf4035e29..288a4ce824 100644 --- a/volume/store/store_test.go +++ b/volume/store/store_test.go @@ -10,7 +10,7 @@ import ( "testing" "github.com/docker/docker/volume" - "github.com/docker/docker/volume/drivers" + volumedrivers "github.com/docker/docker/volume/drivers" volumetestutils "github.com/docker/docker/volume/testutils" "github.com/google/go-cmp/cmp" "github.com/gotestyourself/gotestyourself/assert" @@ -18,18 +18,12 @@ import ( ) func TestCreate(t *testing.T) { - volumedrivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") - defer volumedrivers.Unregister("fake") - dir, err := ioutil.TempDir("", "test-create") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + t.Parallel() + + s, cleanup := setupTest(t) + defer cleanup() + s.drivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") - s, err := New(dir) - if err != nil { - t.Fatal(err) - } v, err := s.Create("fake1", "fake", nil, nil) if err != nil { t.Fatal(err) @@ -53,19 +47,13 @@ func TestCreate(t *testing.T) { } func TestRemove(t *testing.T) { - volumedrivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") - volumedrivers.Register(volumetestutils.NewFakeDriver("noop"), "noop") - defer volumedrivers.Unregister("fake") - defer volumedrivers.Unregister("noop") - dir, err := ioutil.TempDir("", "test-remove") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) - s, err := New(dir) - if err != nil { - t.Fatal(err) - } + t.Parallel() + + s, cleanup := setupTest(t) + defer cleanup() + + s.drivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") + s.drivers.Register(volumetestutils.NewFakeDriver("noop"), "noop") // doing string compare here since this error comes directly from the driver expected := "no such volume" @@ -91,20 +79,19 @@ func TestRemove(t *testing.T) { } func TestList(t *testing.T) { - volumedrivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") - volumedrivers.Register(volumetestutils.NewFakeDriver("fake2"), "fake2") - defer volumedrivers.Unregister("fake") - defer volumedrivers.Unregister("fake2") + t.Parallel() + dir, err := ioutil.TempDir("", "test-list") - if err != nil { - t.Fatal(err) - } + assert.NilError(t, err) defer os.RemoveAll(dir) - s, err := New(dir) - if err != nil { - t.Fatal(err) - } + drivers := volumedrivers.NewStore(nil) + drivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") + drivers.Register(volumetestutils.NewFakeDriver("fake2"), "fake2") + + s, err := New(dir, drivers) + assert.NilError(t, err) + if _, err := s.Create("test", "fake", nil, nil); err != nil { t.Fatal(err) } @@ -124,7 +111,7 @@ func TestList(t *testing.T) { } // and again with a new store - s, err = New(dir) + s, err = New(dir, drivers) if err != nil { t.Fatal(err) } @@ -138,18 +125,12 @@ func TestList(t *testing.T) { } func TestFilterByDriver(t *testing.T) { - volumedrivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") - volumedrivers.Register(volumetestutils.NewFakeDriver("noop"), "noop") - defer volumedrivers.Unregister("fake") - defer volumedrivers.Unregister("noop") - dir, err := ioutil.TempDir("", "test-filter-driver") - if err != nil { - t.Fatal(err) - } - s, err := New(dir) - if err != nil { - t.Fatal(err) - } + t.Parallel() + s, cleanup := setupTest(t) + defer cleanup() + + s.drivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") + s.drivers.Register(volumetestutils.NewFakeDriver("noop"), "noop") if _, err := s.Create("fake1", "fake", nil, nil); err != nil { t.Fatal(err) @@ -171,17 +152,12 @@ func TestFilterByDriver(t *testing.T) { } func TestFilterByUsed(t *testing.T) { - volumedrivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") - volumedrivers.Register(volumetestutils.NewFakeDriver("noop"), "noop") - dir, err := ioutil.TempDir("", "test-filter-used") - if err != nil { - t.Fatal(err) - } + t.Parallel() + s, cleanup := setupTest(t) + defer cleanup() - s, err := New(dir) - if err != nil { - t.Fatal(err) - } + s.drivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") + s.drivers.Register(volumetestutils.NewFakeDriver("noop"), "noop") if _, err := s.CreateWithRef("fake1", "fake", "volReference", nil, nil); err != nil { t.Fatal(err) @@ -213,16 +189,10 @@ func TestFilterByUsed(t *testing.T) { } func TestDerefMultipleOfSameRef(t *testing.T) { - volumedrivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") - dir, err := ioutil.TempDir("", "test-same-deref") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) - s, err := New(dir) - if err != nil { - t.Fatal(err) - } + t.Parallel() + s, cleanup := setupTest(t) + defer cleanup() + s.drivers.Register(volumetestutils.NewFakeDriver("fake"), "fake") v, err := s.CreateWithRef("fake1", "fake", "volReference", nil, nil) if err != nil { @@ -240,17 +210,12 @@ func TestDerefMultipleOfSameRef(t *testing.T) { } func TestCreateKeepOptsLabelsWhenExistsRemotely(t *testing.T) { + t.Parallel() + s, cleanup := setupTest(t) + defer cleanup() + vd := volumetestutils.NewFakeDriver("fake") - volumedrivers.Register(vd, "fake") - dir, err := ioutil.TempDir("", "test-same-deref") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) - s, err := New(dir) - if err != nil { - t.Fatal(err) - } + s.drivers.Register(vd, "fake") // Create a volume in the driver directly if _, err := vd.Create("foo", nil); err != nil { @@ -273,6 +238,8 @@ func TestCreateKeepOptsLabelsWhenExistsRemotely(t *testing.T) { } func TestDefererencePluginOnCreateError(t *testing.T) { + t.Parallel() + var ( l net.Listener err error @@ -286,6 +253,9 @@ func TestDefererencePluginOnCreateError(t *testing.T) { } defer l.Close() + s, cleanup := setupTest(t) + defer cleanup() + d := volumetestutils.NewFakeDriver("TestDefererencePluginOnCreateError") p, err := volumetestutils.MakeFakePlugin(d, l) if err != nil { @@ -293,19 +263,7 @@ func TestDefererencePluginOnCreateError(t *testing.T) { } pg := volumetestutils.NewFakePluginGetter(p) - volumedrivers.RegisterPluginGetter(pg) - defer volumedrivers.RegisterPluginGetter(nil) - - dir, err := ioutil.TempDir("", "test-plugin-deref-err") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) - - s, err := New(dir) - if err != nil { - t.Fatal(err) - } + s.drivers = volumedrivers.NewStore(pg) // create a good volume so we have a plugin reference _, err = s.Create("fake1", d.Name(), nil, nil) @@ -329,8 +287,9 @@ func TestRefDerefRemove(t *testing.T) { t.Parallel() driverName := "test-ref-deref-remove" - s, cleanup := setupTest(t, driverName) - defer cleanup(t) + s, cleanup := setupTest(t) + defer cleanup() + s.drivers.Register(volumetestutils.NewFakeDriver(driverName), driverName) v, err := s.CreateWithRef("test", driverName, "test-ref", nil, nil) assert.NilError(t, err) @@ -348,8 +307,9 @@ func TestGet(t *testing.T) { t.Parallel() driverName := "test-get" - s, cleanup := setupTest(t, driverName) - defer cleanup(t) + s, cleanup := setupTest(t) + defer cleanup() + s.drivers.Register(volumetestutils.NewFakeDriver(driverName), driverName) _, err := s.Get("not-exist") assert.Assert(t, is.ErrorContains(err, "")) @@ -373,8 +333,9 @@ func TestGetWithRef(t *testing.T) { t.Parallel() driverName := "test-get-with-ref" - s, cleanup := setupTest(t, driverName) - defer cleanup(t) + s, cleanup := setupTest(t) + defer cleanup() + s.drivers.Register(volumetestutils.NewFakeDriver(driverName), driverName) _, err := s.GetWithRef("not-exist", driverName, "test-ref") assert.Assert(t, is.ErrorContains(err, "")) @@ -397,32 +358,22 @@ func TestGetWithRef(t *testing.T) { var cmpVolume = cmp.AllowUnexported(volumetestutils.FakeVolume{}, volumeWrapper{}) -func setupTest(t *testing.T, name string) (*VolumeStore, func(*testing.T)) { - t.Helper() - s, cleanup := newTestStore(t) - - volumedrivers.Register(volumetestutils.NewFakeDriver(name), name) - return s, func(t *testing.T) { - cleanup(t) - volumedrivers.Unregister(name) - } -} - -func newTestStore(t *testing.T) (*VolumeStore, func(*testing.T)) { +func setupTest(t *testing.T) (*VolumeStore, func()) { t.Helper() - dir, err := ioutil.TempDir("", "store-root") + dirName := strings.Replace(t.Name(), string(os.PathSeparator), "_", -1) + dir, err := ioutil.TempDir("", dirName) assert.NilError(t, err) - cleanup := func(t *testing.T) { + cleanup := func() { err := os.RemoveAll(dir) assert.Check(t, err) } - s, err := New(dir) + s, err := New(dir, volumedrivers.NewStore(nil)) assert.Check(t, err) - return s, func(t *testing.T) { + return s, func() { s.Shutdown() - cleanup(t) + cleanup() } }