rbd: call undoStagingTransaction() when NodeStageVolume() fails

On line 341 a `transaction` is created. This is passed to the deferred
`undoStagingTransaction()` function when an error in the
`NodeStageVolume` procedure is detected. So far, so good.

However, on line 356 a new `transaction` is returned. This new
`transaction` is not used for the defer call.

By removing the empty `transaction` that is used in the defer call, and
calling `undoStagingTransaction()` on an error of `stageTransaction()`,
the code is a little simpler, and the cleanup of the transaction should
be done correctly now.

Fixes: ceph#2610
Signed-off-by: Niels de Vos <ndevos@redhat.com>

internal/rbd/nodeserver.go

@@ -338,23 +338,18 @@ func (ns *NodeServer) NodeStageVolume(
 		return &csi.NodeStageVolumeResponse{}, nil
 	}
 
-	transaction := stageTransaction{}
 	// Stash image details prior to mapping the image (useful during Unstage as it has no
 	// voloptions passed to the RPC as per the CSI spec)
 	err = stashRBDImageMetadata(rv, stagingParentPath)
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
-	defer func() {
-		if err != nil {
-			ns.undoStagingTransaction(ctx, req, transaction, rv)
-		}
-	}()
 
 	// perform the actual staging and if this fails, have undoStagingTransaction
 	// cleans up for us
-	transaction, err = ns.stageTransaction(ctx, req, cr, rv, isStaticVol)
+	transaction, err := ns.stageTransaction(ctx, req, cr, rv, isStaticVol)
 	if err != nil {
+		ns.undoStagingTransaction(ctx, req, transaction, rv)
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
@@ -372,7 +367,7 @@ func (ns *NodeServer) stageTransaction(
 	req *csi.NodeStageVolumeRequest,
 	cr *util.Credentials,
 	volOptions *rbdVolume,
-	staticVol bool) (stageTransaction, error) {
+	staticVol bool) (*stageTransaction, error) {
 	transaction := stageTransaction{}
 
 	var err error
@@ -387,14 +382,14 @@ func (ns *NodeServer) stageTransaction(
 
 	err = flattenImageBeforeMapping(ctx, volOptions, cr)
 	if err != nil {
-		return transaction, err
+		return &transaction, err
 	}
 
 	// Mapping RBD image
 	var devicePath string
 	devicePath, err = attachRBDImage(ctx, volOptions, devicePath, cr)
 	if err != nil {
-		return transaction, err
+		return &transaction, err
 	}
 	transaction.devicePath = devicePath
 
@@ -408,14 +403,14 @@ func (ns *NodeServer) stageTransaction(
 	if volOptions.Mounter == rbdNbdMounter {
 		err = updateRBDImageMetadataStash(req.GetStagingTargetPath(), devicePath)
 		if err != nil {
-			return transaction, err
+			return &transaction, err
 		}
 	}
 
 	if volOptions.isEncrypted() {
 		devicePath, err = ns.processEncryptedDevice(ctx, volOptions, devicePath)
 		if err != nil {
-			return transaction, err
+			return &transaction, err
 		}
 		transaction.isEncrypted = true
 	}
@@ -425,15 +420,15 @@ func (ns *NodeServer) stageTransaction(
 	isBlock := req.GetVolumeCapability().GetBlock() != nil
 	err = ns.createStageMountPoint(ctx, stagingTargetPath, isBlock)
 	if err != nil {
-		return transaction, err
+		return &transaction, err
 	}
 
 	transaction.isStagePathCreated = true
 
 	// nodeStage Path
 	readOnly, err = ns.mountVolumeToStagePath(ctx, req, staticVol, stagingTargetPath, devicePath)
 	if err != nil {
-		return transaction, err
+		return &transaction, err
 	}
 	transaction.isMounted = true
 
@@ -443,7 +438,7 @@ func (ns *NodeServer) stageTransaction(
 		resizer := mount.NewResizeFs(utilexec.New())
 		ok, err = resizer.NeedResize(devicePath, stagingTargetPath)
 		if err != nil {
-			return transaction, status.Errorf(codes.Internal,
+			return &transaction, status.Errorf(codes.Internal,
 				"Need resize check failed on devicePath %s and staingPath %s, error: %v",
 				devicePath,
 				stagingTargetPath,
@@ -452,7 +447,7 @@ func (ns *NodeServer) stageTransaction(
 		if ok {
 			ok, err = resizer.Resize(devicePath, stagingTargetPath)
 			if !ok {
-				return transaction, status.Errorf(codes.Internal,
+				return &transaction, status.Errorf(codes.Internal,
 					"resize failed on path %s, error: %v", stagingTargetPath, err)
 			}
 		}
@@ -462,7 +457,7 @@ func (ns *NodeServer) stageTransaction(
 		err = os.Chmod(stagingTargetPath, 0o777)
 	}
 
-	return transaction, err
+	return &transaction, err
 }
 
 func flattenImageBeforeMapping(
@@ -503,7 +498,7 @@ func flattenImageBeforeMapping(
 func (ns *NodeServer) undoStagingTransaction(
 	ctx context.Context,
 	req *csi.NodeStageVolumeRequest,
-	transaction stageTransaction,
+	transaction *stageTransaction,
 	volOptions *rbdVolume) {
 	var err error