Move MapBlockVolume call to operation_generator and add UnmapBlockVolume

pkg/volume/awsebs/aws_ebs_block.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	"k8s.io/legacy-cloud-providers/aws"
 	utilstrings "k8s.io/utils/strings"
@@ -148,7 +147,7 @@ func (b *awsElasticBlockStoreMapper) SetUpDevice() (string, error) {
 }
 
 func (b *awsElasticBlockStoreMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return util.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 // GetGlobalMapPath returns global map path and error

pkg/volume/azure_dd/azure_dd_block.go

@@ -28,7 +28,6 @@ import (
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	utilstrings "k8s.io/utils/strings"
 )
@@ -141,7 +140,7 @@ func (b *azureDataDiskMapper) SetUpDevice() (string, error) {
 }
 
 func (b *azureDataDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return util.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 // GetGlobalMapPath returns global map path and error

pkg/volume/cinder/cinder_block.go

@@ -28,7 +28,6 @@ import (
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	utilstrings "k8s.io/utils/strings"
 )
@@ -151,7 +150,7 @@ func (b *cinderVolumeMapper) SetUpDevice() (string, error) {
 }
 
 func (b *cinderVolumeMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return util.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 // GetGlobalMapPath returns global map path and error

pkg/volume/csi/csi_block.go

@@ -31,7 +31,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/pkg/volume"
-	ioutil "k8s.io/kubernetes/pkg/volume/util"
 	utilstrings "k8s.io/utils/strings"
 )
 
@@ -267,7 +266,7 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) {
 }
 
 func (m *csiBlockMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return ioutil.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 var _ volume.BlockVolumeUnmapper = &csiBlockMapper{}

pkg/volume/csi/csi_block_test.go

@@ -315,44 +315,12 @@ func TestBlockMapperMapDevice(t *testing.T) {
 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
 	}
 
-	// Actual SetupDevice should create a symlink to or a bind mout of device in devicePath.
-	// Create dummy file there before calling MapDevice to test it properly.
-	fd, err := os.Create(devicePath)
-	if err != nil {
-		t.Fatalf("mapper failed to create dummy file in devicePath: %v", err)
-	}
-	if err := fd.Close(); err != nil {
-		t.Fatalf("mapper failed to close dummy file in devicePath: %v", err)
-	}
-
 	// Map device to global and pod device map path
 	volumeMapPath, volName := csiMapper.GetPodDeviceMapPath()
 	err = csiMapper.MapDevice(devicePath, globalMapPath, volumeMapPath, volName, csiMapper.podUID)
 	if err != nil {
 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
 	}
-
-	// Check if symlink {globalMapPath}/{podUID} exists
-	globalMapFilePath := filepath.Join(globalMapPath, string(csiMapper.podUID))
-	if _, err := os.Stat(globalMapFilePath); err != nil {
-		if os.IsNotExist(err) {
-			t.Errorf("mapper.MapDevice failed, symlink in globalMapPath not created: %v", err)
-			t.Errorf("mapper.MapDevice devicePath:%v, globalMapPath: %v, globalMapFilePath: %v",
-				devicePath, globalMapPath, globalMapFilePath)
-		} else {
-			t.Errorf("mapper.MapDevice failed: %v", err)
-		}
-	}
-
-	// Check if symlink {volumeMapPath}/{volName} exists
-	volumeMapFilePath := filepath.Join(volumeMapPath, volName)
-	if _, err := os.Stat(volumeMapFilePath); err != nil {
-		if os.IsNotExist(err) {
-			t.Errorf("mapper.MapDevice failed, symlink in volumeMapPath not created: %v", err)
-		} else {
-			t.Errorf("mapper.MapDevice failed: %v", err)
-		}
-	}
 }
 
 func TestBlockMapperMapDeviceNotSupportAttach(t *testing.T) {
@@ -402,44 +370,12 @@ func TestBlockMapperMapDeviceNotSupportAttach(t *testing.T) {
 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
 	}
 
-	// Actual SetupDevice should create a symlink to or a bind mout of device in devicePath.
-	// Create dummy file there before calling MapDevice to test it properly.
-	fd, err := os.Create(devicePath)
-	if err != nil {
-		t.Fatalf("mapper failed to create dummy file in devicePath: %v", err)
-	}
-	if err := fd.Close(); err != nil {
-		t.Fatalf("mapper failed to close dummy file in devicePath: %v", err)
-	}
-
 	// Map device to global and pod device map path
 	volumeMapPath, volName := csiMapper.GetPodDeviceMapPath()
 	err = csiMapper.MapDevice(devicePath, globalMapPath, volumeMapPath, volName, csiMapper.podUID)
 	if err != nil {
 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
 	}
-
-	// Check if symlink {globalMapPath}/{podUID} exists
-	globalMapFilePath := filepath.Join(globalMapPath, string(csiMapper.podUID))
-	if _, err := os.Stat(globalMapFilePath); err != nil {
-		if os.IsNotExist(err) {
-			t.Errorf("mapper.MapDevice failed, symlink in globalMapPath not created: %v", err)
-			t.Errorf("mapper.MapDevice devicePath:%v, globalMapPath: %v, globalMapFilePath: %v",
-				devicePath, globalMapPath, globalMapFilePath)
-		} else {
-			t.Errorf("mapper.MapDevice failed: %v", err)
-		}
-	}
-
-	// Check if symlink {volumeMapPath}/{volName} exists
-	volumeMapFilePath := filepath.Join(volumeMapPath, volName)
-	if _, err := os.Stat(volumeMapFilePath); err != nil {
-		if os.IsNotExist(err) {
-			t.Errorf("mapper.MapDevice failed, symlink in volumeMapPath not created: %v", err)
-		} else {
-			t.Errorf("mapper.MapDevice failed: %v", err)
-		}
-	}
 }
 
 func TestBlockMapperTearDownDevice(t *testing.T) {

pkg/volume/fc/fc.go

@@ -419,7 +419,7 @@ func (b *fcDiskMapper) SetUpDevice() (string, error) {
 }
 
 func (b *fcDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return util.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 type fcDiskUnmapper struct {

pkg/volume/gcepd/gce_pd_block.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	utilstrings "k8s.io/utils/strings"
 )
@@ -158,7 +157,7 @@ func (b *gcePersistentDiskMapper) SetUpDevice() (string, error) {
 }
 
 func (b *gcePersistentDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return util.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 // GetGlobalMapPath returns global map path and error

pkg/volume/iscsi/iscsi.go

@@ -389,7 +389,7 @@ func (b *iscsiDiskMapper) SetUpDevice() (string, error) {
 }
 
 func (b *iscsiDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return ioutil.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 type iscsiDiskUnmapper struct {

pkg/volume/local/local.go

@@ -621,7 +621,7 @@ func (m *localVolumeMapper) SetUpDevice() (string, error) {
 }
 
 func (m *localVolumeMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return util.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 // localVolumeUnmapper implements the BlockVolumeUnmapper interface for local volumes.

pkg/volume/rbd/rbd.go

@@ -917,7 +917,7 @@ func (rbd *rbdDiskMapper) SetUpDevice() (string, error) {
 }
 
 func (rbd *rbdDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return volutil.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 func (rbd *rbd) rbdGlobalMapPath(spec *volume.Spec) (string, error) {

pkg/volume/util/operationexecutor/operation_generator.go

@@ -39,6 +39,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/csi"
 	"k8s.io/kubernetes/pkg/volume/util"
+	ioutil "k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/hostutil"
 	volumetypes "k8s.io/kubernetes/pkg/volume/util/types"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
@@ -1084,21 +1085,19 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
 			return volumeToMount.GenerateError("MapVolume.EvalHostSymlinks failed", err)
 		}
 
-		// Map device to global and pod device map path
+		// Execute driver specific map
 		volumeMapPath, volName := blockVolumeMapper.GetPodDeviceMapPath()
 		mapErr = blockVolumeMapper.MapDevice(devicePath, globalMapPath, volumeMapPath, volName, volumeToMount.Pod.UID)
 		if mapErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return volumeToMount.GenerateError("MapVolume.MapDevice failed", mapErr)
+			return volumeToMount.GenerateError("MapVolume.MapPodDevice failed", mapErr)
 		}
 
-		// Take filedescriptor lock to keep a block device opened. Otherwise, there is a case
+		// Execute common map
-		// that the block device is silently removed and attached another device with same name.
+		mapErr = ioutil.MapBlockVolume(og.blkUtil, devicePath, globalMapPath, volumeMapPath, volName, volumeToMount.Pod.UID)
-		// Container runtime can't handler this problem. To avoid unexpected condition fd lock
+		if mapErr != nil {
-		// for the block device is required.
+			// On failure, return error. Caller will log and retry.
-		_, err = og.blkUtil.AttachFileDevice(filepath.Join(globalMapPath, string(volumeToMount.Pod.UID)))
+			return volumeToMount.GenerateError("MapVolume.MapDevice failed", mapErr)
-		if err != nil {
-			return volumeToMount.GenerateError("MapVolume.AttachFileDevice failed", err)
 		}
 
 		// Update actual state of world to reflect volume is globally mounted
@@ -1207,28 +1206,16 @@ func (og *operationGenerator) GenerateUnmapVolumeFunc(
 	}
 
 	unmapVolumeFunc := func() (error, error) {
-		globalUnmapPath := volumeToUnmount.DeviceMountPath
-
-		// Release file descriptor lock.
-		err := og.blkUtil.DetachFileDevice(filepath.Join(globalUnmapPath, string(volumeToUnmount.PodUID)))
-		if err != nil {
-			return volumeToUnmount.GenerateError("UnmapVolume.UnmapDevice Detaching descriptor lock failed", err)
-		}
-
-		// Try to unmap volumeName symlink under pod device map path dir
 		// pods/{podUid}/volumeDevices/{escapeQualifiedPluginName}/{volumeName}
 		podDeviceUnmapPath, volName := blockVolumeUnmapper.GetPodDeviceMapPath()
-		unmapDeviceErr := og.blkUtil.UnmapDevice(podDeviceUnmapPath, volName, false /* bindMount */)
-		if unmapDeviceErr != nil {
-			// On failure, return error. Caller will log and retry.
-			return volumeToUnmount.GenerateError("UnmapVolume.UnmapDevice on pod device map path failed", unmapDeviceErr)
-		}
-		// Try to unmap podUID symlink under global map path dir
 		// plugins/kubernetes.io/{PluginName}/volumeDevices/{volumePluginDependentPath}/{podUID}
-		unmapDeviceErr = og.blkUtil.UnmapDevice(globalUnmapPath, string(volumeToUnmount.PodUID), true /* bindMount */)
+		globalUnmapPath := volumeToUnmount.DeviceMountPath
-		if unmapDeviceErr != nil {
+
+		// Execute common unmap
+		unmapErr := ioutil.UnmapBlockVolume(og.blkUtil, globalUnmapPath, podDeviceUnmapPath, volName, volumeToUnmount.PodUID)
+		if unmapErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return volumeToUnmount.GenerateError("UnmapVolume.UnmapDevice on global map path failed", unmapDeviceErr)
+			return volumeToUnmount.GenerateError("UnmapVolume.UnmapBlockVolume failed", unmapErr)
 		}
 
 		klog.Infof(

pkg/volume/util/util.go

@@ -503,18 +503,17 @@ func MakeAbsolutePath(goos, path string) string {
 	return "c:\\" + path
 }
 
-// MapBlockVolume is a utility function to provide a common way of mounting
+// MapBlockVolume is a utility function to provide a common way of mapping
 // block device path for a specified volume and pod.  This function should be
 // called by volume plugins that implements volume.BlockVolumeMapper.Map() method.
 func MapBlockVolume(
+	blkUtil volumepathhandler.BlockVolumePathHandler,
 	devicePath,
 	globalMapPath,
 	podVolumeMapPath,
 	volumeMapName string,
 	podUID utypes.UID,
 ) error {
-	blkUtil := volumepathhandler.NewBlockVolumePathHandler()
-
 	// map devicePath to global node path as bind mount
 	mapErr := blkUtil.MapDevice(devicePath, globalMapPath, string(podUID), true /* bindMount */)
 	if mapErr != nil {
@@ -529,6 +528,49 @@ func MapBlockVolume(
 			devicePath, podVolumeMapPath, volumeMapName, false, mapErr)
 	}
 
+	// Take file descriptor lock to keep a block device opened. Otherwise, there is a case
+	// that the block device is silently removed and attached another device with the same name.
+	// Container runtime can't handle this problem. To avoid unexpected condition fd lock
+	// for the block device is required.
+	_, mapErr = blkUtil.AttachFileDevice(filepath.Join(globalMapPath, string(podUID)))
+	if mapErr != nil {
+		return fmt.Errorf("blkUtil.AttachFileDevice failed. globalMapPath:%s, podUID: %s: %v",
+			globalMapPath, string(podUID), mapErr)
+	}
+
+	return nil
+}
+
+// UnmapBlockVolume is a utility function to provide a common way of unmapping
+// block device path for a specified volume and pod.  This function should be
+// called by volume plugins that implements volume.BlockVolumeMapper.Map() method.
+func UnmapBlockVolume(
+	blkUtil volumepathhandler.BlockVolumePathHandler,
+	globalUnmapPath,
+	podDeviceUnmapPath,
+	volumeMapName string,
+	podUID utypes.UID,
+) error {
+	// Release file descriptor lock.
+	err := blkUtil.DetachFileDevice(filepath.Join(globalUnmapPath, string(podUID)))
+	if err != nil {
+		return fmt.Errorf("blkUtil.DetachFileDevice failed. globalUnmapPath:%s, podUID: %s: %v",
+			globalUnmapPath, string(podUID), err)
+	}
+
+	// unmap devicePath from pod volume path
+	unmapDeviceErr := blkUtil.UnmapDevice(podDeviceUnmapPath, volumeMapName, false /* bindMount */)
+	if unmapDeviceErr != nil {
+		return fmt.Errorf("blkUtil.DetachFileDevice failed. podDeviceUnmapPath:%s, volumeMapName: %s, bindMount: %v: %v",
+			podDeviceUnmapPath, volumeMapName, false, unmapDeviceErr)
+	}
+
+	// unmap devicePath from global node path
+	unmapDeviceErr = blkUtil.UnmapDevice(globalUnmapPath, string(podUID), true /* bindMount */)
+	if unmapDeviceErr != nil {
+		return fmt.Errorf("blkUtil.DetachFileDevice failed. globalUnmapPath:%s, podUID: %s, bindMount: %v: %v",
+			globalUnmapPath, string(podUID), true, unmapDeviceErr)
+	}
 	return nil
 }
 

pkg/volume/vsphere_volume/vsphere_volume_block.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	utilstrings "k8s.io/utils/strings"
 )
@@ -138,7 +137,7 @@ func (v vsphereBlockVolumeMapper) SetUpDevice() (string, error) {
 }
 
 func (v vsphereBlockVolumeMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return util.MapBlockVolume(devicePath, globalMapPath, volumeMapPath, volumeMapName, podUID)
+	return nil
 }
 
 var _ volume.BlockVolumeUnmapper = &vsphereBlockVolumeUnmapper{}