Skip to content

Commit 314c86f

Browse files
authored
Merge pull request #124 from Leaseweb/fix/nolock_publish
fix(locking): Remove locking at node publish/unpublish ops
2 parents b32f275 + 84615de commit 314c86f

File tree

1 file changed

+4
-10
lines changed

1 file changed

+4
-10
lines changed

pkg/driver/node.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,8 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
224224
deviceID = req.GetPublishContext()[deviceIDContextKey]
225225
}
226226

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

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

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

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

0 commit comments

Comments
 (0)