diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index 2c9d5eec32..611bf09791 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/pkg/controller/volume/attachdetach/testing/testvolumespec.go @@ -310,6 +310,10 @@ func (plugin *TestPlugin) NewDetacher() (volume.Detacher, error) { return &detacher, nil } +func (plugin *TestPlugin) CanAttach(spec *volume.Spec) bool { + return true +} + func (plugin *TestPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) { return plugin.NewDetacher() } diff --git a/pkg/volume/awsebs/attacher.go b/pkg/volume/awsebs/attacher.go index 1f5eeebc56..0cf2062573 100644 --- a/pkg/volume/awsebs/attacher.go +++ b/pkg/volume/awsebs/attacher.go @@ -277,6 +277,10 @@ func (detacher *awsElasticBlockStoreDetacher) UnmountDevice(deviceMountPath stri return mount.CleanupMountPoint(deviceMountPath, detacher.mounter, false) } +func (plugin *awsElasticBlockStorePlugin) CanAttach(spec *volume.Spec) bool { + return true +} + func setNodeDisk( nodeDiskMap map[types.NodeName]map[*volume.Spec]bool, volumeSpec *volume.Spec, diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index 0a2dfa1842..b130c3ca35 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/pkg/volume/azure_dd/azure_dd.go @@ -235,6 +235,10 @@ func (plugin *azureDataDiskPlugin) NewDetacher() (volume.Detacher, error) { }, nil } +func (plugin *azureDataDiskPlugin) CanAttach(spec *volume.Spec) bool { + return true +} + func (plugin *azureDataDiskPlugin) NewDeleter(spec *volume.Spec) (volume.Deleter, error) { volumeSource, _, err := getVolumeSource(spec) if err != nil { diff --git a/pkg/volume/cinder/attacher.go b/pkg/volume/cinder/attacher.go index 06dab4fbf8..218aaeeb53 100644 --- a/pkg/volume/cinder/attacher.go +++ b/pkg/volume/cinder/attacher.go @@ -406,6 +406,10 @@ func (detacher *cinderDiskDetacher) UnmountDevice(deviceMountPath string) error return mount.CleanupMountPoint(deviceMountPath, detacher.mounter, false) } +func (plugin *cinderPlugin) CanAttach(spec *volume.Spec) bool { + return true +} + func (attacher *cinderDiskAttacher) nodeInstanceID(nodeName types.NodeName) (string, error) { instances, res := attacher.cinderProvider.Instances() if !res { diff --git a/pkg/volume/csi/csi_attacher.go b/pkg/volume/csi/csi_attacher.go index d13a731012..bd3cf1b5f1 100644 --- a/pkg/volume/csi/csi_attacher.go +++ b/pkg/volume/csi/csi_attacher.go @@ -69,16 +69,6 @@ func (c *csiAttacher) Attach(spec *volume.Spec, nodeName types.NodeName) (string return "", err } - skip, err := c.plugin.skipAttach(csiSource.Driver) - if err != nil { - klog.Error(log("attacher.Attach failed to find if driver is attachable: %v", err)) - return "", err - } - if skip { - klog.V(4).Infof(log("skipping attach for driver %s", csiSource.Driver)) - return "", nil - } - node := string(nodeName) pvName := spec.PersistentVolume.GetName() attachID := getAttachmentName(csiSource.VolumeHandle, csiSource.Driver, node) @@ -131,16 +121,6 @@ func (c *csiAttacher) WaitForAttach(spec *volume.Spec, _ string, pod *v1.Pod, ti attachID := getAttachmentName(source.VolumeHandle, source.Driver, string(c.plugin.host.GetNodeName())) - skip, err := c.plugin.skipAttach(source.Driver) - if err != nil { - klog.Error(log("attacher.Attach failed to find if driver is attachable: %v", err)) - return "", err - } - if skip { - klog.V(4).Infof(log("Driver is not attachable, skip waiting for attach")) - return "", nil - } - return c.waitForVolumeAttachment(source.VolumeHandle, attachID, timeout) } diff --git a/pkg/volume/csi/csi_attacher_test.go b/pkg/volume/csi/csi_attacher_test.go index c34008b624..76b40a4011 100644 --- a/pkg/volume/csi/csi_attacher_test.go +++ b/pkg/volume/csi/csi_attacher_test.go @@ -251,6 +251,16 @@ func TestAttacherWithCSIDriver(t *testing.T) { csiAttacher := attacher.(*csiAttacher) spec := volume.NewSpecFromPersistentVolume(makeTestPV("test-pv", 10, test.driver, "test-vol"), false) + pluginCanAttach := plug.CanAttach(spec) + if pluginCanAttach != test.expectVolumeAttachment { + t.Errorf("attacher.CanAttach does not match expected attachment status %t", test.expectVolumeAttachment) + } + + if !pluginCanAttach { + t.Log("plugin is not attachable") + return + } + expectedAttachID := getAttachmentName("test-vol", test.driver, "node") status := storage.VolumeAttachmentStatus{ Attached: true, @@ -258,6 +268,7 @@ func TestAttacherWithCSIDriver(t *testing.T) { if test.expectVolumeAttachment { go markVolumeAttached(t, csiAttacher.k8s, fakeWatcher, expectedAttachID, status) } + attachID, err := csiAttacher.Attach(spec, types.NodeName("node")) if err != nil { t.Errorf("Attach() failed: %s", err) @@ -321,6 +332,13 @@ func TestAttacherWaitForVolumeAttachmentWithCSIDriver(t *testing.T) { } csiAttacher := attacher.(*csiAttacher) spec := volume.NewSpecFromPersistentVolume(makeTestPV("test-pv", 10, test.driver, "test-vol"), false) + + pluginCanAttach := plug.CanAttach(spec) + if !pluginCanAttach { + t.Log("plugin is not attachable") + return + } + _, err = csiAttacher.WaitForAttach(spec, "", nil, time.Second) if err != nil && !test.expectError { t.Errorf("Unexpected error: %s", err) diff --git a/pkg/volume/csi/csi_plugin.go b/pkg/volume/csi/csi_plugin.go index 01b9ac9dad..e47715a034 100644 --- a/pkg/volume/csi/csi_plugin.go +++ b/pkg/volume/csi/csi_plugin.go @@ -439,6 +439,29 @@ func (p *csiPlugin) NewDetacher() (volume.Detacher, error) { }, nil } +func (p *csiPlugin) CanAttach(spec *volume.Spec) bool { + if spec.PersistentVolume == nil { + klog.Error(log("plugin.CanAttach test failed, spec missing PersistentVolume")) + return false + } + + var driverName string + if spec.PersistentVolume.Spec.CSI != nil { + driverName = spec.PersistentVolume.Spec.CSI.Driver + } else { + klog.Error(log("plugin.CanAttach test failed, spec missing CSIPersistentVolume")) + return false + } + + skipAttach, err := p.skipAttach(driverName) + + if err != nil { + klog.Error(log("plugin.CanAttach error when calling plugin.skipAttach for driver %s: %s", driverName, err)) + } + + return !skipAttach +} + func (p *csiPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) { return p.NewDetacher() } diff --git a/pkg/volume/csi/csi_plugin_test.go b/pkg/volume/csi/csi_plugin_test.go index 442dfc981a..75e19fc9fb 100644 --- a/pkg/volume/csi/csi_plugin_test.go +++ b/pkg/volume/csi/csi_plugin_test.go @@ -375,6 +375,36 @@ func TestPluginNewDetacher(t *testing.T) { } } +func TestPluginCanAttach(t *testing.T) { + tests := []struct { + name string + driverName string + canAttach bool + }{ + { + name: "attachable", + driverName: "attachble-driver", + canAttach: true, + }, + } + + for _, test := range tests { + csiDriver := getCSIDriver(test.driverName, nil, &test.canAttach) + t.Run(test.name, func(t *testing.T) { + fakeCSIClient := fakecsi.NewSimpleClientset(csiDriver) + plug, tmpDir := newTestPlugin(t, nil, fakeCSIClient) + defer os.RemoveAll(tmpDir) + spec := volume.NewSpecFromPersistentVolume(makeTestPV("test-pv", 10, test.driverName, "test-vol"), false) + + pluginCanAttach := plug.CanAttach(spec) + if pluginCanAttach != test.canAttach { + t.Fatalf("expecting plugin.CanAttach %t got %t", test.canAttach, pluginCanAttach) + return + } + }) + } +} + func TestPluginNewBlockMapper(t *testing.T) { defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIBlockVolume, true)() diff --git a/pkg/volume/fc/attacher.go b/pkg/volume/fc/attacher.go index 052567beb1..01a02617e8 100644 --- a/pkg/volume/fc/attacher.go +++ b/pkg/volume/fc/attacher.go @@ -175,6 +175,10 @@ func (detacher *fcDetacher) UnmountDevice(deviceMountPath string) error { return nil } +func (plugin *fcPlugin) CanAttach(spec *volume.Spec) bool { + return true +} + func volumeSpecToMounter(spec *volume.Spec, host volume.VolumeHost) (*fcDiskMounter, error) { fc, readOnly, err := getVolumeSource(spec) if err != nil { diff --git a/pkg/volume/flexvolume/plugin.go b/pkg/volume/flexvolume/plugin.go index 5527dba8a9..e907485a0f 100644 --- a/pkg/volume/flexvolume/plugin.go +++ b/pkg/volume/flexvolume/plugin.go @@ -256,6 +256,10 @@ func (plugin *flexVolumeAttachablePlugin) NewDeviceUnmounter() (volume.DeviceUnm return plugin.NewDetacher() } +func (plugin *flexVolumeAttachablePlugin) CanAttach(spec *volume.Spec) bool { + return true +} + // ConstructVolumeSpec is part of the volume.AttachableVolumePlugin interface. func (plugin *flexVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { flexVolume := &api.Volume{ diff --git a/pkg/volume/gcepd/attacher.go b/pkg/volume/gcepd/attacher.go index 4a998579c0..266647c093 100644 --- a/pkg/volume/gcepd/attacher.go +++ b/pkg/volume/gcepd/attacher.go @@ -288,3 +288,7 @@ func (detacher *gcePersistentDiskDetacher) Detach(volumeName string, nodeName ty func (detacher *gcePersistentDiskDetacher) UnmountDevice(deviceMountPath string) error { return mount.CleanupMountPoint(deviceMountPath, detacher.host.GetMounter(gcePersistentDiskPluginName), false) } + +func (plugin *gcePersistentDiskPlugin) CanAttach(spec *volume.Spec) bool { + return true +} diff --git a/pkg/volume/iscsi/attacher.go b/pkg/volume/iscsi/attacher.go index 2a59a3047e..f5be5b9fb7 100644 --- a/pkg/volume/iscsi/attacher.go +++ b/pkg/volume/iscsi/attacher.go @@ -176,6 +176,10 @@ func (detacher *iscsiDetacher) UnmountDevice(deviceMountPath string) error { return nil } +func (plugin *iscsiPlugin) CanAttach(spec *volume.Spec) bool { + return true +} + func volumeSpecToMounter(spec *volume.Spec, host volume.VolumeHost, targetLocks keymutex.KeyMutex, pod *v1.Pod) (*iscsiDiskMounter, error) { var secret map[string]string readOnly, fsType, err := getISCSIVolumeInfo(spec) diff --git a/pkg/volume/photon_pd/attacher.go b/pkg/volume/photon_pd/attacher.go index 3592e7ce8e..dcecf3ac71 100644 --- a/pkg/volume/photon_pd/attacher.go +++ b/pkg/volume/photon_pd/attacher.go @@ -307,3 +307,7 @@ func (detacher *photonPersistentDiskDetacher) WaitForDetach(devicePath string, t func (detacher *photonPersistentDiskDetacher) UnmountDevice(deviceMountPath string) error { return mount.CleanupMountPoint(deviceMountPath, detacher.mounter, false) } + +func (plugin *photonPersistentDiskPlugin) CanAttach(spec *volume.Spec) bool { + return true +} diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index b80149718e..7625d86bbc 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -210,6 +210,8 @@ type AttachableVolumePlugin interface { DeviceMountableVolumePlugin NewAttacher() (Attacher, error) NewDetacher() (Detacher, error) + // CanAttach tests if provided volume spec is attachable + CanAttach(spec *Spec) bool } // DeviceMountableVolumePlugin is an extended interface of VolumePlugin and is used @@ -823,7 +825,9 @@ func (pm *VolumePluginMgr) FindAttachablePluginBySpec(spec *Spec) (AttachableVol return nil, err } if attachableVolumePlugin, ok := volumePlugin.(AttachableVolumePlugin); ok { - return attachableVolumePlugin, nil + if attachableVolumePlugin.CanAttach(spec) { + return attachableVolumePlugin, nil + } } return nil, nil } diff --git a/pkg/volume/rbd/attacher.go b/pkg/volume/rbd/attacher.go index c82a20e40f..c265211e86 100644 --- a/pkg/volume/rbd/attacher.go +++ b/pkg/volume/rbd/attacher.go @@ -71,6 +71,10 @@ func (plugin *rbdPlugin) GetDeviceMountRefs(deviceMountPath string) ([]string, e return mounter.GetMountRefs(deviceMountPath) } +func (plugin *rbdPlugin) CanAttach(spec *volume.Spec) bool { + return true +} + // rbdAttacher implements volume.Attacher interface. type rbdAttacher struct { plugin *rbdPlugin diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 1628585cd3..b294f21bc4 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -484,6 +484,10 @@ func (plugin *FakeVolumePlugin) GetNewDetacherCallCount() int { return plugin.NewDetacherCallCount } +func (plugin *FakeVolumePlugin) CanAttach(spec *Spec) bool { + return true +} + func (plugin *FakeVolumePlugin) Recycle(pvName string, spec *Spec, eventRecorder recyclerclient.RecycleEventRecorder) error { return nil } @@ -634,6 +638,10 @@ func (f *FakeAttachableVolumePlugin) NewDetacher() (Detacher, error) { return f.Plugin.NewDetacher() } +func (f *FakeAttachableVolumePlugin) CanAttach(spec *Spec) bool { + return true +} + var _ VolumePlugin = &FakeAttachableVolumePlugin{} var _ AttachableVolumePlugin = &FakeAttachableVolumePlugin{} diff --git a/pkg/volume/vsphere_volume/attacher.go b/pkg/volume/vsphere_volume/attacher.go index 1551d981d5..ca0e895014 100644 --- a/pkg/volume/vsphere_volume/attacher.go +++ b/pkg/volume/vsphere_volume/attacher.go @@ -295,6 +295,10 @@ func (detacher *vsphereVMDKDetacher) UnmountDevice(deviceMountPath string) error return mount.CleanupMountPoint(deviceMountPath, detacher.mounter, false) } +func (plugin *vsphereVolumePlugin) CanAttach(spec *volume.Spec) bool { + return true +} + func setNodeVolume( nodeVolumeMap map[types.NodeName]map[*volume.Spec]bool, volumeSpec *volume.Spec,