From 44d7010839091b0b63ec6281a0c954693c1554c8 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Wed, 27 Dec 2023 04:10:03 +0000 Subject: [PATCH] fix: reduce mount lock to avoid volumeID collision issue --- pkg/azurefile/nodeserver.go | 10 ++++++---- pkg/azurefile/nodeserver_test.go | 8 ++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/azurefile/nodeserver.go b/pkg/azurefile/nodeserver.go index 28d4618b96..74a8aa8838 100644 --- a/pkg/azurefile/nodeserver.go +++ b/pkg/azurefile/nodeserver.go @@ -240,10 +240,11 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe return nil, status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(%s) is not supported, supported fsGroupChangePolicy list: %v", fsGroupChangePolicy, supportedFSGroupChangePolicyList) } - if acquired := d.volumeLocks.TryAcquire(volumeID); !acquired { + lockKey := fmt.Sprintf("%s-%s", volumeID, targetPath) + if acquired := d.volumeLocks.TryAcquire(lockKey); !acquired { return nil, status.Errorf(codes.Aborted, volumeOperationAlreadyExistsFmt, volumeID) } - defer d.volumeLocks.Release(volumeID) + defer d.volumeLocks.Release(lockKey) if strings.TrimSpace(storageEndpointSuffix) == "" { if d.cloud.Environment.StorageEndpointSuffix != "" { @@ -393,10 +394,11 @@ func (d *Driver) NodeUnstageVolume(_ context.Context, req *csi.NodeUnstageVolume return nil, status.Error(codes.InvalidArgument, "Staging target not provided") } - if acquired := d.volumeLocks.TryAcquire(volumeID); !acquired { + lockKey := fmt.Sprintf("%s-%s", volumeID, stagingTargetPath) + if acquired := d.volumeLocks.TryAcquire(lockKey); !acquired { return nil, status.Errorf(codes.Aborted, volumeOperationAlreadyExistsFmt, volumeID) } - defer d.volumeLocks.Release(volumeID) + defer d.volumeLocks.Release(lockKey) mc := metrics.NewMetricContext(azureFileCSIDriverName, "node_unstage_volume", d.cloud.ResourceGroup, "", d.Name) isOperationSucceeded := false diff --git a/pkg/azurefile/nodeserver_test.go b/pkg/azurefile/nodeserver_test.go index 94147a4531..daa93bc9e5 100644 --- a/pkg/azurefile/nodeserver_test.go +++ b/pkg/azurefile/nodeserver_test.go @@ -525,7 +525,7 @@ func TestNodeStageVolume(t *testing.T) { { desc: "[Error] Volume operation in progress", setup: func() { - d.volumeLocks.TryAcquire("vol_1##") + d.volumeLocks.TryAcquire(fmt.Sprintf("%s-%s", "vol_1##", sourceTest)) }, req: csi.NodeStageVolumeRequest{VolumeId: "vol_1##", StagingTargetPath: sourceTest, VolumeCapability: &stdVolCap, @@ -535,7 +535,7 @@ func TestNodeStageVolume(t *testing.T) { DefaultError: status.Error(codes.Aborted, fmt.Sprintf(volumeOperationAlreadyExistsFmt, "vol_1##")), }, cleanup: func() { - d.volumeLocks.Release("vol_1##") + d.volumeLocks.Release(fmt.Sprintf("%s-%s", "vol_1##", sourceTest)) }, }, { @@ -784,14 +784,14 @@ func TestNodeUnstageVolume(t *testing.T) { { desc: "[Error] Volume operation in progress", setup: func() { - d.volumeLocks.TryAcquire("vol_1") + d.volumeLocks.TryAcquire(fmt.Sprintf("%s-%s", "vol_1", targetFile)) }, req: csi.NodeUnstageVolumeRequest{StagingTargetPath: targetFile, VolumeId: "vol_1"}, expectedErr: testutil.TestError{ DefaultError: status.Error(codes.Aborted, fmt.Sprintf(volumeOperationAlreadyExistsFmt, "vol_1")), }, cleanup: func() { - d.volumeLocks.Release("vol_1") + d.volumeLocks.Release(fmt.Sprintf("%s-%s", "vol_1", targetFile)) }, }, {