Commit c6191126 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #35616 from pospispa/85-refactor-newRecyclerFunc-from-volume-plugins

Automatic merge from submit-queue Simplifies NFS and hostPath plugin code Simplifies NFS and hostPath plugin code. cc: @jsafrane
parents 4dbc532c dc9bb87a
...@@ -34,22 +34,15 @@ import ( ...@@ -34,22 +34,15 @@ import (
func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin { func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin {
return []volume.VolumePlugin{ return []volume.VolumePlugin{
&hostPathPlugin{ &hostPathPlugin{
host: nil, host: nil,
newRecyclerFunc: newRecycler, config: volumeConfig,
newDeleterFunc: newDeleter,
newProvisionerFunc: newProvisioner,
config: volumeConfig,
}, },
} }
} }
type hostPathPlugin struct { type hostPathPlugin struct {
host volume.VolumeHost host volume.VolumeHost
// decouple creating Recyclers/Deleters/Provisioners by deferring to a function. Allows for easier testing. config volume.VolumeConfig
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
} }
var _ volume.VolumePlugin = &hostPathPlugin{} var _ volume.VolumePlugin = &hostPathPlugin{}
...@@ -113,18 +106,18 @@ func (plugin *hostPathPlugin) NewUnmounter(volName string, podUID types.UID) (vo ...@@ -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) { 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) { 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) { func (plugin *hostPathPlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) {
if !plugin.config.ProvisioningEnabled { if !plugin.config.ProvisioningEnabled {
return nil, fmt.Errorf("Provisioning in volume plugin %q is disabled", plugin.GetPluginName()) 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) { func (plugin *hostPathPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
......
...@@ -71,7 +71,7 @@ func TestGetAccessModes(t *testing.T) { ...@@ -71,7 +71,7 @@ func TestGetAccessModes(t *testing.T) {
func TestRecycler(t *testing.T) { func TestRecycler(t *testing.T) {
plugMgr := volume.VolumePluginMgr{} plugMgr := volume.VolumePluginMgr{}
pluginHost := volumetest.NewFakeVolumeHost("/tmp/fake", nil, nil) 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"}}}}} spec := &volume.Spec{PersistentVolume: &api.PersistentVolume{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{HostPath: &api.HostPathVolumeSource{Path: "/foo"}}}}}
plug, err := plugMgr.FindRecyclablePluginBySpec(spec) plug, err := plugMgr.FindRecyclablePluginBySpec(spec)
...@@ -85,9 +85,6 @@ func TestRecycler(t *testing.T) { ...@@ -85,9 +85,6 @@ func TestRecycler(t *testing.T) {
if recycler.GetPath() != spec.PersistentVolume.Spec.HostPath.Path { if recycler.GetPath() != spec.PersistentVolume.Spec.HostPath.Path {
t.Errorf("Expected %s but got %s", spec.PersistentVolume.Spec.HostPath.Path, recycler.GetPath()) 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) { func TestDeleter(t *testing.T) {
......
...@@ -36,18 +36,15 @@ import ( ...@@ -36,18 +36,15 @@ import (
func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin { func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin {
return []volume.VolumePlugin{ return []volume.VolumePlugin{
&nfsPlugin{ &nfsPlugin{
host: nil, host: nil,
newRecyclerFunc: newRecycler, config: volumeConfig,
config: volumeConfig,
}, },
} }
} }
type nfsPlugin struct { type nfsPlugin struct {
host volume.VolumeHost host volume.VolumeHost
// decouple creating recyclers by deferring to a function. Allows for easier testing. config volume.VolumeConfig
newRecyclerFunc func(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error)
config volume.VolumeConfig
} }
var _ volume.VolumePlugin = &nfsPlugin{} var _ volume.VolumePlugin = &nfsPlugin{}
...@@ -133,7 +130,7 @@ func (plugin *nfsPlugin) newUnmounterInternal(volName string, podUID types.UID, ...@@ -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) { 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) { func (plugin *nfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
...@@ -154,8 +151,6 @@ type nfs struct { ...@@ -154,8 +151,6 @@ type nfs struct {
pod *api.Pod pod *api.Pod
mounter mount.Interface mounter mount.Interface
plugin *nfsPlugin 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 volume.MetricsNil
} }
......
...@@ -84,7 +84,7 @@ func TestRecycler(t *testing.T) { ...@@ -84,7 +84,7 @@ func TestRecycler(t *testing.T) {
defer os.RemoveAll(tmpDir) defer os.RemoveAll(tmpDir)
plugMgr := volume.VolumePluginMgr{} 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"}}}}} spec := &volume.Spec{PersistentVolume: &api.PersistentVolume{Spec: api.PersistentVolumeSpec{PersistentVolumeSource: api.PersistentVolumeSource{NFS: &api.NFSVolumeSource{Path: "/foo"}}}}}
plug, err := plugMgr.FindRecyclablePluginBySpec(spec) plug, err := plugMgr.FindRecyclablePluginBySpec(spec)
...@@ -98,15 +98,6 @@ func TestRecycler(t *testing.T) { ...@@ -98,15 +98,6 @@ func TestRecycler(t *testing.T) {
if recycler.GetPath() != spec.PersistentVolume.Spec.NFS.Path { if recycler.GetPath() != spec.PersistentVolume.Spec.NFS.Path {
t.Errorf("Expected %s but got %s", spec.PersistentVolume.Spec.NFS.Path, recycler.GetPath()) 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 { type mockRecycler struct {
......
...@@ -450,15 +450,6 @@ func (fr *fakeRecycler) GetPath() string { ...@@ -450,15 +450,6 @@ func (fr *fakeRecycler) GetPath() string {
return fr.path 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 { type FakeDeleter struct {
path string path string
MetricsNil MetricsNil
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment