From 835183ad5075249c4017f49cf60ae08d7c05687e Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 14 Sep 2018 20:15:27 +0530 Subject: [PATCH] Add AttachDevice() to attach the device to a host AttachDevice() ensures that the volume device is attached to the host before they are used. --- pkg/volume/storageos/storageos.go | 13 ++++++-- pkg/volume/storageos/storageos_test.go | 23 +++++++++---- pkg/volume/storageos/storageos_util.go | 36 ++++++++++++--------- pkg/volume/storageos/storageos_util_test.go | 4 +-- 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/pkg/volume/storageos/storageos.go b/pkg/volume/storageos/storageos.go index 745aefb41e..fc859ce154 100644 --- a/pkg/volume/storageos/storageos.go +++ b/pkg/volume/storageos/storageos.go @@ -281,6 +281,8 @@ type storageosManager interface { CreateVolume(provisioner *storageosProvisioner) (*storageosVolume, error) // Attaches the disk to the kubelet's host machine. AttachVolume(mounter *storageosMounter) (string, error) + // Attaches the device to the host at a mount path. + AttachDevice(mounter *storageosMounter, deviceMountPath string) error // Detaches the disk from the kubelet's host machine. DetachVolume(unmounter *storageosUnmounter, dir string) error // Mounts the disk on the Kubelet's host machine. @@ -351,6 +353,14 @@ func (b *storageosMounter) SetUp(fsGroup *int64) error { b.volNamespace = b.podNamespace } + targetPath := makeGlobalPDName(b.plugin.host, b.pvName, b.volNamespace, b.volName) + + // Attach the device to the host. + if err := b.manager.AttachDevice(b, targetPath); err != nil { + klog.Errorf("Failed to attach device at %s: %s", targetPath, err.Error()) + return err + } + // Attach the StorageOS volume as a block device devicePath, err := b.manager.AttachVolume(b) if err != nil { @@ -359,8 +369,7 @@ func (b *storageosMounter) SetUp(fsGroup *int64) error { } // Mount the loop device into the plugin's disk global mount dir. - globalPDPath := makeGlobalPDName(b.plugin.host, b.pvName, b.podNamespace, b.volName) - err = b.manager.MountVolume(b, devicePath, globalPDPath) + err = b.manager.MountVolume(b, devicePath, targetPath) if err != nil { return err } diff --git a/pkg/volume/storageos/storageos_test.go b/pkg/volume/storageos/storageos_test.go index a5732fff7a..2b448bc916 100644 --- a/pkg/volume/storageos/storageos_test.go +++ b/pkg/volume/storageos/storageos_test.go @@ -75,13 +75,14 @@ func TestGetAccessModes(t *testing.T) { } type fakePDManager struct { - api apiImplementer - attachCalled bool - detachCalled bool - mountCalled bool - unmountCalled bool - createCalled bool - deleteCalled bool + api apiImplementer + attachCalled bool + attachDeviceCalled bool + detachCalled bool + mountCalled bool + unmountCalled bool + createCalled bool + deleteCalled bool } func (fake *fakePDManager) NewAPI(apiCfg *storageosAPIConfig) error { @@ -108,6 +109,11 @@ func (fake *fakePDManager) AttachVolume(b *storageosMounter) (string, error) { return "", nil } +func (fake *fakePDManager) AttachDevice(b *storageosMounter, dir string) error { + fake.attachDeviceCalled = true + return nil +} + func (fake *fakePDManager) DetachVolume(b *storageosUnmounter, loopDevice string) error { fake.detachCalled = true return nil @@ -213,6 +219,9 @@ func TestPlugin(t *testing.T) { } } + if !fakeManager.attachDeviceCalled { + t.Errorf("AttachDevice not called") + } if !fakeManager.attachCalled { t.Errorf("Attach not called") } diff --git a/pkg/volume/storageos/storageos_util.go b/pkg/volume/storageos/storageos_util.go index 0f5ebb80f8..eb669186f6 100644 --- a/pkg/volume/storageos/storageos_util.go +++ b/pkg/volume/storageos/storageos_util.go @@ -203,6 +203,25 @@ func (u *storageosUtil) DetachVolume(b *storageosUnmounter, devicePath string) e return removeLoopDevice(devicePath, b.exec) } +// AttachDevice attaches the volume device to the host at a given mount path. +func (u *storageosUtil) AttachDevice(b *storageosMounter, deviceMountPath string) error { + if err := u.NewAPI(b.apiCfg); err != nil { + return err + } + + opts := storageostypes.VolumeMountOptions{ + Name: b.volName, + Namespace: b.volNamespace, + FsType: b.fsType, + Mountpoint: deviceMountPath, + Client: b.plugin.host.GetHostName(), + } + if err := u.api.VolumeMount(opts); err != nil { + return err + } + return nil +} + // Mount mounts the volume on the host. func (u *storageosUtil) MountVolume(b *storageosMounter, mntDevice, deviceMountPath string) error { notMnt, err := b.mounter.IsLikelyNotMountPoint(deviceMountPath) @@ -231,22 +250,7 @@ func (u *storageosUtil) MountVolume(b *storageosMounter, mntDevice, deviceMountP return err } } - if err != nil { - return err - } - - if err := u.NewAPI(b.apiCfg); err != nil { - return err - } - - opts := storageostypes.VolumeMountOptions{ - Name: b.volName, - Namespace: b.volNamespace, - FsType: b.fsType, - Mountpoint: deviceMountPath, - Client: b.plugin.host.GetHostName(), - } - return u.api.VolumeMount(opts) + return err } // Unmount removes the mount reference from the volume allowing it to be diff --git a/pkg/volume/storageos/storageos_util_test.go b/pkg/volume/storageos/storageos_util_test.go index e4800cbe51..b3dabba28a 100644 --- a/pkg/volume/storageos/storageos_util_test.go +++ b/pkg/volume/storageos/storageos_util_test.go @@ -108,8 +108,8 @@ func (f fakeAPI) VolumeUnmount(opts storageostypes.VolumeUnmountOptions) error { func (f fakeAPI) VolumeDelete(opts storageostypes.DeleteOptions) error { return nil } -func (f fakeAPI) Controller(ref string) (*storageostypes.Controller, error) { - return &storageostypes.Controller{}, nil +func (f fakeAPI) Node(ref string) (*storageostypes.Node, error) { + return &storageostypes.Node{}, nil } func TestCreateVolume(t *testing.T) {