Merge pull request #89172 from jsafrane/fix-mount-opts

Fix mountOptions in iSCSI and FC volume plugins

pkg/volume/fc/fc_test.go

@@ -24,7 +24,7 @@ import (
 	"strings"
 	"testing"
 
-	"k8s.io/utils/exec/testing"
+	testingexec "k8s.io/utils/exec/testing"
 	"k8s.io/utils/mount"
 
 	v1 "k8s.io/api/core/v1"
@@ -122,10 +122,6 @@ func (fake *fakeDiskManager) AttachDisk(b fcDiskMounter) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	// Simulate the global mount so that the fakeMounter returns the
-	// expected number of mounts for the attached disk.
-	b.mounter.Mount(globalPath, globalPath, b.fsType, nil)
-
 	fake.attachCalled = true
 	return "", nil
 }

pkg/volume/fc/fc_util.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/klog"
 	"k8s.io/utils/mount"
 
-	v1 "k8s.io/api/core/v1"
 	"k8s.io/kubernetes/pkg/volume"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 )
@@ -242,34 +241,7 @@ func (util *fcUtil) AttachDisk(b fcDiskMounter) (string, error) {
 		return "", err
 	}
 
-	// If the volumeMode is 'Block', plugin don't have to format the volume.
+	return devicePath, nil
-	// The globalPDPath will be created by operationexecutor. Just return devicePath here.
-	klog.V(5).Infof("fc: AttachDisk volumeMode: %s, devicePath: %s", b.volumeMode, devicePath)
-	if b.volumeMode == v1.PersistentVolumeBlock {
-		return devicePath, nil
-	}
-
-	// mount it
-	globalPDPath := util.MakeGlobalPDName(*b.fcDisk)
-	if err := os.MkdirAll(globalPDPath, 0750); err != nil {
-		return devicePath, fmt.Errorf("fc: failed to mkdir %s, error", globalPDPath)
-	}
-
-	noMnt, err := b.mounter.IsLikelyNotMountPoint(globalPDPath)
-	if err != nil {
-		return devicePath, fmt.Errorf("Heuristic determination of mount point failed:%v", err)
-	}
-	if !noMnt {
-		klog.Infof("fc: %s already mounted", globalPDPath)
-		return devicePath, nil
-	}
-
-	err = b.mounter.FormatAndMount(devicePath, globalPDPath, b.fsType, b.mountOptions)
-	if err != nil {
-		return devicePath, fmt.Errorf("fc: failed to mount fc volume %s [%s] to %s, error %v", devicePath, b.fsType, globalPDPath, err)
-	}
-
-	return devicePath, err
 }
 
 // DetachDisk removes scsi device file such as /dev/sdX from the node.

pkg/volume/iscsi/BUILD

