Change signature of MountDevice function and remove MountDeviceWithStatusTracking

This commit is contained in:
Hemant Kumar 2019-11-06 16:29:54 -05:00
parent 0c52b6606e
commit db9ac38592
19 changed files with 118 additions and 143 deletions

View File

@ -441,14 +441,9 @@ func (attacher *testPluginAttacher) MountDevice(spec *volume.Spec, devicePath st
if spec == nil {
*attacher.ErrorEncountered = true
klog.Errorf("MountDevice called with nil volume spec")
return fmt.Errorf("MountDevice called with nil volume spec")
return volumetypes.OperationFinished, fmt.Errorf("MountDevice called with nil volume spec")
}
return nil
}
func (attacher *testPluginAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
return volumetypes.OperationFinished, nil
}
// Detacher

View File

@ -207,7 +207,7 @@ func (attacher *awsElasticBlockStoreAttacher) GetDeviceMountPath(
}
// FIXME: this method can be further pruned.
func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
mounter := attacher.host.GetMounter(awsElasticBlockStorePluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
if err != nil {
@ -222,17 +222,17 @@ func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, dev
dir = filepath.Dir(deviceMountPath)
}
if err := os.MkdirAll(dir, 0750); err != nil {
return fmt.Errorf("making dir %s failed with %s", dir, err)
return volumetypes.OperationFinished, fmt.Errorf("making dir %s failed with %s", dir, err)
}
notMnt = true
} else {
return err
return volumetypes.OperationFinished, err
}
}
volumeSource, readOnly, err := getVolumeSource(spec)
if err != nil {
return err
return volumetypes.OperationFinished, err
}
options := []string{}
@ -245,15 +245,10 @@ func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, dev
err = diskMounter.FormatAndMount(devicePath, deviceMountPath, volumeSource.FSType, mountOptions)
if err != nil {
os.Remove(deviceMountPath)
return err
return volumetypes.OperationFinished, err
}
}
return nil
}
func (attacher *awsElasticBlockStoreAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
return volumetypes.OperationFinished, nil
}
type awsElasticBlockStoreDetacher struct {

View File

@ -199,7 +199,11 @@ func (a *azureDiskAttacher) GetDeviceMountPath(spec *volume.Spec) (string, error
return makeGlobalPDPath(a.plugin.host, volumeSource.DataDiskURI, isManagedDisk)
}
func (attacher *azureDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
func (attacher *azureDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
return volumetypes.OperationFinished, attacher.mountDeviceInternal(spec, devicePath, deviceMountPath)
}
func (attacher *azureDiskAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error {
mounter := attacher.plugin.host.GetMounter(azureDataDiskPluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
@ -260,11 +264,6 @@ func (attacher *azureDiskAttacher) MountDevice(spec *volume.Spec, devicePath str
return nil
}
func (d *azureDiskAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := d.MountDevice(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
}
// Detach detaches disk from Azure VM.
func (d *azureDiskDetacher) Detach(diskURI string, nodeName types.NodeName) error {
if diskURI == "" {

View File

@ -269,7 +269,7 @@ func (attacher *cinderDiskAttacher) GetDeviceMountPath(
}
// FIXME: this method can be further pruned.
func (attacher *cinderDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
func (attacher *cinderDiskAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error {
mounter := attacher.host.GetMounter(cinderVolumePluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
if err != nil {
@ -304,8 +304,8 @@ func (attacher *cinderDiskAttacher) MountDevice(spec *volume.Spec, devicePath st
return nil
}
func (attacher *cinderDiskAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
func (attacher *cinderDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
}

View File

@ -220,12 +220,7 @@ func (c *csiAttacher) GetDeviceMountPath(spec *volume.Spec) (string, error) {
return deviceMountPath, nil
}
func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
_, err := c.MountDeviceWithStatusTracking(spec, devicePath, deviceMountPath)
return err
}
func (c *csiAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
klog.V(4).Infof(log("attacher.MountDevice(%s, %s)", devicePath, deviceMountPath))
// lets default to operation as finished state
opExitStatus := volumetypes.OperationFinished

View File

@ -1348,7 +1348,7 @@ func TestAttacherMountDeviceWithInline(t *testing.T) {
}()
// Run
err = csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath)
_, err = csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath)
// Verify
if err != nil {

View File

@ -360,7 +360,7 @@ func TestCSI_VolumeAll(t *testing.T) {
if err != nil {
t.Fatalf("csiTest.VolumeAll deviceMounter.GetdeviceMountPath failed %s", err)
}
if err := csiDevMounter.MountDevice(volSpec, devicePath, devMountPath); err != nil {
if _, err := csiDevMounter.MountDevice(volSpec, devicePath, devMountPath); err != nil {
t.Fatalf("csiTest.VolumeAll deviceMounter.MountDevice failed: %v", err)
}
t.Log("csiTest.VolumeAll device mounted at path:", devMountPath)

View File

@ -97,44 +97,42 @@ func (attacher *fcAttacher) GetDeviceMountPath(
return attacher.manager.MakeGlobalPDName(*mounter.fcDisk), nil
}
func (attacher *fcAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
mounter := attacher.host.GetMounter(fcPluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
if err != nil {
if os.IsNotExist(err) {
if err := os.MkdirAll(deviceMountPath, 0750); err != nil {
func (attacher *fcAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
mountInternal := func() error {
mounter := attacher.host.GetMounter(fcPluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
if err != nil {
if os.IsNotExist(err) {
if err := os.MkdirAll(deviceMountPath, 0750); err != nil {
return err
}
notMnt = true
} else {
return err
}
notMnt = true
} else {
return err
}
}
volumeSource, readOnly, err := getVolumeSource(spec)
if err != nil {
return err
}
options := []string{}
if readOnly {
options = append(options, "ro")
}
if notMnt {
diskMounter := &mount.SafeFormatAndMount{Interface: mounter, Exec: attacher.host.GetExec(fcPluginName)}
mountOptions := volumeutil.MountOptionFromSpec(spec, options...)
err = diskMounter.FormatAndMount(devicePath, deviceMountPath, volumeSource.FSType, mountOptions)
volumeSource, readOnly, err := getVolumeSource(spec)
if err != nil {
os.Remove(deviceMountPath)
return err
}
}
return nil
}
func (attacher *fcAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
options := []string{}
if readOnly {
options = append(options, "ro")
}
if notMnt {
diskMounter := &mount.SafeFormatAndMount{Interface: mounter, Exec: attacher.host.GetExec(fcPluginName)}
mountOptions := volumeutil.MountOptionFromSpec(spec, options...)
err = diskMounter.FormatAndMount(devicePath, deviceMountPath, volumeSource.FSType, mountOptions)
if err != nil {
os.Remove(deviceMountPath)
return err
}
}
return nil
}
return volumetypes.OperationFinished, mountInternal()
}
type fcDetacher struct {

View File

@ -71,36 +71,34 @@ func (a *flexVolumeAttacher) GetDeviceMountPath(spec *volume.Spec) (string, erro
}
// MountDevice is part of the volume.Attacher interface
func (a *flexVolumeAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
// Mount only once.
alreadyMounted, err := prepareForMount(a.plugin.host.GetMounter(a.plugin.GetPluginName()), deviceMountPath)
if err != nil {
func (a *flexVolumeAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
mountInternal := func() error {
// Mount only once.
alreadyMounted, err := prepareForMount(a.plugin.host.GetMounter(a.plugin.GetPluginName()), deviceMountPath)
if err != nil {
return err
}
if alreadyMounted {
return nil
}
call := a.plugin.NewDriverCall(mountDeviceCmd)
call.Append(deviceMountPath)
call.Append(devicePath)
call.AppendSpec(spec, a.plugin.host, nil)
_, err = call.Run()
if isCmdNotSupportedErr(err) {
// Devicepath is empty if the plugin does not support attach calls. Ignore mountDevice calls if the
// plugin does not implement attach interface.
if devicePath != "" {
return (*attacherDefaults)(a).MountDevice(spec, devicePath, deviceMountPath, a.plugin.host.GetMounter(a.plugin.GetPluginName()))
}
return nil
}
return err
}
if alreadyMounted {
return nil
}
call := a.plugin.NewDriverCall(mountDeviceCmd)
call.Append(deviceMountPath)
call.Append(devicePath)
call.AppendSpec(spec, a.plugin.host, nil)
_, err = call.Run()
if isCmdNotSupportedErr(err) {
// Devicepath is empty if the plugin does not support attach calls. Ignore mountDevice calls if the
// plugin does not implement attach interface.
if devicePath != "" {
return (*attacherDefaults)(a).MountDevice(spec, devicePath, deviceMountPath, a.plugin.host.GetMounter(a.plugin.GetPluginName()))
}
return nil
}
return err
}
func (a *flexVolumeAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := a.MountDevice(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
return volumetypes.OperationFinished, mountInternal()
}
func (a *flexVolumeAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName types.NodeName) (map[*volume.Spec]bool, error) {

View File

@ -287,7 +287,7 @@ func (attacher *gcePersistentDiskAttacher) GetDeviceMountPath(
return makeGlobalPDName(attacher.host, volumeSource.PDName), nil
}
func (attacher *gcePersistentDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
func (attacher *gcePersistentDiskAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error {
// Only mount the PD globally once.
mounter := attacher.host.GetMounter(gcePersistentDiskPluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
@ -329,8 +329,8 @@ func (attacher *gcePersistentDiskAttacher) MountDevice(spec *volume.Spec, device
return nil
}
func (attacher *gcePersistentDiskAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
func (attacher *gcePersistentDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
}

View File

@ -102,7 +102,7 @@ func (attacher *iscsiAttacher) GetDeviceMountPath(
return attacher.manager.MakeGlobalPDName(*mounter.iscsiDisk), nil
}
func (attacher *iscsiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
func (attacher *iscsiAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error {
mounter := attacher.host.GetMounter(iscsiPluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
if err != nil {
@ -136,8 +136,8 @@ func (attacher *iscsiAttacher) MountDevice(spec *volume.Spec, devicePath string,
return nil
}
func (attacher *iscsiAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
func (attacher *iscsiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
}

View File

@ -349,31 +349,29 @@ func (dm *deviceMounter) mountLocalBlockDevice(spec *volume.Spec, devicePath str
return nil
}
func (dm *deviceMounter) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
if spec.PersistentVolume.Spec.Local == nil || len(spec.PersistentVolume.Spec.Local.Path) == 0 {
return fmt.Errorf("local volume source is nil or local path is not set")
}
fileType, err := dm.hostUtil.GetFileType(spec.PersistentVolume.Spec.Local.Path)
if err != nil {
return err
}
func (dm *deviceMounter) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
mountInternal := func() error {
if spec.PersistentVolume.Spec.Local == nil || len(spec.PersistentVolume.Spec.Local.Path) == 0 {
return fmt.Errorf("local volume source is nil or local path is not set")
}
fileType, err := dm.hostUtil.GetFileType(spec.PersistentVolume.Spec.Local.Path)
if err != nil {
return err
}
switch fileType {
case hostutil.FileTypeBlockDev:
// local volume plugin does not implement AttachableVolumePlugin interface, so set devicePath to Path in PV spec directly
devicePath = spec.PersistentVolume.Spec.Local.Path
return dm.mountLocalBlockDevice(spec, devicePath, deviceMountPath)
case hostutil.FileTypeDirectory:
// if the given local volume path is of already filesystem directory, return directly
return nil
default:
return fmt.Errorf("only directory and block device are supported")
switch fileType {
case hostutil.FileTypeBlockDev:
// local volume plugin does not implement AttachableVolumePlugin interface, so set devicePath to Path in PV spec directly
devicePath = spec.PersistentVolume.Spec.Local.Path
return dm.mountLocalBlockDevice(spec, devicePath, deviceMountPath)
case hostutil.FileTypeDirectory:
// if the given local volume path is of already filesystem directory, return directly
return nil
default:
return fmt.Errorf("only directory and block device are supported")
}
}
}
func (dm *deviceMounter) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := dm.MountDevice(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
return volumetypes.OperationFinished, mountInternal()
}
func getVolumeSourceFSType(spec *volume.Spec) (string, error) {

View File

@ -231,7 +231,7 @@ func TestBlockDeviceGlobalPathAndMountDevice(t *testing.T) {
fmt.Println("expected global path is:", expectedGlobalPath)
err = dm.MountDevice(pvSpec, tmpBlockDir, expectedGlobalPath)
_, err = dm.MountDevice(pvSpec, tmpBlockDir, expectedGlobalPath)
if err != nil {
t.Fatal(err)
}
@ -276,7 +276,7 @@ func TestFSGlobalPathAndMountDevice(t *testing.T) {
}
// Actually, we will do nothing if the local path is FS type
err = dm.MountDevice(pvSpec, tmpFSDir, expectedGlobalPath)
_, err = dm.MountDevice(pvSpec, tmpFSDir, expectedGlobalPath)
if err != nil {
t.Fatal(err)
}

View File

@ -144,10 +144,7 @@ func (attacher *rbdAttacher) GetDeviceMountPath(spec *volume.Spec) (string, erro
return makePDNameInternal(attacher.plugin.host, pool, img), nil
}
// MountDevice implements Attacher.MountDevice. It is called by the kubelet to
// mount device at the given mount path.
// This method is idempotent, callers are responsible for retrying on failure.
func (attacher *rbdAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
func (attacher *rbdAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error {
klog.V(4).Infof("rbd: mouting device %s to %s", devicePath, deviceMountPath)
notMnt, err := attacher.mounter.IsLikelyNotMountPoint(deviceMountPath)
if err != nil {
@ -185,8 +182,11 @@ func (attacher *rbdAttacher) MountDevice(spec *volume.Spec, devicePath string, d
return nil
}
func (attacher *rbdAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
// MountDevice implements Attacher.MountDevice. It is called by the kubelet to
// mount device at the given mount path.
// This method is idempotent, callers are responsible for retrying on failure.
func (attacher *rbdAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
}

View File

@ -281,7 +281,7 @@ func doTestPlugin(t *testing.T, c *testcase) {
if deviceMountPath != c.expectedDeviceMountPath {
t.Errorf("Unexpected mount path, expected %q, not: %q", c.expectedDeviceMountPath, deviceMountPath)
}
err = attacher.MountDevice(c.spec, devicePath, deviceMountPath)
_, err = attacher.MountDevice(c.spec, devicePath, deviceMountPath)
if err != nil {
t.Fatal(err)
}

View File

@ -1058,7 +1058,7 @@ func (fv *FakeVolume) GetDeviceMountPath(spec *Spec) (string, error) {
return "", nil
}
func (fv *FakeVolume) MountDevice(spec *Spec, devicePath string, deviceMountPath string) error {
func (fv *FakeVolume) mountDeviceInternal(spec *Spec, devicePath string, deviceMountPath string) error {
fv.Lock()
defer fv.Unlock()
if spec.Name() == TimeoutOnMountDeviceVolumeName {
@ -1086,8 +1086,8 @@ func (fv *FakeVolume) MountDevice(spec *Spec, devicePath string, deviceMountPath
return nil
}
func (fv *FakeVolume) MountDeviceWithStatusTracking(spec *Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := fv.MountDevice(spec, devicePath, deviceMountPath)
func (fv *FakeVolume) MountDevice(spec *Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := fv.mountDeviceInternal(spec, devicePath, deviceMountPath)
if volumetypes.IsOperationTimeOutError(err) {
return volumetypes.OperationInProgress, err
}

View File

@ -575,7 +575,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
}
// Mount device to global mount path
operationState, err := volumeDeviceMounter.MountDeviceWithStatusTracking(
operationState, err := volumeDeviceMounter.MountDevice(
volumeToMount.VolumeSpec,
devicePath,
deviceMountPath)

View File

@ -253,10 +253,7 @@ type DeviceMounter interface {
// MountDevice mounts the disk to a global path which
// individual pods can then bind mount
// Note that devicePath can be empty if the volume plugin does not implement any of Attach and WaitForAttach methods.
MountDevice(spec *Spec, devicePath string, deviceMountPath string) error
// MountDeviceWithStatusTracking is same as MountDevice except status of mount operation is also returned
MountDeviceWithStatusTracking(spec *Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error)
MountDevice(spec *Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error)
}
type BulkVolumeVerifier interface {

View File

@ -208,8 +208,7 @@ func (plugin *vsphereVolumePlugin) GetDeviceMountRefs(deviceMountPath string) ([
return mounter.GetMountRefs(deviceMountPath)
}
// MountDevice mounts device to global mount point.
func (attacher *vsphereVMDKAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error {
func (attacher *vsphereVMDKAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error {
klog.Info("vsphere MountDevice", devicePath, deviceMountPath)
mounter := attacher.host.GetMounter(vsphereVolumePluginName)
notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath)
@ -249,8 +248,9 @@ func (attacher *vsphereVMDKAttacher) MountDevice(spec *volume.Spec, devicePath s
return nil
}
func (attacher *vsphereVMDKAttacher) MountDeviceWithStatusTracking(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.MountDevice(spec, devicePath, deviceMountPath)
// MountDevice mounts device to global mount point.
func (attacher *vsphereVMDKAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) {
err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath)
return volumetypes.OperationFinished, err
}