From 86a5bd98b644b4b79f9efa1da406ab505ca51a7f Mon Sep 17 00:00:00 2001
From: Jan Safranek <jsafrane@redhat.com>
Date: Mon, 2 Mar 2020 12:54:02 +0100
Subject: [PATCH] Add uncertain map state to block volumes

Volume mount should be marked as uncertain after NodeStage / NodePublish
timeout or similar error, when the driver can continue with the operation in
background.
---
 pkg/volume/csi/csi_block.go                   |  4 +--
 .../operationexecutor/operation_generator.go  | 35 ++++++++++++-------
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/pkg/volume/csi/csi_block.go b/pkg/volume/csi/csi_block.go
index ee2ae319ee4..ceacdc4a140 100644
--- a/pkg/volume/csi/csi_block.go
+++ b/pkg/volume/csi/csi_block.go
@@ -189,7 +189,7 @@ func (m *csiBlockMapper) stageVolumeForBlock(
 		nil /* MountOptions */)
 
 	if err != nil {
-		return "", errors.New(log("blockMapper.stageVolumeForBlock failed: %v", err))
+		return "", err
 	}
 
 	klog.V(4).Infof(log("blockMapper.stageVolumeForBlock successfully requested NodeStageVolume [%s]", stagingPath))
@@ -249,7 +249,7 @@ func (m *csiBlockMapper) publishVolumeForBlock(
 	)
 
 	if err != nil {
-		return "", errors.New(log("blockMapper.publishVolumeForBlock failed: %v", err))
+		return "", err
 	}
 
 	return publishPath, nil
diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go
index 5aeb095226b..9f86cf93f8e 100644
--- a/pkg/volume/util/operationexecutor/operation_generator.go
+++ b/pkg/volume/util/operationexecutor/operation_generator.go
@@ -929,7 +929,7 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
 		volumeAttacher, _ = attachableVolumePlugin.NewAttacher()
 	}
 
-	mapVolumeFunc := func() (error, error) {
+	mapVolumeFunc := func() (simpleErr error, detailedErr error) {
 		var devicePath string
 		// Set up global map path under the given plugin directory using symbolic link
 		globalMapPath, err :=
@@ -956,6 +956,7 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
 		if customBlockVolumeMapper, ok := blockVolumeMapper.(volume.CustomBlockVolumeMapper); ok {
 			mapErr := customBlockVolumeMapper.SetUpDevice()
 			if mapErr != nil {
+				og.markDeviceErrorState(volumeToMount, devicePath, globalMapPath, mapErr, actualStateOfWorld)
 				// On failure, return error. Caller will log and retry.
 				return volumeToMount.GenerateError("MapVolume.SetUpDevice failed", mapErr)
 			}
@@ -970,15 +971,36 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
 			return volumeToMount.GenerateError("MapVolume.MarkDeviceAsMounted failed", markDeviceMappedErr)
 		}
 
+		markVolumeOpts := MarkVolumeOpts{
+			PodName:             volumeToMount.PodName,
+			PodUID:              volumeToMount.Pod.UID,
+			VolumeName:          volumeToMount.VolumeName,
+			BlockVolumeMapper:   blockVolumeMapper,
+			OuterVolumeSpecName: volumeToMount.OuterVolumeSpecName,
+			VolumeGidVolume:     volumeToMount.VolumeGidValue,
+			VolumeSpec:          volumeToMount.VolumeSpec,
+			VolumeMountState:    VolumeMounted,
+		}
+
 		// 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.
+				og.markVolumeErrorState(volumeToMount, markVolumeOpts, mapErr, actualStateOfWorld)
 				return volumeToMount.GenerateError("MapVolume.MapPodDevice failed", mapErr)
 			}
 
+			// From now on, the volume is mapped. Mark it as uncertain on error,
+			// so it is is unmapped when corresponding pod is deleted.
+			defer func() {
+				if simpleErr != nil {
+					errText := simpleErr.Error()
+					og.markVolumeErrorState(volumeToMount, markVolumeOpts, volumetypes.NewUncertainProgressError(errText), actualStateOfWorld)
+				}
+			}()
+
 			// if pluginDevicePath is provided, assume attacher may not provide device
 			// or attachment flow uses SetupDevice to get device path
 			if len(pluginDevicePath) != 0 {
@@ -1044,17 +1066,6 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
 			return volumeToMount.GenerateError("MapVolume.MarkVolumeAsMounted failed while expanding volume", resizeError)
 		}
 
-		markVolumeOpts := MarkVolumeOpts{
-			PodName:             volumeToMount.PodName,
-			PodUID:              volumeToMount.Pod.UID,
-			VolumeName:          volumeToMount.VolumeName,
-			BlockVolumeMapper:   blockVolumeMapper,
-			OuterVolumeSpecName: volumeToMount.OuterVolumeSpecName,
-			VolumeGidVolume:     volumeToMount.VolumeGidValue,
-			VolumeSpec:          volumeToMount.VolumeSpec,
-			VolumeMountState:    VolumeMounted,
-		}
-
 		markVolMountedErr := actualStateOfWorld.MarkVolumeAsMounted(markVolumeOpts)
 		if markVolMountedErr != nil {
 			// On failure, return error. Caller will log and retry.