github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-27 21:47:07 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #76151 from vladimirvivien/plugin-CanAttach-return-error

Browse Source 
Ability for volume AttachablePlugin.CanAttach() to return both bool and error
This commit is contained in:
Kubernetes Prow Robot 2019-04-08 20:29:05 -07:00 committed by GitHub
commit fa9b36de77
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 131 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/controller/volume/attachdetach/testing/testvolumespec.go
View File

@ -308,8 +308,8 @@ func (plugin *TestPlugin) NewDetacher() (volume.Detacher, error) {
return &detacher, nil
}
func (plugin *TestPlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *TestPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *TestPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

4
pkg/volume/awsebs/attacher.go
View File

@ -277,8 +277,8 @@ func (detacher *awsElasticBlockStoreDetacher) UnmountDevice(deviceMountPath stri
return mount.CleanupMountPoint(deviceMountPath, detacher.mounter, false)
}
func (plugin *awsElasticBlockStorePlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *awsElasticBlockStorePlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *awsElasticBlockStorePlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

4
pkg/volume/azure_dd/azure_dd.go
View File

@ -238,8 +238,8 @@ func (plugin *azureDataDiskPlugin) NewDetacher() (volume.Detacher, error) {
}, nil
}
func (plugin *azureDataDiskPlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *azureDataDiskPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *azureDataDiskPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

4
pkg/volume/cinder/attacher.go
View File

@ -406,8 +406,8 @@ 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 (plugin *cinderPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *cinderPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

10
pkg/volume/csi/csi_attacher_test.go
View File

@ -348,7 +348,10 @@ func TestAttacherWithCSIDriver(t *testing.T) {
csiAttacher := attacher.(*csiAttacher)
spec := volume.NewSpecFromPersistentVolume(makeTestPV("test-pv", 10, test.driver, "test-vol"), false)
pluginCanAttach := plug.CanAttach(spec)
pluginCanAttach, err := plug.CanAttach(spec)
if err != nil {
t.Fatalf("attacher.CanAttach failed: %s", err)
}
if pluginCanAttach != test.expectVolumeAttachment {
t.Errorf("attacher.CanAttach does not match expected attachment status %t", test.expectVolumeAttachment)
}
@ -429,7 +432,10 @@ func TestAttacherWaitForVolumeAttachmentWithCSIDriver(t *testing.T) {
csiAttacher := attacher.(*csiAttacher)
spec := volume.NewSpecFromPersistentVolume(makeTestPV("test-pv", 10, test.driver, "test-vol"), false)
pluginCanAttach := plug.CanAttach(spec)
pluginCanAttach, err := plug.CanAttach(spec)
if err != nil {
t.Fatalf("plugin.CanAttach test failed: %s", err)
}
if !pluginCanAttach {
t.Log("plugin is not attachable")
return

18
pkg/volume/csi/csi_plugin.go
View File

@ -581,34 +581,30 @@ func (p *csiPlugin) NewDetacher() (volume.Detacher, error) {
}, nil
}
// TODO change CanAttach to return error to propagate ability
// to support Attachment or an error - see https://github.com/kubernetes/kubernetes/issues/74810
func (p *csiPlugin) CanAttach(spec *volume.Spec) bool {
func (p *csiPlugin) CanAttach(spec *volume.Spec) (bool, error) {
driverMode, err := p.getDriverMode(spec)
if err != nil {
return false
return false, err
}
if driverMode == ephemeralDriverMode {
klog.V(4).Info(log("driver ephemeral mode detected for spec %v", spec.Name))
return false
klog.V(5).Info(log("plugin.CanAttach = false, ephemeral mode detected for spec %v", spec.Name()))
return false, nil
}
pvSrc, err := getCSISourceFromSpec(spec)
if err != nil {
klog.Error(log("plugin.CanAttach failed to get info from spec: %s", err))
return false
return false, err
}
driverName := pvSrc.Driver
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 false
return false, err
}
return !skipAttach
return !skipAttach, nil
}
// TODO (#75352) add proper logic to determine device moutability by inspecting the spec.

88
pkg/volume/csi/csi_plugin_test.go
View File

@ -880,6 +880,7 @@ func TestPluginCanAttach(t *testing.T) {
driverName string
spec *volume.Spec
canAttach bool
shouldFail bool
}{
{
name: "non-attachable inline",
@ -893,6 +894,19 @@ func TestPluginCanAttach(t *testing.T) {
spec: volume.NewSpecFromPersistentVolume(makeTestPV("test-vol", 20, "attachable-pv", testVol), true),
canAttach: true,
},
{
name: "incomplete spec",
driverName: "attachable-pv",
spec: &volume.Spec{ReadOnly: true},
canAttach: false,
shouldFail: true,
},
{
name: "nil spec",
driverName: "attachable-pv",
canAttach: false,
shouldFail: true,
},
}
for _, test := range tests {
@ -902,10 +916,80 @@ func TestPluginCanAttach(t *testing.T) {
plug, tmpDir := newTestPlugin(t, fakeCSIClient)
defer os.RemoveAll(tmpDir)
pluginCanAttach := plug.CanAttach(test.spec)
pluginCanAttach, err := plug.CanAttach(test.spec)
if err != nil && !test.shouldFail {
t.Fatalf("unexected plugin.CanAttach error: %s", err)
}
if pluginCanAttach != test.canAttach {
t.Fatalf("expecting plugin.CanAttach %t got %t", test.canAttach, pluginCanAttach)
return
}
})
}
}
func TestPluginFindAttachablePlugin(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)()
tests := []struct {
name string
driverName string
spec *volume.Spec
canAttach bool
shouldFail bool
}{
{
name: "non-attachable inline",
driverName: "attachable-inline",
spec: volume.NewSpecFromVolume(makeTestVol("test-vol", "attachable-inline")),
canAttach: false,
},
{
name: "attachable PV",
driverName: "attachable-pv",
spec: volume.NewSpecFromPersistentVolume(makeTestPV("test-vol", 20, "attachable-pv", testVol), true),
canAttach: true,
},
{
name: "incomplete spec",
driverName: "attachable-pv",
spec: &volume.Spec{ReadOnly: true},
canAttach: false,
shouldFail: true,
},
{
name: "nil spec",
driverName: "attachable-pv",
canAttach: false,
shouldFail: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
tmpDir, err := utiltesting.MkTmpdir("csi-test")
if err != nil {
t.Fatalf("can't create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)
client := fakeclient.NewSimpleClientset(getCSIDriver(test.driverName, nil, &test.canAttach))
factory := informers.NewSharedInformerFactory(client, csiResyncPeriod)
host := volumetest.NewFakeVolumeHostWithCSINodeName(
tmpDir,
client,
nil,
"fakeNode",
factory.Storage().V1beta1().CSIDrivers().Lister(),
)
plugMgr := &volume.VolumePluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), nil /* prober */, host)
plugin, err := plugMgr.FindAttachablePluginBySpec(test.spec)
if err != nil && !test.shouldFail {
t.Fatalf("unexected error calling pluginMgr.FindAttachablePluginBySpec: %s", err)
}
if (plugin != nil) != test.canAttach {
t.Fatal("expecting attachable plugin, but got nil")
}
})
}

4
pkg/volume/fc/attacher.go
View File

@ -175,8 +175,8 @@ func (detacher *fcDetacher) UnmountDevice(deviceMountPath string) error {
return nil
}
func (plugin *fcPlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *fcPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *fcPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

4
pkg/volume/flexvolume/plugin.go
View File

@ -256,8 +256,8 @@ func (plugin *flexVolumeAttachablePlugin) NewDeviceUnmounter() (volume.DeviceUnm
return plugin.NewDetacher()
}
func (plugin *flexVolumeAttachablePlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *flexVolumeAttachablePlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *flexVolumeAttachablePlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

4
pkg/volume/gcepd/attacher.go
View File

@ -350,8 +350,8 @@ func (detacher *gcePersistentDiskDetacher) UnmountDevice(deviceMountPath string)
return mount.CleanupMountPoint(deviceMountPath, detacher.host.GetMounter(gcePersistentDiskPluginName), false)
}
func (plugin *gcePersistentDiskPlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *gcePersistentDiskPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *gcePersistentDiskPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

4
pkg/volume/iscsi/attacher.go
View File

@ -176,8 +176,8 @@ func (detacher *iscsiDetacher) UnmountDevice(deviceMountPath string) error {
return nil
}
func (plugin *iscsiPlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *iscsiPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *iscsiPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

4
pkg/volume/photon_pd/attacher.go
View File

@ -308,8 +308,8 @@ func (detacher *photonPersistentDiskDetacher) UnmountDevice(deviceMountPath stri
return mount.CleanupMountPoint(deviceMountPath, detacher.mounter, false)
}
func (plugin *photonPersistentDiskPlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *photonPersistentDiskPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *photonPersistentDiskPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

6
pkg/volume/plugins.go
View File

@ -235,7 +235,7 @@ type AttachableVolumePlugin interface {
NewAttacher() (Attacher, error)
NewDetacher() (Detacher, error)
// CanAttach tests if provided volume spec is attachable
CanAttach(spec *Spec) bool
CanAttach(spec *Spec) (bool, error)
}
// DeviceMountableVolumePlugin is an extended interface of VolumePlugin and is used
@ -891,7 +891,9 @@ func (pm *VolumePluginMgr) FindAttachablePluginBySpec(spec *Spec) (AttachableVol
return nil, err
}
if attachableVolumePlugin, ok := volumePlugin.(AttachableVolumePlugin); ok {
if attachableVolumePlugin.CanAttach(spec) {
if canAttach, err := attachableVolumePlugin.CanAttach(spec); err != nil {
return nil, err
} else if canAttach {
return attachableVolumePlugin, nil
}
}

4
pkg/volume/rbd/attacher.go
View File

@ -71,8 +71,8 @@ func (plugin *rbdPlugin) GetDeviceMountRefs(deviceMountPath string) ([]string, e
return mounter.GetMountRefs(deviceMountPath)
}
func (plugin *rbdPlugin) CanAttach(spec *volume.Spec) bool {
return true
func (plugin *rbdPlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *rbdPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {

8
pkg/volume/testing/testing.go
View File

@ -492,8 +492,8 @@ func (plugin *FakeVolumePlugin) GetNewDetacherCallCount() int {
return plugin.NewDetacherCallCount
}
func (plugin *FakeVolumePlugin) CanAttach(spec *Spec) bool {
return true
func (plugin *FakeVolumePlugin) CanAttach(spec *Spec) (bool, error) {
return true, nil
}
func (plugin *FakeVolumePlugin) CanDeviceMount(spec *Spec) (bool, error) {
@ -654,8 +654,8 @@ func (f *FakeAttachableVolumePlugin) NewDetacher() (Detacher, error) {
return f.Plugin.NewDetacher()
}
func (f *FakeAttachableVolumePlugin) CanAttach(spec *Spec) bool {
return true
func (f *FakeAttachableVolumePlugin) CanAttach(spec *Spec) (bool, error) {
return true, nil
}
var _ VolumePlugin = &FakeAttachableVolumePlugin{}

4
pkg/volume/vsphere_volume/attacher.go
View File

@ -295,8 +295,8 @@ 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 (plugin *vsphereVolumePlugin) CanAttach(spec *volume.Spec) (bool, error) {
return true, nil
}
func (plugin *vsphereVolumePlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {