Skip to content

Commit

Permalink
Merge pull request #124 from Leaseweb/fix/nolock_publish
Browse files Browse the repository at this point in the history
fix(locking): Remove locking at node publish/unpublish ops
  • Loading branch information
hrak authored Jun 18, 2024
2 parents b32f275 + 84615de commit 314c86f
Showing 1 changed file with 4 additions and 10 deletions.
14 changes: 4 additions & 10 deletions pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,8 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
deviceID = req.GetPublishContext()[deviceIDContextKey]
}

if acquired := ns.volumeLocks.TryAcquire(volumeID); !acquired {
ctxzap.Extract(ctx).Sugar().Errorf(util.VolumeOperationAlreadyExistsFmt, volumeID)
return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID)
}
defer ns.volumeLocks.Release(volumeID)
// Considering kubelet ensures the stage and publish operations
// are serialized, we don't need any extra locking in NodePublishVolume.

if req.GetVolumeCapability().GetMount() != nil {
source := req.GetStagingTargetPath()
Expand Down Expand Up @@ -323,11 +320,8 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu
return nil, status.Error(codes.InvalidArgument, "Target path missing in request")
}

if acquired := ns.volumeLocks.TryAcquire(volumeID); !acquired {
ctxzap.Extract(ctx).Sugar().Errorf(util.VolumeOperationAlreadyExistsFmt, volumeID)
return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID)
}
defer ns.volumeLocks.Release(volumeID)
// Considering that kubelet ensures the stage and publish operations
// are serialized, we don't need any extra locking in NodeUnpublishVolume.

if _, err := ns.connector.GetVolumeByID(ctx, volumeID); err == cloud.ErrNotFound {
return nil, status.Errorf(codes.NotFound, "Volume %v not found", volumeID)
Expand Down

0 comments on commit 314c86f

Please sign in to comment.