Adjust defer to correctly call

openshift · Feb 9, 2021 · d691bcf · d691bcf
1 parent f0a40f4
commit d691bcf
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 15 deletions.
diff --git a/pkg/volume/csi/csi_attacher.go b/pkg/volume/csi/csi_attacher.go
@@ -296,11 +296,8 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo
 		volDataKey.volHandle:  csiSource.VolumeHandle,
 		volDataKey.driverName: csiSource.Driver,
 	}
-	if err = saveVolumeData(dataDir, volDataFileName, data); err != nil {
-		errMsg := log("failed to save volume info data: %v", err)
-		klog.Error(errMsg)
-		return errors.New(errMsg)
-	}
+
+	err = saveVolumeData(dataDir, volDataFileName, data)
 	defer func() {
 		// Only if there was an error and volume operation was considered
 		// finished, we should remove the directory.
@@ -313,6 +310,12 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo
 		}
 	}()
 
+	if err != nil {
+		errMsg := log("failed to save volume info data: %v", err)
+		klog.Error(errMsg)
+		return errors.New(errMsg)
+	}
+
 	if !stageUnstageSet {
 		klog.Infof(log("attacher.MountDevice STAGE_UNSTAGE_VOLUME capability not set. Skipping MountDevice..."))
 		// defer does *not* remove the metadata file and it's correct - UnmountDevice needs it there.

diff --git a/pkg/volume/csi/csi_plugin.go b/pkg/volume/csi/csi_plugin.go
@@ -43,6 +43,7 @@ import (
 	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/csi/nodeinfomanager"
+	volumetypes "k8s.io/kubernetes/pkg/volume/util/types"
 )
 
 const (
@@ -457,15 +458,22 @@ func (p *csiPlugin) NewMounter(
 	attachID := getAttachmentName(volumeHandle, driverName, node)
 	volData[volDataKey.attachmentID] = attachID
 
-	if err := saveVolumeData(dataDir, volDataFileName, volData); err != nil {
+	err = saveVolumeData(dataDir, volDataFileName, volData)
+	defer func() {
+		// Only if there was an error and volume operation was considered
+		// finished, we should remove the directory.
+		if err != nil && volumetypes.IsOperationFinishedError(err) {
+			// attempt to cleanup volume mount dir.
+			if err = removeMountDir(p, dir); err != nil {
+				klog.Error(log("attacher.MountDevice failed to remove mount dir after error [%s]: %v", dir, err))
+			}
+		}
+	}()
+
+	if err != nil {
 		errorMsg := log("csi.NewMounter failed to save volume info data: %v", err)
 		klog.Error(errorMsg)
 
-		// attempt to cleanup volume mount dir.
-		if removeMountDirErr := removeMountDir(p, dir); removeMountDirErr != nil {
-			klog.Error(log("csi.NewMounter failed to remove mount dir [%s]: %v", dir, removeMountDirErr))
-		}
-
 		return nil, errors.New(errorMsg)
 	}
 
@@ -707,11 +715,21 @@ func (p *csiPlugin) NewBlockVolumeMapper(spec *volume.Spec, podRef *api.Pod, opt
 		volDataKey.attachmentID: attachID,
 	}
 
-	if err := saveVolumeData(dataDir, volDataFileName, volData); err != nil {
-		if removeErr := os.RemoveAll(dataDir); removeErr != nil {
-			klog.Error(log("failed to remove dir after error [%s]: %v", dataDir, removeErr))
+	err = saveVolumeData(dataDir, volDataFileName, volData)
+	defer func() {
+		// Only if there was an error and volume operation was considered
+		// finished, we should remove the directory.
+		if err != nil && volumetypes.IsOperationFinishedError(err) {
+			// attempt to cleanup volume mount dir.
+			if err = removeMountDir(p, dataDir); err != nil {
+				klog.Error(log("attacher.MountDevice failed to remove mount dir after error [%s]: %v", dataDir, err))
+			}
 		}
-		return nil, errors.New(log("failed to save volume info data: %v", err))
+	}()
+	if err != nil {
+		errorMsg := log("csi.NewBlockVolumeMapper failed to save volume info data: %v", err)
+		klog.Error(errorMsg)
+		return nil, errors.New(errorMsg)
 	}
 
 	return mapper, nil