@@ -20,6 +20,7 @@ go_library(
         "//pkg/kubelet/config:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
+        "//pkg/volume/util/types:go_default_library",
         "//pkg/volume/util/volumepathhandler:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

pkg/volume/iscsi/iscsi_util.go

@@ -37,6 +37,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/config"
 	"k8s.io/kubernetes/pkg/volume"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
+	"k8s.io/kubernetes/pkg/volume/util/types"
 )
 
 const (
@@ -194,7 +195,9 @@ func (util *ISCSIUtil) MakeGlobalVDPDName(iscsi iscsiDisk) string {
 	return makeVDPDNameInternal(iscsi.plugin.host, iscsi.Portals[0], iscsi.Iqn, iscsi.Lun, iscsi.Iface)
 }
 
-func (util *ISCSIUtil) persistISCSI(conf iscsiDisk, mnt string) error {
+// persistISCSIFile saves iSCSI volume configuration for DetachDisk
+// into given directory.
+func (util *ISCSIUtil) persistISCSIFile(conf iscsiDisk, mnt string) error {
 	file := filepath.Join(mnt, "iscsi.json")
 	fp, err := os.Create(file)
 	if err != nil {
@@ -457,61 +460,32 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) (string, error) {
 	}
 
 	klog.V(5).Infof("iscsi: AttachDisk devicePath: %s", devicePath)
-	// run global mount path related operations based on volumeMode
+
-	return globalPDPathOperation(b)(b, devicePath, util)
+	if err = util.persistISCSI(b); err != nil {
+		// Return uncertain error so kubelet calls Unmount / Unmap when the pod
+		// is deleted.
+		return "", types.NewUncertainProgressError(err.Error())
+	}
+	return devicePath, nil
 }
 
-// globalPDPathOperation returns global mount path related operations based on volumeMode.
+// persistISCSI saves iSCSI volume configuration for DetachDisk into global
-// If the volumeMode is 'Filesystem' or not defined, plugin needs to create a dir, persist
+// mount / map directory.
-// iscsi configurations, and then format/mount the volume.
+func (util *ISCSIUtil) persistISCSI(b iscsiDiskMounter) error {
-// If the volumeMode is 'Block', plugin creates a dir and persists iscsi configurations.
-// Since volume type is block, plugin doesn't need to format/mount the volume.
-func globalPDPathOperation(b iscsiDiskMounter) func(iscsiDiskMounter, string, *ISCSIUtil) (string, error) {
 	klog.V(5).Infof("iscsi: AttachDisk volumeMode: %s", b.volumeMode)
+	var globalPDPath string
 	if b.volumeMode == v1.PersistentVolumeBlock {
-		// If the volumeMode is 'Block', plugin don't need to format the volume.
+		globalPDPath = b.manager.MakeGlobalVDPDName(*b.iscsiDisk)
-		return func(b iscsiDiskMounter, devicePath string, util *ISCSIUtil) (string, error) {
+	} else {
-			globalPDPath := b.manager.MakeGlobalVDPDName(*b.iscsiDisk)
+		globalPDPath = b.manager.MakeGlobalPDName(*b.iscsiDisk)
-			// Create dir like /var/lib/kubelet/plugins/kubernetes.io/iscsi/volumeDevices/{ifaceName}/{portal-some_iqn-lun-lun_id}
-			if err := os.MkdirAll(globalPDPath, 0750); err != nil {
-				klog.Errorf("iscsi: failed to mkdir %s, error", globalPDPath)
-				return "", err
-			}
-			// Persist iscsi disk config to json file for DetachDisk path
-			util.persistISCSI(*(b.iscsiDisk), globalPDPath)
-
-			return devicePath, nil
-		}
 	}
 
-	// If the volumeMode is 'Filesystem', plugin needs to format the volume
+	if err := os.MkdirAll(globalPDPath, 0750); err != nil {
-	// and mount it to globalPDPath.
+		klog.Errorf("iscsi: failed to mkdir %s, error", globalPDPath)
-	return func(b iscsiDiskMounter, devicePath string, util *ISCSIUtil) (string, error) {
+		return err
-		globalPDPath := b.manager.MakeGlobalPDName(*b.iscsiDisk)
-		notMnt, err := b.mounter.IsLikelyNotMountPoint(globalPDPath)
-		if err != nil && !os.IsNotExist(err) {
-			return "", fmt.Errorf("Heuristic determination of mount point failed:%v", err)
-		}
-		// Return confirmed devicePath to caller
-		if !notMnt {
-			klog.Infof("iscsi: %s already mounted", globalPDPath)
-			return devicePath, nil
-		}
-		// Create dir like /var/lib/kubelet/plugins/kubernetes.io/iscsi/{ifaceName}/{portal-some_iqn-lun-lun_id}
-		if err := os.MkdirAll(globalPDPath, 0750); err != nil {
-			klog.Errorf("iscsi: failed to mkdir %s, error", globalPDPath)
-			return "", err
-		}
-		// Persist iscsi disk config to json file for DetachDisk path
-		util.persistISCSI(*(b.iscsiDisk), globalPDPath)
-
-		err = b.mounter.FormatAndMount(devicePath, globalPDPath, b.fsType, nil)
-		if err != nil {
-			klog.Errorf("iscsi: failed to mount iscsi volume %s [%s] to %s, error %v", devicePath, b.fsType, globalPDPath, err)
-		}
-
-		return devicePath, nil
 	}
+	// Persist iscsi disk config to json file for DetachDisk path
+	return util.persistISCSIFile(*(b.iscsiDisk), globalPDPath)
 }
 
 // Delete 1 block device of the form "sd*"