Skip to content

Commit a2ba7ab

Browse files
authored
Merge pull request #198 from Madhan-SWE/vol-type-fix
Added default value for expected volume type for volume verification
2 parents 68f20a6 + e6841c9 commit a2ba7ab

File tree

3 files changed

+79
-9
lines changed

3 files changed

+79
-9
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ There are several optional parameters that could be passed into ``` CreateVolume
2222

2323
| **Parameters** | **Values** | **Default** | **Description**|
2424
| ----------------------------- | ----------------------------- | ----------- | ----------------------------- |
25+
| "type" | tier1, tier3 | tier1 | PowerVS Disk type that will be created during volume creation |
2526
| "csi.storage.k8s.io/fstype" | xfs, ext2, ext3, ext4 | ext4 | File system type that will be formatted during volume creation. This parameter is case sensitive! |
2627

2728

pkg/driver/controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
122122
return nil, status.Error(codes.InvalidArgument, errString)
123123
}
124124

125-
var volumeType string
125+
volumeType := cloud.DefaultVolumeType
126126

127127
for key, value := range req.GetParameters() {
128128
switch strings.ToLower(key) {

pkg/driver/controller_test.go

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ func TestCreateVolume(t *testing.T) {
4646

4747
stdVolSize := int64(5 * 1024 * 1024 * 1024)
4848
stdCapRange := &csi.CapacityRange{RequiredBytes: stdVolSize}
49-
stdParams := map[string]string{}
49+
stdParams := map[string]string{
50+
"type": cloud.DefaultVolumeType,
51+
}
5052

5153
testCases := []struct {
5254
name string
@@ -60,14 +62,15 @@ func TestCreateVolume(t *testing.T) {
6062
Name: "random-vol-name",
6163
CapacityRange: stdCapRange,
6264
VolumeCapabilities: stdVolCap,
63-
Parameters: nil,
65+
Parameters: stdParams,
6466
}
6567

6668
ctx := context.Background()
6769

6870
mockDisk := &cloud.Disk{
6971
VolumeID: req.Name,
7072
CapacityGiB: util.BytesToGiB(stdVolSize),
73+
DiskType: cloud.DefaultVolumeType,
7174
}
7275

7376
mockCtl := gomock.NewController(t)
@@ -109,7 +112,7 @@ func TestCreateVolume(t *testing.T) {
109112
},
110113
},
111114
},
112-
Parameters: nil,
115+
Parameters: stdParams,
113116
}
114117

115118
ctx := context.Background()
@@ -121,6 +124,7 @@ func TestCreateVolume(t *testing.T) {
121124
mockDiskOpts := &cloud.DiskOptions{
122125
Shareable: true,
123126
CapacityBytes: stdVolSize,
127+
VolumeType: cloud.DefaultVolumeType,
124128
}
125129

126130
mockCtl := gomock.NewController(t)
@@ -163,18 +167,20 @@ func TestCreateVolume(t *testing.T) {
163167
},
164168
},
165169
},
166-
Parameters: nil,
170+
Parameters: stdParams,
167171
}
168172

169173
ctx := context.Background()
170174
mockDisk := &cloud.Disk{
171175
VolumeID: req.Name,
172176
CapacityGiB: util.BytesToGiB(stdVolSize),
173177
Shareable: true,
178+
DiskType: cloud.DefaultVolumeType,
174179
}
175180
mockDiskOpts := &cloud.DiskOptions{
176181
Shareable: true,
177182
CapacityBytes: stdVolSize,
183+
VolumeType: cloud.DefaultVolumeType,
178184
}
179185

180186
mockCtl := gomock.NewController(t)
@@ -217,18 +223,20 @@ func TestCreateVolume(t *testing.T) {
217223
},
218224
},
219225
},
220-
Parameters: nil,
226+
Parameters: stdParams,
221227
}
222228

223229
ctx := context.Background()
224230
mockDisk := &cloud.Disk{
225231
VolumeID: req.Name,
226232
CapacityGiB: util.BytesToGiB(stdVolSize),
227233
Shareable: false,
234+
DiskType: cloud.DefaultVolumeType,
228235
}
229236
mockDiskOpts := &cloud.DiskOptions{
230237
Shareable: false,
231238
CapacityBytes: stdVolSize,
239+
VolumeType: cloud.DefaultVolumeType,
232240
}
233241

234242
mockCtl := gomock.NewController(t)
@@ -319,6 +327,7 @@ func TestCreateVolume(t *testing.T) {
319327
mockDisk := &cloud.Disk{
320328
VolumeID: req.Name,
321329
CapacityGiB: util.BytesToGiB(stdVolSize),
330+
DiskType: cloud.DefaultVolumeType,
322331
}
323332

324333
mockCtl := gomock.NewController(t)
@@ -401,6 +410,7 @@ func TestCreateVolume(t *testing.T) {
401410
mockDisk := &cloud.Disk{
402411
VolumeID: req.Name,
403412
CapacityGiB: util.BytesToGiB(cloud.DefaultVolumeSize),
413+
DiskType: cloud.DefaultVolumeType,
404414
}
405415

406416
mockCtl := gomock.NewController(t)
@@ -449,7 +459,7 @@ func TestCreateVolume(t *testing.T) {
449459
Name: "vol-test",
450460
CapacityRange: &csi.CapacityRange{RequiredBytes: 1073741825},
451461
VolumeCapabilities: stdVolCap,
452-
Parameters: nil,
462+
Parameters: stdParams,
453463
}
454464
expVol := &csi.Volume{
455465
CapacityBytes: 2147483648, // 1 GiB + 1 byte = 2 GiB
@@ -462,6 +472,7 @@ func TestCreateVolume(t *testing.T) {
462472
mockDisk := &cloud.Disk{
463473
VolumeID: req.Name,
464474
CapacityGiB: util.BytesToGiB(expVol.CapacityBytes),
475+
DiskType: cloud.DefaultVolumeType,
465476
}
466477

467478
mockCtl := gomock.NewController(t)
@@ -516,14 +527,19 @@ func TestCreateVolume(t *testing.T) {
516527
mockDisk := &cloud.Disk{
517528
VolumeID: req.Name,
518529
CapacityGiB: util.BytesToGiB(volSize),
530+
DiskType: cloud.VolumeTypeTier1,
519531
}
520532

533+
mockDiskOpts := &cloud.DiskOptions{
534+
CapacityBytes: volSize,
535+
VolumeType: cloud.VolumeTypeTier1,
536+
}
521537
mockCtl := gomock.NewController(t)
522538
defer mockCtl.Finish()
523539

524540
mockCloud := mocks.NewMockCloud(mockCtl)
525541
mockCloud.EXPECT().GetDiskByName(gomock.Eq(req.Name)).Return(nil, nil)
526-
mockCloud.EXPECT().CreateDisk(gomock.Eq(req.Name), gomock.Any()).Return(mockDisk, nil)
542+
mockCloud.EXPECT().CreateDisk(gomock.Eq(req.Name), mockDiskOpts).Return(mockDisk, nil)
527543

528544
powervsDriver := controllerService{
529545
cloud: mockCloud,
@@ -560,14 +576,19 @@ func TestCreateVolume(t *testing.T) {
560576
mockDisk := &cloud.Disk{
561577
VolumeID: req.Name,
562578
CapacityGiB: util.BytesToGiB(volSize),
579+
DiskType: cloud.VolumeTypeTier3,
580+
}
581+
mockDiskOpts := &cloud.DiskOptions{
582+
CapacityBytes: volSize,
583+
VolumeType: cloud.VolumeTypeTier3,
563584
}
564585

565586
mockCtl := gomock.NewController(t)
566587
defer mockCtl.Finish()
567588

568589
mockCloud := mocks.NewMockCloud(mockCtl)
569590
mockCloud.EXPECT().GetDiskByName(gomock.Eq(req.Name)).Return(nil, nil)
570-
mockCloud.EXPECT().CreateDisk(gomock.Eq(req.Name), gomock.Any()).Return(mockDisk, nil)
591+
mockCloud.EXPECT().CreateDisk(gomock.Eq(req.Name), mockDiskOpts).Return(mockDisk, nil)
571592

572593
powervsDriver := controllerService{
573594
cloud: mockCloud,
@@ -624,6 +645,54 @@ func TestCreateVolume(t *testing.T) {
624645
}
625646
},
626647
},
648+
{
649+
name: "Pass with no volume type parameter",
650+
testFunc: func(t *testing.T) {
651+
// iops 5000 requires at least 10GB
652+
volSize := int64(20 * 1024 * 1024 * 1024)
653+
capRange := &csi.CapacityRange{RequiredBytes: volSize}
654+
req := &csi.CreateVolumeRequest{
655+
Name: "vol-test",
656+
CapacityRange: capRange,
657+
VolumeCapabilities: stdVolCap,
658+
Parameters: map[string]string{},
659+
}
660+
661+
ctx := context.Background()
662+
663+
mockDisk := &cloud.Disk{
664+
VolumeID: req.Name,
665+
CapacityGiB: util.BytesToGiB(volSize),
666+
DiskType: cloud.DefaultVolumeType,
667+
}
668+
669+
mockDiskOpts := &cloud.DiskOptions{
670+
CapacityBytes: volSize,
671+
VolumeType: cloud.DefaultVolumeType,
672+
}
673+
674+
mockCtl := gomock.NewController(t)
675+
defer mockCtl.Finish()
676+
677+
mockCloud := mocks.NewMockCloud(mockCtl)
678+
mockCloud.EXPECT().GetDiskByName(gomock.Eq(req.Name)).Return(nil, nil)
679+
mockCloud.EXPECT().CreateDisk(gomock.Eq(req.Name), mockDiskOpts).Return(mockDisk, nil)
680+
681+
powervsDriver := controllerService{
682+
cloud: mockCloud,
683+
driverOptions: &Options{},
684+
volumeLocks: util.NewVolumeLocks(),
685+
}
686+
687+
if _, err := powervsDriver.CreateVolume(ctx, req); err != nil {
688+
srvErr, ok := status.FromError(err)
689+
if !ok {
690+
t.Fatalf("Could not get error status code from error: %v", srvErr)
691+
}
692+
t.Fatalf("Unexpected error: %v", srvErr.Code())
693+
}
694+
},
695+
},
627696
{
628697
name: "fail locked volume request",
629698
testFunc: func(t *testing.T) {

0 commit comments

Comments
 (0)