Skip to content

Commit c1b6b66

Browse files
rcmadhankumarrcmadhankumar
committed
Volume lock tests added for controller and node plugins
1 parent 5816832 commit c1b6b66

File tree

2 files changed

+210
-24
lines changed

2 files changed

+210
-24
lines changed

‎pkg/driver/controller_test.go

Lines changed: 152 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,35 @@ func TestCreateVolume(t *testing.T) {
461461
}
462462
},
463463
},
464+
{
465+
name: "fail locked volume request",
466+
testFunc: func(t *testing.T) {
467+
req := &csi.CreateVolumeRequest{
468+
Name: "random-vol-name",
469+
CapacityRange: stdCapRange,
470+
VolumeCapabilities: stdVolCap,
471+
Parameters: nil,
472+
}
473+
474+
ctx := context.Background()
475+
mockCtl := gomock.NewController(t)
476+
defer mockCtl.Finish()
477+
478+
mockCloud := mocks.NewMockCloud(mockCtl)
479+
480+
powervsDriver := controllerService{
481+
cloud: mockCloud,
482+
driverOptions: &Options{},
483+
volumeLocks: util.NewVolumeLocks(),
484+
}
485+
486+
powervsDriver.volumeLocks.TryAcquire(req.Name)
487+
defer powervsDriver.volumeLocks.Release(req.Name)
488+
489+
_, err := powervsDriver.CreateVolume(ctx, req)
490+
checkExpectedErrorCode(t, err, codes.Aborted)
491+
},
492+
},
464493
}
465494

466495
for _, tc := range testCases {
@@ -575,6 +604,32 @@ func TestDeleteVolume(t *testing.T) {
575604
}
576605
},
577606
},
607+
{
608+
name: "fail if volume is already locked",
609+
testFunc: func(t *testing.T) {
610+
req := &csi.DeleteVolumeRequest{
611+
VolumeId: "vol-test",
612+
}
613+
614+
ctx := context.Background()
615+
mockCtl := gomock.NewController(t)
616+
defer mockCtl.Finish()
617+
618+
mockCloud := mocks.NewMockCloud(mockCtl)
619+
620+
powervsDriver := controllerService{
621+
cloud: mockCloud,
622+
driverOptions: &Options{},
623+
volumeLocks: util.NewVolumeLocks(),
624+
}
625+
626+
powervsDriver.volumeLocks.TryAcquire(req.VolumeId)
627+
defer powervsDriver.volumeLocks.Release(req.VolumeId)
628+
629+
_, err := powervsDriver.DeleteVolume(ctx, req)
630+
checkExpectedErrorCode(t, err, codes.Aborted)
631+
},
632+
},
578633
}
579634

580635
for _, tc := range testCases {
@@ -855,6 +910,34 @@ func TestControllerPublishVolume(t *testing.T) {
855910
}
856911
},
857912
},
913+
{
914+
name: "fail if volume already locked",
915+
testFunc: func(t *testing.T) {
916+
req := &csi.ControllerPublishVolumeRequest{
917+
NodeId: expInstanceID,
918+
VolumeCapability: stdVolCap,
919+
VolumeId: volumeName,
920+
}
921+
922+
ctx := context.Background()
923+
mockCtl := gomock.NewController(t)
924+
defer mockCtl.Finish()
925+
926+
mockCloud := mocks.NewMockCloud(mockCtl)
927+
powervsDriver := controllerService{
928+
cloud: mockCloud,
929+
driverOptions: &Options{},
930+
volumeLocks: util.NewVolumeLocks(),
931+
}
932+
933+
powervsDriver.volumeLocks.TryAcquire(req.VolumeId)
934+
defer powervsDriver.volumeLocks.Release(req.VolumeId)
935+
936+
_, err := powervsDriver.ControllerPublishVolume(ctx, req)
937+
checkExpectedErrorCode(t, err, codes.Aborted)
938+
939+
},
940+
},
858941
}
859942

860943
for _, tc := range testCases {
@@ -1000,6 +1083,32 @@ func TestControllerUnpublishVolume(t *testing.T) {
10001083
}
10011084
},
10021085
},
1086+
{
1087+
name: "fail if volume already locked",
1088+
testFunc: func(t *testing.T) {
1089+
req := &csi.ControllerUnpublishVolumeRequest{
1090+
NodeId: expInstanceID,
1091+
VolumeId: "vol-test",
1092+
}
1093+
1094+
ctx := context.Background()
1095+
mockCtl := gomock.NewController(t)
1096+
defer mockCtl.Finish()
1097+
1098+
mockCloud := mocks.NewMockCloud(mockCtl)
1099+
powervsDriver := controllerService{
1100+
cloud: mockCloud,
1101+
driverOptions: &Options{},
1102+
volumeLocks: util.NewVolumeLocks(),
1103+
}
1104+
1105+
powervsDriver.volumeLocks.TryAcquire(req.VolumeId)
1106+
defer powervsDriver.volumeLocks.Release(req.VolumeId)
1107+
1108+
_, err := powervsDriver.ControllerUnpublishVolume(ctx, req)
1109+
checkExpectedErrorCode(t, err, codes.Aborted)
1110+
},
1111+
},
10031112
}
10041113

