Skip to content

Commit

Permalink
rbd: call undoStagingTransaction() when NodeStageVolume() fails
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
nixpanic authored and mergify-bot committed Nov 17, 2021
1 parent fac3ef0 commit 547b673
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions internal/rbd/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

// perform the actual staging and if this fails, have undoStagingTransaction
// cleans up for us
txn, err := ns.stageTransaction(ctx, req, cr, rv, isStaticVol)
defer func() {
if err != nil {
ns.undoStagingTransaction(ctx, req, transaction, rv)
ns.undoStagingTransaction(ctx, req, txn, rv)
}
}()

// perform the actual staging and if this fails, have undoStagingTransaction
// cleans up for us
transaction, err = ns.stageTransaction(ctx, req, cr, rv, isStaticVol)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 547b673

Please sign in to comment.