Merge pull request #84747 from mkimuram/refactor-mapper

Refactor mapper/unmapper of block volume

pkg/kubelet/kubelet_volumes_test.go

@@ -555,10 +555,14 @@ func (f *stubBlockVolume) SetUpDevice() (string, error) {
 	return "", nil
 }
 
-func (f stubBlockVolume) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
+func (f stubBlockVolume) MapPodDevice() error {
 	return nil
 }
 
 func (f *stubBlockVolume) TearDownDevice(mapPath string, devicePath string) error {
 	return nil
 }
+
+func (f *stubBlockVolume) UnmapPodDevice() error {
+	return nil
+}

pkg/kubelet/volumemanager/reconciler/reconciler_test.go

@@ -527,7 +527,7 @@ func Test_Run_Positive_VolumeAttachAndMap(t *testing.T) {
 		1 /* expectedAttachCallCount */, fakePlugin))
 	assert.NoError(t, volumetesting.VerifyWaitForAttachCallCount(
 		1 /* expectedWaitForAttachCallCount */, fakePlugin))
-	assert.NoError(t, volumetesting.VerifyGetMapDeviceCallCount(
+	assert.NoError(t, volumetesting.VerifyGetMapPodDeviceCallCount(
 		1 /* expectedGetMapDeviceCallCount */, fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroTearDownDeviceCallCount(fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroDetachCallCount(fakePlugin))
@@ -631,7 +631,7 @@ func Test_Run_Positive_BlockVolumeMapControllerAttachEnabled(t *testing.T) {
 	assert.NoError(t, volumetesting.VerifyZeroAttachCalls(fakePlugin))
 	assert.NoError(t, volumetesting.VerifyWaitForAttachCallCount(
 		1 /* expectedWaitForAttachCallCount */, fakePlugin))
-	assert.NoError(t, volumetesting.VerifyGetMapDeviceCallCount(
+	assert.NoError(t, volumetesting.VerifyGetMapPodDeviceCallCount(
 		1 /* expectedGetMapDeviceCallCount */, fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroTearDownDeviceCallCount(fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroDetachCallCount(fakePlugin))
@@ -731,7 +731,7 @@ func Test_Run_Positive_BlockVolumeAttachMapUnmapDetach(t *testing.T) {
 		1 /* expectedAttachCallCount */, fakePlugin))
 	assert.NoError(t, volumetesting.VerifyWaitForAttachCallCount(
 		1 /* expectedWaitForAttachCallCount */, fakePlugin))
-	assert.NoError(t, volumetesting.VerifyGetMapDeviceCallCount(
+	assert.NoError(t, volumetesting.VerifyGetMapPodDeviceCallCount(
 		1 /* expectedGetMapDeviceCallCount */, fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroTearDownDeviceCallCount(fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroDetachCallCount(fakePlugin))
@@ -847,7 +847,7 @@ func Test_Run_Positive_VolumeUnmapControllerAttachEnabled(t *testing.T) {
 	assert.NoError(t, volumetesting.VerifyZeroAttachCalls(fakePlugin))
 	assert.NoError(t, volumetesting.VerifyWaitForAttachCallCount(
 		1 /* expectedWaitForAttachCallCount */, fakePlugin))
-	assert.NoError(t, volumetesting.VerifyGetMapDeviceCallCount(
+	assert.NoError(t, volumetesting.VerifyGetMapPodDeviceCallCount(
 		1 /* expectedGetMapDeviceCallCount */, fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroTearDownDeviceCallCount(fakePlugin))
 	assert.NoError(t, volumetesting.VerifyZeroDetachCallCount(fakePlugin))

pkg/volume/awsebs/aws_ebs_block.go

@@ -125,10 +125,6 @@ func (plugin *awsElasticBlockStorePlugin) newUnmapperInternal(volName string, po
 		}}, nil
 }
 
-func (c *awsElasticBlockStoreUnmapper) TearDownDevice(mapPath, devicePath string) error {
-	return nil
-}
-
 type awsElasticBlockStoreUnmapper struct {
 	*awsElasticBlockStore
 }
@@ -142,14 +138,6 @@ type awsElasticBlockStoreMapper struct {
 
 var _ volume.BlockVolumeMapper = &awsElasticBlockStoreMapper{}
 
-func (b *awsElasticBlockStoreMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (b *awsElasticBlockStoreMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/volumeID
 //       plugins/kubernetes.io/aws-ebs/volumeDevices/vol-XXXXXX

pkg/volume/azure_dd/azure_dd_block.go

@@ -118,10 +118,6 @@ func (plugin *azureDataDiskPlugin) newUnmapperInternal(volName string, podUID ty
 	return &azureDataDiskUnmapper{dataDisk: disk}, nil
 }
 
-func (c *azureDataDiskUnmapper) TearDownDevice(mapPath, devicePath string) error {
-	return nil
-}
-
 type azureDataDiskUnmapper struct {
 	*dataDisk
 }
@@ -135,14 +131,6 @@ type azureDataDiskMapper struct {
 
 var _ volume.BlockVolumeMapper = &azureDataDiskMapper{}
 
-func (b *azureDataDiskMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (b *azureDataDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/volumeID
 //       plugins/kubernetes.io/azure-disk/volumeDevices/vol-XXXXXX

pkg/volume/cinder/cinder_block.go

@@ -128,10 +128,6 @@ func (plugin *cinderPlugin) newUnmapperInternal(volName string, podUID types.UID
 		}}, nil
 }
 
-func (c *cinderPluginUnmapper) TearDownDevice(mapPath, devicePath string) error {
-	return nil
-}
-
 type cinderPluginUnmapper struct {
 	*cinderVolume
 }
@@ -145,14 +141,6 @@ type cinderVolumeMapper struct {
 
 var _ volume.BlockVolumeMapper = &cinderVolumeMapper{}
 
-func (b *cinderVolumeMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (b *cinderVolumeMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/volumeID
 //       plugins/kubernetes.io/cinder/volumeDevices/vol-XXXXXX

pkg/volume/csi/csi_block.go

@@ -48,6 +48,7 @@ type csiBlockMapper struct {
 }
 
 var _ volume.BlockVolumeMapper = &csiBlockMapper{}
+var _ volume.CustomBlockVolumeMapper = &csiBlockMapper{}
 
 // GetGlobalMapPath returns a global map path (on the node) to a device file which will be symlinked to
 // Example: plugins/kubernetes.io/csi/volumeDevices/{pvname}/dev
@@ -203,26 +204,26 @@ func (m *csiBlockMapper) publishVolumeForBlock(
 }
 
 // SetUpDevice ensures the device is attached returns path where the device is located.
-func (m *csiBlockMapper) SetUpDevice() (string, error) {
+func (m *csiBlockMapper) SetUpDevice() error {
 	if !m.plugin.blockEnabled {
-		return "", errors.New("CSIBlockVolume feature not enabled")
+		return errors.New("CSIBlockVolume feature not enabled")
 	}
 	klog.V(4).Infof(log("blockMapper.SetUpDevice called"))
 
 	// Get csiSource from spec
 	if m.spec == nil {
-		return "", errors.New(log("blockMapper.SetUpDevice spec is nil"))
+		return errors.New(log("blockMapper.SetUpDevice spec is nil"))
 	}
 
 	csiSource, err := getCSISourceFromSpec(m.spec)
 	if err != nil {
-		return "", errors.New(log("blockMapper.SetUpDevice failed to get CSI persistent source: %v", err))
+		return errors.New(log("blockMapper.SetUpDevice failed to get CSI persistent source: %v", err))
 	}
 
 	driverName := csiSource.Driver
 	skip, err := m.plugin.skipAttach(driverName)
 	if err != nil {
-		return "", errors.New(log("blockMapper.SetupDevice failed to check CSIDriver for %s: %v", driverName, err))
+		return errors.New(log("blockMapper.SetupDevice failed to check CSIDriver for %s: %v", driverName, err))
 	}
 
 	var attachment *storage.VolumeAttachment
@@ -232,7 +233,7 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) {
 		attachID := getAttachmentName(csiSource.VolumeHandle, csiSource.Driver, nodeName)
 		attachment, err = m.k8s.StorageV1().VolumeAttachments().Get(attachID, meta.GetOptions{})
 		if err != nil {
-			return "", errors.New(log("blockMapper.SetupDevice failed to get volume attachment [id=%v]: %v", attachID, err))
+			return errors.New(log("blockMapper.SetupDevice failed to get volume attachment [id=%v]: %v", attachID, err))
 		}
 	}
 
@@ -247,29 +248,30 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) {
 
 	csiClient, err := m.csiClientGetter.Get()
 	if err != nil {
-		return "", errors.New(log("blockMapper.SetUpDevice failed to get CSI client: %v", err))
+		return errors.New(log("blockMapper.SetUpDevice failed to get CSI client: %v", err))
 	}
 
 	// Call NodeStageVolume
 	stagingPath, err := m.stageVolumeForBlock(ctx, csiClient, accessMode, csiSource, attachment)
 	if err != nil {
-		return "", err
+		return err
 	}
 
 	// Call NodePublishVolume
-	publishPath, err := m.publishVolumeForBlock(ctx, csiClient, accessMode, csiSource, attachment, stagingPath)
+	_, err = m.publishVolumeForBlock(ctx, csiClient, accessMode, csiSource, attachment, stagingPath)
 	if err != nil {
-		return "", err
+		return err
 	}
 
-	return publishPath, nil
-}
-
-func (m *csiBlockMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
 	return nil
 }
 
+func (m *csiBlockMapper) MapPodDevice() (string, error) {
+	return m.getPublishPath(), nil
+}
+
 var _ volume.BlockVolumeUnmapper = &csiBlockMapper{}
+var _ volume.CustomBlockVolumeUnmapper = &csiBlockMapper{}
 
 // unpublishVolumeForBlock unpublishes a block volume from publishPath
 func (m *csiBlockMapper) unpublishVolumeForBlock(ctx context.Context, csi csiClient, publishPath string) error {
@@ -362,3 +364,8 @@ func (m *csiBlockMapper) TearDownDevice(globalMapPath, devicePath string) error
 
 	return nil
 }
+
+// UnmapPodDevice unmaps the block device path.
+func (m *csiBlockMapper) UnmapPodDevice() error {
+	return nil
+}

pkg/volume/csi/csi_block_test.go

@@ -239,17 +239,11 @@ func TestBlockMapperSetupDevice(t *testing.T) {
 	}
 	t.Log("created attachement ", attachID)
 
-	devicePath, err := csiMapper.SetUpDevice()
+	err = csiMapper.SetUpDevice()
 	if err != nil {
 		t.Fatalf("mapper failed to SetupDevice: %v", err)
 	}
 
-	// Check if SetUpDevice returns the right path
-	publishPath := csiMapper.getPublishPath()
-	if devicePath != publishPath {
-		t.Fatalf("mapper.SetupDevice returned unexpected path %s instead of %v", devicePath, publishPath)
-	}
-
 	// Check if NodeStageVolume staged to the right path
 	stagingPath := csiMapper.getStagingPath()
 	svols := csiMapper.csiClient.(*fakeCsiDriverClient).nodeClient.GetNodeStagedVolumes()
@@ -267,12 +261,14 @@ func TestBlockMapperSetupDevice(t *testing.T) {
 	if !ok {
 		t.Error("csi server may not have received NodePublishVolume call")
 	}
+
+	publishPath := csiMapper.getPublishPath()
 	if pvol.Path != publishPath {
 		t.Errorf("csi server expected path %s, got %s", publishPath, pvol.Path)
 	}
 }
 
-func TestBlockMapperMapDevice(t *testing.T) {
+func TestBlockMapperMapPodDevice(t *testing.T) {
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIBlockVolume, true)()
 
 	plug, tmpDir := newTestPlugin(t, nil)
@@ -306,24 +302,18 @@ func TestBlockMapperMapDevice(t *testing.T) {
 	}
 	t.Log("created attachement ", attachID)
 
-	devicePath, err := csiMapper.SetUpDevice()
-	if err != nil {
-		t.Fatalf("mapper failed to SetupDevice: %v", err)
-	}
-	globalMapPath, err := csiMapper.GetGlobalMapPath(csiMapper.spec)
-	if err != nil {
-		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
-	}
-
 	// Map device to global and pod device map path
-	volumeMapPath, volName := csiMapper.GetPodDeviceMapPath()
+	path, err := csiMapper.MapPodDevice()
-	err = csiMapper.MapDevice(devicePath, globalMapPath, volumeMapPath, volName, csiMapper.podUID)
 	if err != nil {
 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
 	}
+	publishPath := csiMapper.getPublishPath()
+	if path != publishPath {
+		t.Errorf("path %s and %s doesn't match", path, publishPath)
+	}
 }
 
-func TestBlockMapperMapDeviceNotSupportAttach(t *testing.T) {
+func TestBlockMapperMapPodDeviceNotSupportAttach(t *testing.T) {
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIBlockVolume, true)()
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIDriverRegistry, true)()
 
@@ -361,21 +351,16 @@ func TestBlockMapperMapDeviceNotSupportAttach(t *testing.T) {
 		t.Fatalf("Failed to make a new Mapper: %v", err)
 	}
 	csiMapper.csiClient = setupClient(t, true)
-	devicePath, err := csiMapper.SetUpDevice()
-	if err != nil {
-		t.Fatalf("mapper failed to SetupDevice: %v", err)
-	}
-	globalMapPath, err := csiMapper.GetGlobalMapPath(csiMapper.spec)
-	if err != nil {
-		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
-	}
 
 	// Map device to global and pod device map path
-	volumeMapPath, volName := csiMapper.GetPodDeviceMapPath()
+	path, err := csiMapper.MapPodDevice()
-	err = csiMapper.MapDevice(devicePath, globalMapPath, volumeMapPath, volName, csiMapper.podUID)
 	if err != nil {
 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err)
 	}
+	publishPath := csiMapper.getPublishPath()
+	if path != publishPath {
+		t.Errorf("path %s and %s doesn't match", path, publishPath)
+	}
 }
 
 func TestBlockMapperTearDownDevice(t *testing.T) {

pkg/volume/fc/fc.go

@@ -414,20 +414,13 @@ type fcDiskMapper struct {
 
 var _ volume.BlockVolumeMapper = &fcDiskMapper{}
 
-func (b *fcDiskMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (b *fcDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 type fcDiskUnmapper struct {
 	*fcDisk
 	deviceUtil util.DeviceUtil
 }
 
 var _ volume.BlockVolumeUnmapper = &fcDiskUnmapper{}
+var _ volume.CustomBlockVolumeUnmapper = &fcDiskUnmapper{}
 
 func (c *fcDiskUnmapper) TearDownDevice(mapPath, devicePath string) error {
 	err := c.manager.DetachBlockFCDisk(*c, mapPath, devicePath)
@@ -442,6 +435,10 @@ func (c *fcDiskUnmapper) TearDownDevice(mapPath, devicePath string) error {
 	return nil
 }
 
+func (c *fcDiskUnmapper) UnmapPodDevice() error {
+	return nil
+}
+
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/{WWID}/{podUid}
 func (fc *fcDisk) GetGlobalMapPath(spec *volume.Spec) (string, error) {

pkg/volume/gcepd/gce_pd_block.go

@@ -135,10 +135,6 @@ func (plugin *gcePersistentDiskPlugin) newUnmapperInternal(volName string, podUI
 		}}, nil
 }
 
-func (c *gcePersistentDiskUnmapper) TearDownDevice(mapPath, devicePath string) error {
-	return nil
-}
-
 type gcePersistentDiskUnmapper struct {
 	*gcePersistentDisk
 }
@@ -152,14 +148,6 @@ type gcePersistentDiskMapper struct {
 
 var _ volume.BlockVolumeMapper = &gcePersistentDiskMapper{}
 
-func (b *gcePersistentDiskMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (b *gcePersistentDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/pdName
 func (pd *gcePersistentDisk) GetGlobalMapPath(spec *volume.Spec) (string, error) {

pkg/volume/iscsi/iscsi.go

@@ -384,14 +384,6 @@ type iscsiDiskMapper struct {
 
 var _ volume.BlockVolumeMapper = &iscsiDiskMapper{}
 
-func (b *iscsiDiskMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (b *iscsiDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 type iscsiDiskUnmapper struct {
 	*iscsiDisk
 	exec       utilexec.Interface
@@ -399,6 +391,7 @@ type iscsiDiskUnmapper struct {
 }
 
 var _ volume.BlockVolumeUnmapper = &iscsiDiskUnmapper{}
+var _ volume.CustomBlockVolumeUnmapper = &iscsiDiskUnmapper{}
 
 // Even though iSCSI plugin has attacher/detacher implementation, iSCSI plugin
 // needs volume detach operation during TearDownDevice(). This method is only
@@ -417,6 +410,10 @@ func (c *iscsiDiskUnmapper) TearDownDevice(mapPath, _ string) error {
 	return nil
 }
 
+func (c *iscsiDiskUnmapper) UnmapPodDevice() error {
+	return nil
+}
+
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/{ifaceName}/{portal-some_iqn-lun-lun_id}
 func (iscsi *iscsiDisk) GetGlobalMapPath(spec *volume.Spec) (string, error) {

pkg/volume/local/local.go

@@ -612,16 +612,18 @@ type localVolumeMapper struct {
 }
 
 var _ volume.BlockVolumeMapper = &localVolumeMapper{}
+var _ volume.CustomBlockVolumeMapper = &localVolumeMapper{}
 
-// SetUpDevice provides physical device path for the local PV.
+// SetUpDevice prepares the volume to the node by the plugin specific way.
-func (m *localVolumeMapper) SetUpDevice() (string, error) {
+func (m *localVolumeMapper) SetUpDevice() error {
-	globalPath := util.MakeAbsolutePath(runtime.GOOS, m.globalPath)
+	return nil
-	klog.V(4).Infof("SetupDevice returning path %s", globalPath)
-	return globalPath, nil
 }
 
-func (m *localVolumeMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
+// MapPodDevice provides physical device path for the local PV.
-	return nil
+func (m *localVolumeMapper) MapPodDevice() (string, error) {
+	globalPath := util.MakeAbsolutePath(runtime.GOOS, m.globalPath)
+	klog.V(4).Infof("MapPodDevice returning path %s", globalPath)
+	return globalPath, nil
 }
 
 // localVolumeUnmapper implements the BlockVolumeUnmapper interface for local volumes.
@@ -631,12 +633,6 @@ type localVolumeUnmapper struct {
 
 var _ volume.BlockVolumeUnmapper = &localVolumeUnmapper{}
 
-// TearDownDevice will undo SetUpDevice procedure. In local PV, all of this already handled by operation_generator.
-func (u *localVolumeUnmapper) TearDownDevice(mapPath, _ string) error {
-	klog.V(4).Infof("local: TearDownDevice completed for: %s", mapPath)
-	return nil
-}
-
 // GetGlobalMapPath returns global map path and error.
 // path: plugins/kubernetes.io/kubernetes.io/local-volume/volumeDevices/{volumeName}
 func (l *localVolume) GetGlobalMapPath(spec *volume.Spec) (string, error) {

pkg/volume/local/local_test.go

@@ -372,9 +372,17 @@ func TestMapUnmap(t *testing.T) {
 	if volName != testPVName {
 		t.Errorf("Got unexpected volNamne: %s, expected %s", volName, testPVName)
 	}
-	devPath, err := mapper.SetUpDevice()
+	var devPath string
-	if err != nil {
+
-		t.Errorf("Failed to SetUpDevice, err: %v", err)
+	if customMapper, ok := mapper.(volume.CustomBlockVolumeMapper); ok {
+		err = customMapper.SetUpDevice()
+		if err != nil {
+			t.Errorf("Failed to SetUpDevice, err: %v", err)
+		}
+		devPath, err = customMapper.MapPodDevice()
+		if err != nil {
+			t.Errorf("Failed to MapPodDevice, err: %v", err)
+		}
 	}
 
 	if _, err := os.Stat(devPath); err != nil {
@@ -393,8 +401,14 @@ func TestMapUnmap(t *testing.T) {
 		t.Fatalf("Got a nil Unmapper")
 	}
 
-	if err := unmapper.TearDownDevice(globalPath, devPath); err != nil {
+	if customUnmapper, ok := unmapper.(volume.CustomBlockVolumeUnmapper); ok {
-		t.Errorf("TearDownDevice failed, err: %v", err)
+		if err := customUnmapper.UnmapPodDevice(); err != nil {
+			t.Errorf("UnmapPodDevice failed, err: %v", err)
+		}
+
+		if err := customUnmapper.TearDownDevice(globalPath, devPath); err != nil {
+			t.Errorf("TearDownDevice failed, err: %v", err)
+		}
 	}
 }
 

pkg/volume/rbd/rbd.go

@@ -898,6 +898,7 @@ type rbdDiskMapper struct {
 }
 
 var _ volume.BlockVolumeUnmapper = &rbdDiskUnmapper{}
+var _ volume.CustomBlockVolumeUnmapper = &rbdDiskUnmapper{}
 
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/{rbd pool}-image-{rbd image-name}/{podUid}
@@ -912,14 +913,6 @@ func (rbd *rbd) GetPodDeviceMapPath() (string, string) {
 	return rbd.rbdPodDeviceMapPath()
 }
 
-func (rbd *rbdDiskMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (rbd *rbdDiskMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 func (rbd *rbd) rbdGlobalMapPath(spec *volume.Spec) (string, error) {
 	mon, err := getVolumeSourceMonitors(spec)
 	if err != nil {
@@ -1003,6 +996,10 @@ func (rbd *rbdDiskUnmapper) TearDownDevice(mapPath, _ string) error {
 	return nil
 }
 
+func (rbd *rbdDiskUnmapper) UnmapPodDevice() error {
+	return nil
+}
+
 func getVolumeSourceMonitors(spec *volume.Spec) ([]string, error) {
 	if spec.Volume != nil && spec.Volume.RBD != nil {
 		return spec.Volume.RBD.CephMonitors, nil

pkg/volume/testing/testing.go

@@ -804,7 +804,8 @@ type FakeVolume struct {
 	GetDeviceMountPathCallCount int
 	SetUpDeviceCallCount        int
 	TearDownDeviceCallCount     int
-	MapDeviceCallCount          int
+	MapPodDeviceCallCount       int
+	UnmapPodDeviceCallCount     int
 	GlobalMapPathCallCount      int
 	PodDeviceMapPathCallCount   int
 }
@@ -880,11 +881,11 @@ func (fv *FakeVolume) TearDownAt(dir string) error {
 }
 
 // Block volume support
-func (fv *FakeVolume) SetUpDevice() (string, error) {
+func (fv *FakeVolume) SetUpDevice() error {
 	fv.Lock()
 	defer fv.Unlock()
 	fv.SetUpDeviceCallCount++
-	return "", nil
+	return nil
 }
 
 // Block volume support
@@ -950,18 +951,33 @@ func (fv *FakeVolume) GetTearDownDeviceCallCount() int {
 }
 
 // Block volume support
-func (fv *FakeVolume) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, pod types.UID) error {
+func (fv *FakeVolume) UnmapPodDevice() error {
 	fv.Lock()
 	defer fv.Unlock()
-	fv.MapDeviceCallCount++
+	fv.UnmapPodDeviceCallCount++
 	return nil
 }
 
 // Block volume support
-func (fv *FakeVolume) GetMapDeviceCallCount() int {
+func (fv *FakeVolume) GetUnmapPodDeviceCallCount() int {
 	fv.RLock()
 	defer fv.RUnlock()
-	return fv.MapDeviceCallCount
+	return fv.UnmapPodDeviceCallCount
+}
+
+// Block volume support
+func (fv *FakeVolume) MapPodDevice() (string, error) {
+	fv.Lock()
+	defer fv.Unlock()
+	fv.MapPodDeviceCallCount++
+	return "", nil
+}
+
+// Block volume support
+func (fv *FakeVolume) GetMapPodDeviceCallCount() int {
+	fv.RLock()
+	defer fv.RUnlock()
+	return fv.MapPodDeviceCallCount
 }
 
 func (fv *FakeVolume) Attach(spec *Spec, nodeName types.NodeName) (string, error) {
@@ -1493,22 +1509,22 @@ func VerifyGetPodDeviceMapPathCallCount(
 		expectedPodDeviceMapPathCallCount)
 }
 
-// VerifyGetMapDeviceCallCount ensures that at least one of the Mappers for this
+// VerifyGetMapPodDeviceCallCount ensures that at least one of the Mappers for this
-// plugin has the expectedMapDeviceCallCount number of calls. Otherwise it
+// plugin has the expectedMapPodDeviceCallCount number of calls. Otherwise it
 // returns an error.
-func VerifyGetMapDeviceCallCount(
+func VerifyGetMapPodDeviceCallCount(
-	expectedMapDeviceCallCount int,
+	expectedMapPodDeviceCallCount int,
 	fakeVolumePlugin *FakeVolumePlugin) error {
 	for _, mapper := range fakeVolumePlugin.GetBlockVolumeMapper() {
-		actualCallCount := mapper.GetMapDeviceCallCount()
+		actualCallCount := mapper.GetMapPodDeviceCallCount()
-		if actualCallCount >= expectedMapDeviceCallCount {
+		if actualCallCount >= expectedMapPodDeviceCallCount {
 			return nil
 		}
 	}
 
 	return fmt.Errorf(
-		"No Mapper have expected MapdDeviceCallCount. Expected: <%v>.",
+		"No Mapper have expected MapPodDeviceCallCount. Expected: <%v>.",
-		expectedMapDeviceCallCount)
+		expectedMapPodDeviceCallCount)
 }
 
 // GetTestVolumePluginMgr creates, initializes, and returns a test volume plugin

pkg/volume/util/operationexecutor/operation_generator.go

@@ -1055,20 +1055,41 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
 			klog.Infof(volumeToMount.GenerateMsgDetailed("MapVolume.WaitForAttach succeeded", fmt.Sprintf("DevicePath %q", devicePath)))
 
 		}
-		// A plugin doesn't have attacher also needs to map device to global map path with SetUpDevice()
+		// Call SetUpDevice if blockVolumeMapper implements CustomBlockVolumeMapper
-		pluginDevicePath, mapErr := blockVolumeMapper.SetUpDevice()
+		if customBlockVolumeMapper, ok := blockVolumeMapper.(volume.CustomBlockVolumeMapper); ok {
-		if mapErr != nil {
+			mapErr := customBlockVolumeMapper.SetUpDevice()
-			// On failure, return error. Caller will log and retry.
+			if mapErr != nil {
-			return volumeToMount.GenerateError("MapVolume.SetUpDevice failed", mapErr)
+				// On failure, return error. Caller will log and retry.
+				return volumeToMount.GenerateError("MapVolume.SetUpDevice failed", mapErr)
+			}
 		}
 
-		// if pluginDevicePath is provided, assume attacher may not provide device
+		// Update actual state of world to reflect volume is globally mounted
-		// or attachment flow uses SetupDevice to get device path
+		markedDevicePath := devicePath
-		if len(pluginDevicePath) != 0 {
+		markDeviceMappedErr := actualStateOfWorld.MarkDeviceAsMounted(
-			devicePath = pluginDevicePath
+			volumeToMount.VolumeName, markedDevicePath, globalMapPath)
+		if markDeviceMappedErr != nil {
+			// On failure, return error. Caller will log and retry.
+			return volumeToMount.GenerateError("MapVolume.MarkDeviceAsMounted failed", markDeviceMappedErr)
 		}
-		if len(devicePath) == 0 {
+
-			return volumeToMount.GenerateError("MapVolume failed", goerrors.New("Device path of the volume is empty"))
+		// Call MapPodDevice if blockVolumeMapper implements CustomBlockVolumeMapper
+		if customBlockVolumeMapper, ok := blockVolumeMapper.(volume.CustomBlockVolumeMapper); ok {
+			// Execute driver specific map
+			pluginDevicePath, mapErr := customBlockVolumeMapper.MapPodDevice()
+			if mapErr != nil {
+				// On failure, return error. Caller will log and retry.
+				return volumeToMount.GenerateError("MapVolume.MapPodDevice failed", mapErr)
+			}
+
+			// if pluginDevicePath is provided, assume attacher may not provide device
+			// or attachment flow uses SetupDevice to get device path
+			if len(pluginDevicePath) != 0 {
+				devicePath = pluginDevicePath
+			}
+			if len(devicePath) == 0 {
+				return volumeToMount.GenerateError("MapVolume failed", goerrors.New("Device path of the volume is empty"))
+			}
 		}
 
 		// When kubelet is containerized, devicePath may be a symlink at a place unavailable to
@@ -1085,37 +1106,33 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
 			return volumeToMount.GenerateError("MapVolume.EvalHostSymlinks failed", err)
 		}
 
-		// Execute driver specific map
+		// Update actual state of world with the devicePath again, if devicePath has changed from markedDevicePath
-		volumeMapPath, volName := blockVolumeMapper.GetPodDeviceMapPath()
+		// TODO: This can be improved after #82492 is merged and ASW has state.
-		mapErr = blockVolumeMapper.MapDevice(devicePath, globalMapPath, volumeMapPath, volName, volumeToMount.Pod.UID)
+		if markedDevicePath != devicePath {
-		if mapErr != nil {
+			markDeviceMappedErr := actualStateOfWorld.MarkDeviceAsMounted(
-			// On failure, return error. Caller will log and retry.
+				volumeToMount.VolumeName, devicePath, globalMapPath)
-			return volumeToMount.GenerateError("MapVolume.MapDevice failed", mapErr)
+			if markDeviceMappedErr != nil {
+				// On failure, return error. Caller will log and retry.
+				return volumeToMount.GenerateError("MapVolume.MarkDeviceAsMounted failed", markDeviceMappedErr)
+			}
 		}
 
 		// Execute common map
-		mapErr = ioutil.MapBlockVolume(og.blkUtil, devicePath, globalMapPath, volumeMapPath, volName, volumeToMount.Pod.UID)
+		volumeMapPath, volName := blockVolumeMapper.GetPodDeviceMapPath()
+		mapErr := ioutil.MapBlockVolume(og.blkUtil, devicePath, globalMapPath, volumeMapPath, volName, volumeToMount.Pod.UID)
 		if mapErr != nil {
 			// On failure, return error. Caller will log and retry.
 			return volumeToMount.GenerateError("MapVolume.MapBlockVolume failed", mapErr)
 		}
 
-		// Update actual state of world to reflect volume is globally mounted
-		markDeviceMappedErr := actualStateOfWorld.MarkDeviceAsMounted(
-			volumeToMount.VolumeName, devicePath, globalMapPath)
-		if markDeviceMappedErr != nil {
-			// On failure, return error. Caller will log and retry.
-			return volumeToMount.GenerateError("MapVolume.MarkDeviceAsMounted failed", markDeviceMappedErr)
-		}
-
 		// Device mapping for global map path succeeded
-		simpleMsg, detailedMsg := volumeToMount.GenerateMsg("MapVolume.MapDevice succeeded", fmt.Sprintf("globalMapPath %q", globalMapPath))
+		simpleMsg, detailedMsg := volumeToMount.GenerateMsg("MapVolume.MapPodDevice succeeded", fmt.Sprintf("globalMapPath %q", globalMapPath))
 		verbosity := klog.Level(4)
 		og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeNormal, kevents.SuccessfulMountVolume, simpleMsg)
 		klog.V(verbosity).Infof(detailedMsg)
 
 		// Device mapping for pod device map path succeeded
-		simpleMsg, detailedMsg = volumeToMount.GenerateMsg("MapVolume.MapDevice succeeded", fmt.Sprintf("volumeMapPath %q", volumeMapPath))
+		simpleMsg, detailedMsg = volumeToMount.GenerateMsg("MapVolume.MapPodDevice succeeded", fmt.Sprintf("volumeMapPath %q", volumeMapPath))
 		verbosity = klog.Level(1)
 		og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeNormal, kevents.SuccessfulMountVolume, simpleMsg)
 		klog.V(verbosity).Infof(detailedMsg)
@@ -1218,6 +1235,16 @@ func (og *operationGenerator) GenerateUnmapVolumeFunc(
 			return volumeToUnmount.GenerateError("UnmapVolume.UnmapBlockVolume failed", unmapErr)
 		}
 
+		// Call UnmapPodDevice if blockVolumeUnmapper implements CustomBlockVolumeUnmapper
+		if customBlockVolumeUnmapper, ok := blockVolumeUnmapper.(volume.CustomBlockVolumeUnmapper); ok {
+			// Execute plugin specific unmap
+			unmapErr = customBlockVolumeUnmapper.UnmapPodDevice()
+			if unmapErr != nil {
+				// On failure, return error. Caller will log and retry.
+				return volumeToUnmount.GenerateError("UnmapVolume.UnmapPodDevice failed", unmapErr)
+			}
+		}
+
 		klog.Infof(
 			"UnmapVolume succeeded for volume %q (OuterVolumeSpecName: %q) pod %q (UID: %q). InnerVolumeSpecName %q. PluginName %q, VolumeGidValue %q",
 			volumeToUnmount.VolumeName,
@@ -1309,11 +1336,14 @@ func (og *operationGenerator) GenerateUnmapDeviceFunc(
 			return deviceToDetach.GenerateError("UnmapDevice failed", err)
 		}
 
-		// Execute tear down device
+		// Call TearDownDevice if blockVolumeUnmapper implements CustomBlockVolumeUnmapper
-		unmapErr := blockVolumeUnmapper.TearDownDevice(globalMapPath, deviceToDetach.DevicePath)
+		if customBlockVolumeUnmapper, ok := blockVolumeUnmapper.(volume.CustomBlockVolumeUnmapper); ok {
-		if unmapErr != nil {
+			// Execute tear down device
-			// On failure, return error. Caller will log and retry.
+			unmapErr := customBlockVolumeUnmapper.TearDownDevice(globalMapPath, deviceToDetach.DevicePath)
-			return deviceToDetach.GenerateError("UnmapDevice.TearDownDevice failed", unmapErr)
+			if unmapErr != nil {
+				// On failure, return error. Caller will log and retry.
+				return deviceToDetach.GenerateError("UnmapDevice.TearDownDevice failed", unmapErr)
+			}
 		}
 
 		// Plugin finished TearDownDevice(). Now globalMapPath dir and plugin's stored data

pkg/volume/util/volumepathhandler/volume_path_handler.go

@@ -252,7 +252,7 @@ func (v VolumePathHandler) IsSymlinkExist(mapPath string) (bool, error) {
 	if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
 		return true, nil
 	}
-	// If file exits but it's not symbolic link, return fale and no error
+	// If file exits but it's not symbolic link, return false and no error
 	return false, nil
 }
 
@@ -274,7 +274,7 @@ func (v VolumePathHandler) IsDeviceBindMountExist(mapPath string) (bool, error)
 	if fi.Mode()&os.ModeDevice == os.ModeDevice {
 		return true, nil
 	}
-	// If file exits but it's not device, return fale and no error
+	// If file exits but it's not device, return false and no error
 	return false, nil
 }
 

pkg/volume/volume.go

@@ -152,35 +152,43 @@ type Unmounter interface {
 	TearDownAt(dir string) error
 }
 
-// BlockVolumeMapper interface provides methods to set up/map the volume.
+// BlockVolumeMapper interface is a mapper interface for block volume.
 type BlockVolumeMapper interface {
 	BlockVolume
-	// SetUpDevice prepares the volume to a self-determined directory path,
-	// which may or may not exist yet and returns combination of physical
-	// device path of a block volume and error.
-	// If the plugin is non-attachable, it should prepare the device
-	// in /dev/ (or where appropriate) and return unique device path.
-	// Unique device path across kubelet node reboot is required to avoid
-	// unexpected block volume destruction.
-	// If the plugin is attachable, it should not do anything here,
-	// just return empty string for device path.
-	// Instead, attachable plugin have to return unique device path
-	// at attacher.Attach() and attacher.WaitForAttach().
-	// This may be called more than once, so implementations must be idempotent.
-	SetUpDevice() (string, error)
-
-	// Map maps the block device path for the specified spec and pod.
-	MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error
 }
 
-// BlockVolumeUnmapper interface provides methods to cleanup/unmap the volumes.
+// CustomBlockVolumeMapper interface provides custom methods to set up/map the volume.
+type CustomBlockVolumeMapper interface {
+	BlockVolumeMapper
+	// SetUpDevice prepares the volume to the node by the plugin specific way.
+	// For most in-tree plugins, attacher.Attach() and attacher.WaitForAttach()
+	// will do necessary works.
+	// This may be called more than once, so implementations must be idempotent.
+	SetUpDevice() error
+
+	// MapPodDevice maps the block device to a path and return the path.
+	// Unique device path across kubelet node reboot is required to avoid
+	// unexpected block volume destruction.
+	// If empty string is returned, the path retuned by attacher.Attach() and
+	// attacher.WaitForAttach() will be used.
+	MapPodDevice() (string, error)
+}
+
+// BlockVolumeUnmapper interface is an unmapper interface for block volume.
 type BlockVolumeUnmapper interface {
 	BlockVolume
-	// TearDownDevice removes traces of the SetUpDevice procedure under
+}
-	// a self-determined directory.
+
+// CustomBlockVolumeUnmapper interface provides custom methods to cleanup/unmap the volumes.
+type CustomBlockVolumeUnmapper interface {
+	BlockVolumeUnmapper
+	// TearDownDevice removes traces of the SetUpDevice procedure.
 	// If the plugin is non-attachable, this method detaches the volume
 	// from a node.
 	TearDownDevice(mapPath string, devicePath string) error
+
+	// UnmapPodDevice removes traces of the MapPodDevice procedure.
+	UnmapPodDevice() error
 }
 
 // Provisioner is an interface that creates templates for PersistentVolumes

pkg/volume/vsphere_volume/vsphere_volume_block.go

@@ -132,24 +132,12 @@ type vsphereBlockVolumeMapper struct {
 	*vsphereVolume
 }
 
-func (v vsphereBlockVolumeMapper) SetUpDevice() (string, error) {
-	return "", nil
-}
-
-func (v vsphereBlockVolumeMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error {
-	return nil
-}
-
 var _ volume.BlockVolumeUnmapper = &vsphereBlockVolumeUnmapper{}
 
 type vsphereBlockVolumeUnmapper struct {
 	*vsphereVolume
 }
 
-func (v *vsphereBlockVolumeUnmapper) TearDownDevice(mapPath, devicePath string) error {
-	return nil
-}
-
 // GetGlobalMapPath returns global map path and error
 // path: plugins/kubernetes.io/{PluginName}/volumeDevices/volumePath
 func (v *vsphereVolume) GetGlobalMapPath(spec *volume.Spec) (string, error) {