From 7e221801253c7ec35f22943faf5e7711a080e43d Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos@redhat.com>
Date: Wed, 3 Nov 2021 14:18:20 +0100
Subject: [PATCH] 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.

Updates: #2610
Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 internal/rbd/nodeserver.go | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go
index 1b5214a53..4a5593cf8 100644
--- a/internal/rbd/nodeserver.go
+++ b/internal/rbd/nodeserver.go
@@ -335,22 +335,21 @@ 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)
+	txn, err := ns.stageTransaction(ctx, req, cr, rv, isStaticVol)
+	defer func() {
+		if err != nil {
+			ns.undoStagingTransaction(ctx, req, txn, rv)
+		}
+	}()
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
@@ -369,8 +368,8 @@ func (ns *NodeServer) stageTransaction(
 	req *csi.NodeStageVolumeRequest,
 	cr *util.Credentials,
 	volOptions *rbdVolume,
-	staticVol bool) (stageTransaction, error) {
-	transaction := stageTransaction{}
+	staticVol bool) (*stageTransaction, error) {
+	transaction := &stageTransaction{}
 
 	var err error
 	var readOnly bool
@@ -500,7 +499,7 @@ func flattenImageBeforeMapping(
 func (ns *NodeServer) undoStagingTransaction(
 	ctx context.Context,
 	req *csi.NodeStageVolumeRequest,
-	transaction stageTransaction,
+	transaction *stageTransaction,
 	volOptions *rbdVolume) {
 	var err error