10051114
for _, tc := range testCases {
@@ -1009,11 +1118,12 @@ func TestControllerUnpublishVolume(t *testing.T) {
10091118

10101119
func TestControllerExpandVolume(t *testing.T) {
10111120
testCases := []struct {
1012-
name string
1013-
req *csi.ControllerExpandVolumeRequest
1014-
newSize int64
1015-
expResp *csi.ControllerExpandVolumeResponse
1016-
expError bool
1121+
name string
1122+
req *csi.ControllerExpandVolumeRequest
1123+
newSize int64
1124+
expResp *csi.ControllerExpandVolumeResponse
1125+
expError bool
1126+
volumeLock bool
10171127
}{
10181128
{
10191129
name: "success normal",
@@ -1043,6 +1153,16 @@ func TestControllerExpandVolume(t *testing.T) {
10431153
},
10441154
expError: true,
10451155
},
1156+
{
1157+
name: "fail if volume already locked",
1158+
req: &csi.ControllerExpandVolumeRequest{
1159+
VolumeId: "vol-test",
1160+
CapacityRange: &csi.CapacityRange{
1161+
RequiredBytes: 5 * util.GiB,
1162+
},
1163+
},
1164+
volumeLock: true,
1165+
},
10461166
}
10471167

10481168
for _, tc := range testCases {
@@ -1059,34 +1179,43 @@ func TestControllerExpandVolume(t *testing.T) {
10591179
}
10601180

10611181
mockCloud := mocks.NewMockCloud(mockCtl)
1062-
mockCloud.EXPECT().ResizeDisk(gomock.Eq(tc.req.VolumeId), gomock.Any()).Return(retSizeGiB, nil).AnyTimes()
1063-
10641182
powervsDriver := controllerService{
10651183
cloud: mockCloud,
10661184
driverOptions: &Options{},
10671185
volumeLocks: util.NewVolumeLocks(),
10681186
}
10691187

1070-
resp, err := powervsDriver.ControllerExpandVolume(ctx, tc.req)
1071-
if err != nil {
1072-
srvErr, ok := status.FromError(err)
1073-
if !ok {
1074-
t.Fatalf("Could not get error status code from error: %v", srvErr)
1075-
}
1076-
if !tc.expError {
1077-
t.Fatalf("Unexpected error: %v", err)
1078-
}
1188+
if tc.volumeLock {
1189+
powervsDriver.volumeLocks.TryAcquire(tc.req.VolumeId)
1190+
defer powervsDriver.volumeLocks.Release(tc.req.VolumeId)
1191+
1192+
_, err := powervsDriver.ControllerExpandVolume(ctx, tc.req)
1193+
checkExpectedErrorCode(t, err, codes.Aborted)
1194+
10791195
} else {
1080-
if tc.expError {
1081-
t.Fatalf("Expected error from ControllerExpandVolume, got nothing")
1196+
mockCloud.EXPECT().ResizeDisk(gomock.Eq(tc.req.VolumeId), gomock.Any()).Return(retSizeGiB, nil).AnyTimes()
1197+
resp, err := powervsDriver.ControllerExpandVolume(ctx, tc.req)
1198+
if err != nil {
1199+
srvErr, ok := status.FromError(err)
1200+
if !ok {
1201+
t.Fatalf("Could not get error status code from error: %v", srvErr)
1202+
}
1203+
if !tc.expError {
1204+
t.Fatalf("Unexpected error: %v", err)
1205+
}
1206+
} else {
1207+
if tc.expError {
1208+
t.Fatalf("Expected error from ControllerExpandVolume, got nothing")
1209+
}
10821210
}
1083-
}
10841211

1085-
sizeGiB := util.BytesToGiB(resp.GetCapacityBytes())
1086-
expSizeGiB := util.BytesToGiB(tc.expResp.GetCapacityBytes())
1087-
if sizeGiB != expSizeGiB {
1088-
t.Fatalf("Expected size %d GiB, got %d GiB", expSizeGiB, sizeGiB)
1212+
sizeGiB := util.BytesToGiB(resp.GetCapacityBytes())
1213+
expSizeGiB := util.BytesToGiB(tc.expResp.GetCapacityBytes())
1214+
if sizeGiB != expSizeGiB {
1215+
t.Fatalf("Expected size %d GiB, got %d GiB", expSizeGiB, sizeGiB)
1216+
}
10891217
}
1218+
10901219
})
10911220
}
10921221
}

‎pkg/driver/node_test.go

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ func TestNodeStageVolume(t *testing.T) {
6666
request *csi.NodeStageVolumeRequest
6767
expectMock func(mockMounter mocks.MockMounter)
6868
expectedCode codes.Code
69+
volumeLock bool
6970
}{
7071

7172
{
@@ -256,6 +257,17 @@ func TestNodeStageVolume(t *testing.T) {
256257
},
257258
expectedCode: codes.InvalidArgument,
258259
},
260+
{
261+
name: "fail_if_volume_already_locked",
262+
request: &csi.NodeStageVolumeRequest{
263+
PublishContext: map[string]string{WWNKey: devicePath},
264+
StagingTargetPath: targetPath,
265+
VolumeCapability: stdVolCap,
266+
VolumeId: volumeID,
267+
},
268+
volumeLock: true,
269+
expectedCode: codes.Aborted,
270+
},
259271
}
260272

261273
for _, tc := range testCases {
@@ -270,6 +282,11 @@ func TestNodeStageVolume(t *testing.T) {
270282
volumeLocks: util.NewVolumeLocks(),
271283
}
272284

285+
if tc.volumeLock {
286+
powervsDriver.volumeLocks.TryAcquire(tc.request.VolumeId)
287+
defer powervsDriver.volumeLocks.Release(tc.request.VolumeId)
288+
}
289+
273290
if tc.expectMock != nil {
274291
tc.expectMock(*mockMounter)
275292
}
@@ -291,27 +308,42 @@ func TestNodeExpandVolume(t *testing.T) {
291308
mockMounter := mocks.NewMockMounter(mockCtl)
292309

293310
powervsDriver := &nodeService{
294-
mounter: mockMounter,
311+
mounter: mockMounter,
312+
volumeLocks: util.NewVolumeLocks(),
295313
}
296314

297315
tests := []struct {
298316
name string
299317
request csi.NodeExpandVolumeRequest
300318
expectResponseCode codes.Code
301319
expectMock func(mockMounter mocks.MockMounter)
320+
volumeLock bool
302321
}{
303322
{
304323
name: "fail missing volumeId",
305324
request: csi.NodeExpandVolumeRequest{},
306325
expectResponseCode: codes.InvalidArgument,
307326
},
327+
{
328+
name: "fail if volume already locked",
329+
request: csi.NodeExpandVolumeRequest{
330+
VolumeId: "test-volume-id",
331+
},
332+
expectResponseCode: codes.Aborted,
333+
volumeLock: true,
334+
},
308335
}
309336

310337
for _, test := range tests {
311338
t.Run(test.name, func(t *testing.T) {
312339
if test.expectMock != nil {
313340
test.expectMock(*mockMounter)
314341
}
342+
343+
if test.volumeLock {
344+
powervsDriver.volumeLocks.TryAcquire(test.request.VolumeId)
345+
defer powervsDriver.volumeLocks.Release(test.request.VolumeId)
346+
}
315347
_, err := powervsDriver.NodeExpandVolume(context.Background(), &test.request)
316348
if err != nil {
317349
if test.expectResponseCode != codes.OK {
@@ -421,6 +453,31 @@ func TestNodeUnpublishVolume(t *testing.T) {
421453
expectErr(t, err, codes.Internal)
422454
},
423455
},
456+
{
457+
name: "fail if volume already locked",
458+
testFunc: func(t *testing.T) {
459+
mockCtl := gomock.NewController(t)
460+
defer mockCtl.Finish()
461+
462+
mockMounter := mocks.NewMockMounter(mockCtl)
463+
464+
powervsDriver := &nodeService{
465+
mounter: mockMounter,
466+
volumeLocks: util.NewVolumeLocks(),
467+
}
468+
469+
req := &csi.NodeUnpublishVolumeRequest{
470+
TargetPath: targetPath,
471+
VolumeId: "vol-test",
472+
}
473+
474+
powervsDriver.volumeLocks.TryAcquire(req.TargetPath)
475+
defer powervsDriver.volumeLocks.Release(req.TargetPath)
476+
477+
_, err := powervsDriver.NodeUnpublishVolume(context.TODO(), req)
478+
checkExpectedErrorCode(t, err, codes.Aborted)
479+
},
480+
},
424481
}
425482

426483
for _, tc := range testCases {

0 commit comments

Comments
 (0)