From dc9bb87ac72c468b72910ba4108d86e2d9dd7a4d Mon Sep 17 00:00:00 2001 From: pospispa Date: Tue, 25 Oct 2016 12:36:49 +0200 Subject: [PATCH] Simplifies NFS and Host Path Plugin - Removed newRecyclerFunc, newDeleterFunc and newProvisionerFunc struct hostPathPlugin contains newRecyclerFunc, newDeleterFunc and newProvisionerFunc items that have only one instance, i.e. newRecycler, newDeleter or newProvisioner function. That's why the newRecyclerFunc, newDeleterFunc and newProvisionerFunc items are removed and the newRecycler, newDeleter or newProvisioner functions are called directly. In addition, the TestRecycler tests whether NewFakeRecycler function is called and returns nil. This is no longer needed so this particular part of the test is removed. In addition, the no longer used NewFakeRecycler function is removed also. Similarly for the NFS plugin, struct nfsPlugin contains newRecyclerFunc item that has only one instance, i.e. newRecycler function. That's why the newRecyclerFunc item is removed and the newRecycler function is called directly. In addition, the TestRecycler tests whether newMockRecycler function is called and returns nil. This is no longer needed so this particular part of the test is removed. In addition, the no longer used newMockRecycler function is removed also. --- pkg/volume/host_path/host_path.go | 21 +++++++-------------- pkg/volume/host_path/host_path_test.go | 5 +---- pkg/volume/nfs/nfs.go | 15 +++++---------- pkg/volume/nfs/nfs_test.go | 11 +---------- pkg/volume/testing/testing.go | 9 --------- 5 files changed, 14 insertions(+), 47 deletions(-) diff --git a/pkg/volume/host_path/host_path.go b/pkg/volume/host_path/host_path.go index 6089d53a839..72940c62ff8 100644 --- a/pkg/volume/host_path/host_path.go +++ b/pkg/volume/host_path/host_path.go @@ -34,22 +34,15 @@ import ( func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin { return []volume.VolumePlugin{ &hostPathPlugin{ - host: nil, - newRecyclerFunc: newRecycler, - newDeleterFunc: newDeleter, - newProvisionerFunc: newProvisioner, - config: volumeConfig, + host: nil, + config: volumeConfig, }, } } type hostPathPlugin struct { - host volume.VolumeHost - // decouple creating Recyclers/Deleters/Provisioners by deferring to a function. Allows for easier testing. - newRecyclerFunc func(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error) - newDeleterFunc func(spec *volume.Spec, host volume.VolumeHost) (volume.Deleter, error) - newProvisionerFunc func(options volume.VolumeOptions, host volume.VolumeHost, plugin *hostPathPlugin) (volume.Provisioner, error) - config volume.VolumeConfig + host volume.VolumeHost + config volume.VolumeConfig } var _ volume.VolumePlugin = &hostPathPlugin{} @@ -113,18 +106,18 @@ func (plugin *hostPathPlugin) NewUnmounter(volName string, podUID types.UID) (vo } func (plugin *hostPathPlugin) NewRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder) (volume.Recycler, error) { - return plugin.newRecyclerFunc(pvName, spec, eventRecorder, plugin.host, plugin.config) + return newRecycler(pvName, spec, eventRecorder, plugin.host, plugin.config) } func (plugin *hostPathPlugin) NewDeleter(spec *volume.Spec) (volume.Deleter, error) { - return plugin.newDeleterFunc(spec, plugin.host) + return newDeleter(spec, plugin.host) } func (plugin *hostPathPlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) { if !plugin.config.ProvisioningEnabled { return nil, fmt.Errorf("Provisioning in volume plugin %q is disabled", plugin.GetPluginName()) } - return plugin.newProvisionerFunc(options, plugin.host, plugin) + return newProvisioner(options, plugin.host, plugin) } func (plugin *hostPathPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { diff --git a/pkg/volume/host_path/host_path_test.go b/pkg/volume/host_path/host_path_test.go index 042f732d6e3..342bdcad2a7 100644 --- a/pkg/volume/host_path/host_path_test.go +++ b/pkg/volume/host_path/host_path_test.go @@ -71,7 +71,7 @@ func TestGetAccessModes(t *testing.T) { func TestRecycler(t *testing.T) { plugMgr := volume.VolumePluginMgr{} pluginHost := volumetest.NewFakeVolumeHost("/tmp/fake", nil, nil) - plugMgr.InitPlugins([]volume.VolumePlugin{&hostPathPlugin{nil, volumetest.NewFakeRecycler, nil, nil, volume.VolumeConfig{}}}, pluginHost) + plugMgr.InitPlugins([]volume.VolumePlugin{&hostPathPlugin{nil, volume.VolumeConfig{}}}, pluginHost) spec := &volume.Spec{PersistentVolume: &api.PersistentVolume{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{HostPath: &api.HostPathVolumeSource{Path: "/foo"}}}}} plug, err := plugMgr.FindRecyclablePluginBySpec(spec) @@ -85,9 +85,6 @@ func TestRecycler(t *testing.T) { if recycler.GetPath() != spec.PersistentVolume.Spec.HostPath.Path { t.Errorf("Expected %s but got %s", spec.PersistentVolume.Spec.HostPath.Path, recycler.GetPath()) } - if err := recycler.Recycle(); err != nil { - t.Errorf("Mock Recycler expected to return nil but got %s", err) - } } func TestDeleter(t *testing.T) { diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go index a7eb34a000e..61c5f3db78f 100644 --- a/pkg/volume/nfs/nfs.go +++ b/pkg/volume/nfs/nfs.go @@ -36,18 +36,15 @@ import ( func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin { return []volume.VolumePlugin{ &nfsPlugin{ - host: nil, - newRecyclerFunc: newRecycler, - config: volumeConfig, + host: nil, + config: volumeConfig, }, } } type nfsPlugin struct { - host volume.VolumeHost - // decouple creating recyclers by deferring to a function. Allows for easier testing. - newRecyclerFunc func(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error) - config volume.VolumeConfig + host volume.VolumeHost + config volume.VolumeConfig } var _ volume.VolumePlugin = &nfsPlugin{} @@ -133,7 +130,7 @@ func (plugin *nfsPlugin) newUnmounterInternal(volName string, podUID types.UID, } func (plugin *nfsPlugin) NewRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder) (volume.Recycler, error) { - return plugin.newRecyclerFunc(pvName, spec, eventRecorder, plugin.host, plugin.config) + return newRecycler(pvName, spec, eventRecorder, plugin.host, plugin.config) } func (plugin *nfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { @@ -154,8 +151,6 @@ type nfs struct { pod *api.Pod mounter mount.Interface plugin *nfsPlugin - // decouple creating recyclers by deferring to a function. Allows for easier testing. - newRecyclerFunc func(spec *volume.Spec, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error) volume.MetricsNil } diff --git a/pkg/volume/nfs/nfs_test.go b/pkg/volume/nfs/nfs_test.go index 30d46e1f66a..b576033b560 100644 --- a/pkg/volume/nfs/nfs_test.go +++ b/pkg/volume/nfs/nfs_test.go @@ -84,7 +84,7 @@ func TestRecycler(t *testing.T) { defer os.RemoveAll(tmpDir) plugMgr := volume.VolumePluginMgr{} - plugMgr.InitPlugins([]volume.VolumePlugin{&nfsPlugin{nil, newMockRecycler, volume.VolumeConfig{}}}, volumetest.NewFakeVolumeHost(tmpDir, nil, nil)) + plugMgr.InitPlugins([]volume.VolumePlugin{&nfsPlugin{nil, volume.VolumeConfig{}}}, volumetest.NewFakeVolumeHost(tmpDir, nil, nil)) spec := &volume.Spec{PersistentVolume: &api.PersistentVolume{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{NFS: &api.NFSVolumeSource{Path: "/foo"}}}}} plug, err := plugMgr.FindRecyclablePluginBySpec(spec) @@ -98,15 +98,6 @@ func TestRecycler(t *testing.T) { if recycler.GetPath() != spec.PersistentVolume.Spec.NFS.Path { t.Errorf("Expected %s but got %s", spec.PersistentVolume.Spec.NFS.Path, recycler.GetPath()) } - if err := recycler.Recycle(); err != nil { - t.Errorf("Mock Recycler expected to return nil but got %s", err) - } -} - -func newMockRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, config volume.VolumeConfig) (volume.Recycler, error) { - return &mockRecycler{ - path: spec.PersistentVolume.Spec.NFS.Path, - }, nil } type mockRecycler struct { diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index eaa2aba8dc2..01f09322e03 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -450,15 +450,6 @@ func (fr *fakeRecycler) GetPath() string { return fr.path } -func NewFakeRecycler(pvName string, spec *Spec, eventRecorder RecycleEventRecorder, host VolumeHost, config VolumeConfig) (Recycler, error) { - if spec.PersistentVolume == nil || spec.PersistentVolume.Spec.HostPath == nil { - return nil, fmt.Errorf("fakeRecycler only supports spec.PersistentVolume.Spec.HostPath") - } - return &fakeRecycler{ - path: spec.PersistentVolume.Spec.HostPath.Path, - }, nil -} - type FakeDeleter struct { path string MetricsNil