Skip to content

Commit 5327262

Browse files
sunnylovestiramisusunnylovestiramisu
committed
Change iops params directly convert string to int64
1 parent 190beb9 commit 5327262

File tree

9 files changed

+128
-100
lines changed

9 files changed

+128
-100
lines changed

‎pkg/common/parameters.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
139139
p.Labels[labelKey] = labelValue
140140
}
141141
case ParameterKeyProvisionedIOPSOnCreate:
142-
paramProvisionedIOPSOnCreate, err := ConvertGiBStringToInt64(v)
142+
paramProvisionedIOPSOnCreate, err := ConvertStringToInt64(v)
143143
if err != nil {
144144
return p, fmt.Errorf("parameters contain invalid provisionedIOPSOnCreate parameter: %w", err)
145145
}

‎pkg/common/parameters_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
7676
},
7777
{
7878
name: "values from parameters, checking pd-extreme",
79-
parameters: map[string]string{ParameterKeyType: "pd-extreme", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyProvisionedIOPSOnCreate: "10000Gi"},
79+
parameters: map[string]string{ParameterKeyType: "pd-extreme", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyProvisionedIOPSOnCreate: "10k"},
8080
labels: map[string]string{},
8181
expectParams: DiskParameters{
8282
DiskType: "pd-extreme",

‎pkg/common/utils.go

+6-8
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,11 @@ func ParseMachineType(machineTypeUrl string) (string, error) {
271271
return machineType[1], nil
272272
}
273273

274-
// ConvertGiBStringToInt64 converts a GiB string to int64
275-
func ConvertGiBStringToInt64(str string) (int64, error) {
276-
// Verify regex before
277-
match, _ := regexp.MatchString("^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$", str)
278-
if !match {
279-
return 0, fmt.Errorf("invalid string %s", str)
274+
// ConvertStringToInt64 converts a string to int64
275+
func ConvertStringToInt64(str string) (int64, error) {
276+
quantity, err := resource.ParseQuantity(str)
277+
if err != nil {
278+
return -1, err
280279
}
281-
quantity := resource.MustParse(str)
282-
return volumehelpers.RoundUpToGiB(quantity)
280+
return volumehelpers.RoundUpToB(quantity)
283281
}

‎pkg/common/utils_test.go

+82-46
Original file line numberDiff line numberDiff line change
@@ -632,83 +632,119 @@ func TestParseMachineType(t *testing.T) {
632632
}
633633
}
634634

635-
func TestConvertGiBStringToInt64(t *testing.T) {
635+
func TestConvertStringToInt64(t *testing.T) {
636636
tests := []struct {
637637
desc string
638638
inputStr string
639639
expInt64 int64
640640
expectError bool
641641
}{
642642
{
643-
"valid number string",
644-
"10000",
645-
1,
646-
false,
643+
desc: "valid number string",
644+
inputStr: "10000",
645+
expInt64: 10000,
646+
expectError: false,
647647
},
648648
{
649-
"round Ki to GiB",
650-
"1000Ki",
651-
1,
652-
false,
649+
desc: "test higher number",
650+
inputStr: "15000",
651+
expInt64: 15000,
652+
expectError: false,
653653
},
654654
{
655-
"round k to GiB",
656-
"1000k",
657-
1,
658-
false,
655+
desc: "round M to number",
656+
inputStr: "1M",
657+
expInt64: 1000000,
658+
expectError: false,
659659
},
660660
{
661-
"round Mi to GiB",
662-
"1000Mi",
663-
1,
664-
false,
661+
desc: "round m to number",
662+
inputStr: "1m",
663+
expInt64: 1,
664+
expectError: false,
665665
},
666666
{
667-
"round M to GiB",
668-
"1000M",
669-
1,
670-
false,
667+
desc: "round k to number",
668+
inputStr: "1k",
669+
expInt64: 1000,
670+
expectError: false,
671671
},
672672
{
673-
"round G to GiB",
674-
"1000G",
675-
932,
676-
false,
673+
desc: "invalid empty string",
674+
inputStr: "",
675+
expInt64: 10000,
676+
expectError: true,
677677
},
678678
{
679-
"round Gi to GiB",
680-
"10000Gi",
681-
10000,
682-
false,
679+
desc: "invalid string",
680+
inputStr: "ew%65",
681+
expInt64: 10000,
682+
expectError: true,
683683
},
684684
{
685-
"round decimal to GiB",
686-
"1.2Gi",
687-
2,
688-
false,
685+
desc: "invalid KiB string",
686+
inputStr: "10KiB",
687+
expInt64: 10000,
688+
expectError: true,
689689
},
690690
{
691-
"round big value to GiB",
692-
"8191Pi",
693-
8588886016,
694-
false,
691+
desc: "invalid GB string",
692+
inputStr: "10GB",
693+
expInt64: 10000,
694+
expectError: true,
695695
},
696696
{
697-
"invalid empty string",
698-
"",
699-
10000,
700-
true,
697+
desc: "round Ki to number",
698+
inputStr: "1Ki",
699+
expInt64: 1024,
700+
expectError: false,
701701
},
702702
{
703-
"invalid string",
704-
"ew%65",
705-
10000,
706-
true,
703+
desc: "round k to number",
704+
inputStr: "10k",
705+
expInt64: 10000,
706+
expectError: false,
707+
},
708+
{
709+
desc: "round Mi to number",
710+
inputStr: "10Mi",
711+
expInt64: 10485760,
712+
expectError: false,
713+
},
714+
{
715+
desc: "round M to number",
716+
inputStr: "10M",
717+
expInt64: 10000000,
718+
expectError: false,
719+
},
720+
{
721+
desc: "round G to number",
722+
inputStr: "10G",
723+
expInt64: 10000000000,
724+
expectError: false,
725+
},
726+
{
727+
desc: "round Gi to number",
728+
inputStr: "100Gi",
729+
expInt64: 107374182400,
730+
expectError: false,
731+
},
732+
{
733+
desc: "round decimal to number",
734+
inputStr: "1.2Gi",
735+
expInt64: 1288490189,
736+
expectError: false,
737+
},
738+
{
739+
desc: "round big value to number",
740+
inputStr: "8191Pi",
741+
expInt64: 9222246136947933184,
742+
expectError: false,
707743
},
708744
}
709745
for _, tc := range tests {
710746
t.Run(tc.desc, func(t *testing.T) {
711-
actualInt64, err := ConvertGiBStringToInt64(tc.inputStr)
747+
actualInt64, err := ConvertStringToInt64(tc.inputStr)
712748
if err != nil && !tc.expectError {
713749
t.Errorf("Got error %v converting string to int64 %s; expect no error", err, tc.inputStr)
714750
}

‎pkg/gce-cloud-provider/compute/gce-compute.go

+17-12
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ func convertV1DiskToBetaDisk(v1Disk *computev1.Disk) *computebeta.Disk {
409409
}
410410

411411
// Note: this is an incomplete list. It only includes the fields we use for disk creation.
412-
return &computebeta.Disk{
412+
betaDisk := &computebeta.Disk{
413413
Name: v1Disk.Name,
414414
SizeGb: v1Disk.SizeGb,
415415
Description: v1Disk.Description,
@@ -418,6 +418,11 @@ func convertV1DiskToBetaDisk(v1Disk *computev1.Disk) *computebeta.Disk {
418418
ReplicaZones: v1Disk.ReplicaZones,
419419
DiskEncryptionKey: dek,
420420
}
421+
if v1Disk.ProvisionedIops > 0 {
422+
betaDisk.ProvisionedIops = v1Disk.ProvisionedIops
423+
}
424+
425+
return betaDisk
421426
}
422427

423428
func (cloud *CloudProvider) insertRegionalDisk(
@@ -443,12 +448,11 @@ func (cloud *CloudProvider) insertRegionalDisk(
443448
}
444449

445450
diskToCreate := &computev1.Disk{
446-
Name: volKey.Name,
447-
SizeGb: common.BytesToGbRoundUp(capBytes),
448-
Description: description,
449-
Type: cloud.GetDiskTypeURI(cloud.project, volKey, params.DiskType),
450-
Labels: params.Labels,
451-
ProvisionedIops: params.ProvisionedIOPSOnCreate,
451+
Name: volKey.Name,
452+
SizeGb: common.BytesToGbRoundUp(capBytes),
453+
Description: description,
454+
Type: cloud.GetDiskTypeURI(cloud.project, volKey, params.DiskType),
455+
Labels: params.Labels,
452456
}
453457
if snapshotID != "" {
454458
_, snapshotType, _, err := common.SnapshotIDToProjectKey(snapshotID)
@@ -556,11 +560,12 @@ func (cloud *CloudProvider) insertZonalDisk(
556560
}
557561

558562
diskToCreate := &computev1.Disk{
559-
Name: volKey.Name,
560-
SizeGb: common.BytesToGbRoundUp(capBytes),
561-
Description: description,
562-
Type: cloud.GetDiskTypeURI(project, volKey, params.DiskType),
563-
Labels: params.Labels,
563+
Name: volKey.Name,
564+
SizeGb: common.BytesToGbRoundUp(capBytes),
565+
Description: description,
566+
Type: cloud.GetDiskTypeURI(project, volKey, params.DiskType),
567+
Labels: params.Labels,
568+
ProvisionedIops: params.ProvisionedIOPSOnCreate,
564569
}
565570

566571
if snapshotID != "" {

‎test/e2e/tests/single_zone_e2e_test.go

+11-7
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,16 @@ import (
4343
const (
4444
testNamePrefix = "gcepd-csi-e2e-"
4545

46-
defaultSizeGb int64 = 5
47-
defaultRepdSizeGb int64 = 200
48-
defaultMwSizeGb int64 = 200
49-
readyState = "READY"
50-
standardDiskType = "pd-standard"
51-
defaultVolumeLimit int64 = 127
46+
defaultSizeGb int64 = 5
47+
defaultExtremeSizeGb int64 = 500
48+
defaultRepdSizeGb int64 = 200
49+
defaultMwSizeGb int64 = 200
50+
defaultVolumeLimit int64 = 127
51+
readyState = "READY"
52+
standardDiskType = "pd-standard"
53+
extremeDiskType = "pd-extreme"
54+
provisionedIOPSOnCreate = "12345"
55+
provisionedIOPSOnCreateInt = int64(12345)
5256

5357
defaultEpsilon = 500000000 // 500M
5458
)
@@ -1177,7 +1181,7 @@ func createAndValidateUniqueZonalDisk(client *remote.CsiClient, project, zone st
11771181
Expect(err).To(BeNil(), "Could not get disk from cloud directly")
11781182
Expect(cloudDisk.Type).To(ContainSubstring(diskType))
11791183
Expect(cloudDisk.Status).To(Equal(readyState))
1180-
Expect(cloudDisk.SizeGb).To(Equal(defaultSizeGb))
1184+
Expect(cloudDisk.SizeGb).To(Equal(diskSize))
11811185
Expect(cloudDisk.Name).To(Equal(volName))
11821186
return
11831187
}

‎test/e2e/tests/single_zone_pd_extreme_e2e_test.go

+8-14
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@ import (
3232
fieldmask "google.golang.org/genproto/protobuf/field_mask"
3333
)
3434

35-
const (
36-
extremeDiskType = "pd-extreme"
37-
provisionedIOPSOnCreate = "100000Gi"
38-
provisionedIOPSOnCreateInt = int64(100000)
39-
)
40-
4135
var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
4236

4337
It("Should create and delete pd-extreme disk", func() {
@@ -53,7 +47,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
5347
common.ParameterKeyType: extremeDiskType,
5448
common.ParameterKeyProvisionedIOPSOnCreate: provisionedIOPSOnCreate,
5549
}
56-
volID, err := client.CreateVolume(volName, params, defaultSizeGb, nil, nil)
50+
volID, err := client.CreateVolume(volName, params, defaultExtremeSizeGb, nil, nil)
5751
Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
5852

5953
// Validate Disk Created
@@ -62,7 +56,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
6256
Expect(cloudDisk.Type).To(ContainSubstring(extremeDiskType))
6357
Expect(cloudDisk.ProvisionedIops).To(Equal(provisionedIOPSOnCreateInt))
6458
Expect(cloudDisk.Status).To(Equal(readyState))
65-
Expect(cloudDisk.SizeGb).To(Equal(defaultSizeGb))
59+
Expect(cloudDisk.SizeGb).To(Equal(defaultExtremeSizeGb))
6660
Expect(cloudDisk.Name).To(Equal(volName))
6761

6862
defer func() {
@@ -90,7 +84,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
9084
common.ParameterKeyType: extremeDiskType,
9185
common.ParameterKeyProvisionedIOPSOnCreate: provisionedIOPSOnCreate,
9286
}
93-
volID, err := client.CreateVolume(volName, params, defaultSizeGb, nil, nil)
87+
volID, err := client.CreateVolume(volName, params, defaultExtremeSizeGb, nil, nil)
9488
Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
9589

9690
// Validate Disk Created
@@ -99,7 +93,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
9993
Expect(cloudDisk.Type).To(ContainSubstring(extremeDiskType))
10094
Expect(cloudDisk.ProvisionedIops).To(Equal(provisionedIOPSOnCreateInt))
10195
Expect(cloudDisk.Status).To(Equal(readyState))
102-
Expect(cloudDisk.SizeGb).To(Equal(defaultSizeGb))
96+
Expect(cloudDisk.SizeGb).To(Equal(defaultExtremeSizeGb))
10397
Expect(cloudDisk.Labels).To(Equal(map[string]string{
10498
"key1": "value1",
10599
"key2": "value2",
@@ -157,7 +151,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
157151
common.ParameterKeyDiskEncryptionKmsKey: key.Name,
158152
common.ParameterKeyType: extremeDiskType,
159153
common.ParameterKeyProvisionedIOPSOnCreate: provisionedIOPSOnCreate,
160-
}, defaultSizeGb,
154+
}, defaultExtremeSizeGb,
161155
&csi.TopologyRequirement{
162156
Requisite: []*csi.Topology{
163157
{
@@ -173,7 +167,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
173167
Expect(cloudDisk.Type).To(ContainSubstring(extremeDiskType))
174168
Expect(cloudDisk.ProvisionedIops).To(Equal(provisionedIOPSOnCreateInt))
175169
Expect(cloudDisk.Status).To(Equal(readyState))
176-
Expect(cloudDisk.SizeGb).To(Equal(defaultSizeGb))
170+
Expect(cloudDisk.SizeGb).To(Equal(defaultExtremeSizeGb))
177171
Expect(cloudDisk.Name).To(Equal(volName))
178172

179173
defer func() {
@@ -250,7 +244,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
250244
common.ParameterKeyPVName: "test-pv-name",
251245
common.ParameterKeyType: extremeDiskType,
252246
common.ParameterKeyProvisionedIOPSOnCreate: provisionedIOPSOnCreate,
253-
}, defaultSizeGb, nil /* topReq */, nil)
247+
}, defaultExtremeSizeGb, nil /* topReq */, nil)
254248
Expect(err).To(BeNil(), "CreateVolume failed with error: %v", err)
255249

256250
// Validate Disk Created
@@ -259,7 +253,7 @@ var _ = Describe("GCE PD CSI Driver pd-extreme", func() {
259253
Expect(cloudDisk.Type).To(ContainSubstring(extremeDiskType))
260254
Expect(cloudDisk.ProvisionedIops).To(Equal(provisionedIOPSOnCreateInt))
261255
Expect(cloudDisk.Status).To(Equal(readyState))
262-
Expect(cloudDisk.SizeGb).To(Equal(defaultSizeGb))
256+
Expect(cloudDisk.SizeGb).To(Equal(defaultExtremeSizeGb))
263257
Expect(cloudDisk.Name).To(Equal(volName))
264258
Expect(cloudDisk.Description).To(Equal("{\"kubernetes.io/created-for/pv/name\":\"test-pv-name\",\"kubernetes.io/created-for/pvc/name\":\"test-pvc\",\"kubernetes.io/created-for/pvc/namespace\":\"test-pvc-namespace\",\"storage.gke.io/created-by\":\"pd.csi.storage.gke.io\"}"))
265259
defer func() {

‎test/k8s-integration/config/sc-extreme.yaml

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ metadata:
55
provisioner: pd.csi.storage.gke.io
66
parameters:
77
type: pd-extreme
8-
provisioned-iops-on-create: '10000Gi'
8+
provisioned-iops-on-create: '10000'
99
# Add labels for testing.
1010
labels: key1=value1,key2=value2
11-
volumeBindingMode: WaitForFirstConsumer
11+
volumeBindingMode: WaitForFirstConsumer

0 commit comments

Comments
 (0)