Merge pull request kubernetes#75844 from vladimirvivien/volume-skip-device-mount

k8s-ci-robot · web-flow · commit 7a478a46099e · 2019-03-29T13:02:13.000-07:00
Volume DeviceMountablePlugin.CanDeviceMount check when retrieving plugins
diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go
@@ -312,6 +312,10 @@ func (plugin *TestPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *TestPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 func (plugin *TestPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) {
 	return plugin.NewDetacher()
 }
diff --git a/pkg/volume/awsebs/attacher.go b/pkg/volume/awsebs/attacher.go
@@ -281,6 +281,10 @@ func (plugin *awsElasticBlockStorePlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *awsElasticBlockStorePlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 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
@@ -242,6 +242,10 @@ func (plugin *azureDataDiskPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *azureDataDiskPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 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
@@ -410,6 +410,10 @@ func (plugin *cinderPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *cinderPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 func (attacher *cinderDiskAttacher) nodeInstanceID(nodeName types.NodeName) (string, error) {
 	instances, res := attacher.cinderProvider.Instances()
 	if !res {
diff --git a/pkg/volume/csi/csi_plugin.go b/pkg/volume/csi/csi_plugin.go
@@ -604,6 +604,11 @@ func (p *csiPlugin) CanAttach(spec *volume.Spec) bool {
 	return !skipAttach
 }
 
+// TODO (#75352) add proper logic to determine device moutability by inspecting the spec.
+func (p *csiPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 func (p *csiPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) {
 	return p.NewDetacher()
 }
diff --git a/pkg/volume/fc/attacher.go b/pkg/volume/fc/attacher.go
@@ -179,6 +179,10 @@ func (plugin *fcPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *fcPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 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
@@ -260,6 +260,10 @@ func (plugin *flexVolumeAttachablePlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *flexVolumeAttachablePlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 // 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
@@ -353,3 +353,7 @@ func (detacher *gcePersistentDiskDetacher) UnmountDevice(deviceMountPath string)
 func (plugin *gcePersistentDiskPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
+
+func (plugin *gcePersistentDiskPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
diff --git a/pkg/volume/iscsi/attacher.go b/pkg/volume/iscsi/attacher.go
@@ -180,6 +180,10 @@ func (plugin *iscsiPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *iscsiPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 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/local/local.go b/pkg/volume/local/local.go
@@ -253,6 +253,10 @@ type deviceMounter struct {
 
 var _ volume.DeviceMounter = &deviceMounter{}
 
+func (plugin *localVolumePlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 func (plugin *localVolumePlugin) NewDeviceMounter() (volume.DeviceMounter, error) {
 	return &deviceMounter{
 		plugin:  plugin,
diff --git a/pkg/volume/photon_pd/attacher.go b/pkg/volume/photon_pd/attacher.go
@@ -311,3 +311,7 @@ func (detacher *photonPersistentDiskDetacher) UnmountDevice(deviceMountPath stri
 func (plugin *photonPersistentDiskPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
+
+func (plugin *photonPersistentDiskPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go
@@ -241,6 +241,8 @@ type DeviceMountableVolumePlugin interface {
 	NewDeviceMounter() (DeviceMounter, error)
 	NewDeviceUnmounter() (DeviceUnmounter, error)
 	GetDeviceMountRefs(deviceMountPath string) ([]string, error)
+	// CanDeviceMount determines if device in volume.Spec is mountable
+	CanDeviceMount(spec *Spec) (bool, error)
 }
 
 // ExpandableVolumePlugin is an extended interface of VolumePlugin and is used for volumes that can be
@@ -902,7 +904,11 @@ func (pm *VolumePluginMgr) FindDeviceMountablePluginBySpec(spec *Spec) (DeviceMo
 		return nil, err
 	}
 	if deviceMountableVolumePlugin, ok := volumePlugin.(DeviceMountableVolumePlugin); ok {
-		return deviceMountableVolumePlugin, nil
+		if canMount, err := deviceMountableVolumePlugin.CanDeviceMount(spec); err != nil {
+			return nil, err
+		} else if canMount {
+			return deviceMountableVolumePlugin, nil
+		}
 	}
 	return nil, nil
 }
diff --git a/pkg/volume/rbd/attacher.go b/pkg/volume/rbd/attacher.go
@@ -75,6 +75,10 @@ func (plugin *rbdPlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *rbdPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 // rbdAttacher implements volume.Attacher interface.
 type rbdAttacher struct {
 	plugin  *rbdPlugin
diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go
@@ -488,6 +488,10 @@ func (plugin *FakeVolumePlugin) CanAttach(spec *Spec) bool {
 	return true
 }
 
+func (plugin *FakeVolumePlugin) CanDeviceMount(spec *Spec) (bool, error) {
+	return true, nil
+}
+
 func (plugin *FakeVolumePlugin) Recycle(pvName string, spec *Spec, eventRecorder recyclerclient.RecycleEventRecorder) error {
 	return nil
 }
@@ -610,6 +614,10 @@ type FakeDeviceMountableVolumePlugin struct {
 	FakeBasicVolumePlugin
 }
 
+func (f *FakeDeviceMountableVolumePlugin) CanDeviceMount(spec *Spec) (bool, error) {
+	return true, nil
+}
+
 func (f *FakeDeviceMountableVolumePlugin) NewDeviceMounter() (DeviceMounter, error) {
 	return f.Plugin.NewDeviceMounter()
 }
diff --git a/pkg/volume/vsphere_volume/attacher.go b/pkg/volume/vsphere_volume/attacher.go
@@ -299,6 +299,10 @@ func (plugin *vsphereVolumePlugin) CanAttach(spec *volume.Spec) bool {
 	return true
 }
 
+func (plugin *vsphereVolumePlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
 func setNodeVolume(
 	nodeVolumeMap map[types.NodeName]map[*volume.Spec]bool,
 	volumeSpec *volume.Spec,

Original file line number	Diff line number	Diff line change
`@@ -312,6 +312,10 @@ func (plugin TestPlugin) CanAttach(spec volume.Spec) bool {`
`312`	`312`	`return true`
`313`	`313`	`}`
`314`	`314`
	`315`	`+func (plugin TestPlugin) CanDeviceMount(spec volume.Spec) (bool, error) {`
	`316`	`+ return true, nil`
	`317`	`+}`
	`318`	`+`
`315`	`319`	`func (plugin *TestPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) {`
`316`	`320`	`return plugin.NewDetacher()`
`317`	`321`	`}`
Original file line number	Diff line number	Diff line change
`@@ -604,6 +604,11 @@ func (p csiPlugin) CanAttach(spec volume.Spec) bool {`
`604`	`604`	`return !skipAttach`
`605`	`605`	`}`
`606`	`606`
	`607`	`+// TODO (#75352) add proper logic to determine device moutability by inspecting the spec.`
	`608`	`+func (p csiPlugin) CanDeviceMount(spec volume.Spec) (bool, error) {`
	`609`	`+ return true, nil`
	`610`	`+}`
	`611`	`+`
`607`	`612`	`func (p *csiPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) {`
`608`	`613`	`return p.NewDetacher()`
`609`	`614`	`}`
Original file line number	Diff line number	Diff line change
`@@ -353,3 +353,7 @@ func (detacher *gcePersistentDiskDetacher) UnmountDevice(deviceMountPath string)`
`353`	`353`	`func (plugin gcePersistentDiskPlugin) CanAttach(spec volume.Spec) bool {`
`354`	`354`	`return true`
`355`	`355`	`}`
	`356`	`+`
	`357`	`+func (plugin gcePersistentDiskPlugin) CanDeviceMount(spec volume.Spec) (bool, error) {`
	`358`	`+ return true, nil`
	`359`	`+}`