Merge pull request #1128 from sunnylovestiramisu/fixExtreme

Change iops params directly convert string to int64
kubernetes-sigs · Feb 9, 2023 · 0a39f68 · 0a39f68
2 parents 831d1ea + 50853f3
commit 0a39f68
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 64 deletions.
diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go
@@ -152,7 +152,7 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
 				p.Labels[labelKey] = labelValue
 			}
 		case ParameterKeyProvisionedIOPSOnCreate:
-			paramProvisionedIOPSOnCreate, err := ConvertGiBStringToInt64(v)
+			paramProvisionedIOPSOnCreate, err := ConvertStringToInt64(v)
 			if err != nil {
 				return p, fmt.Errorf("parameters contain invalid provisionedIOPSOnCreate parameter: %w", err)
 			}

diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go
@@ -76,7 +76,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 		},
 		{
 			name:       "values from parameters, checking pd-extreme",
-			parameters: map[string]string{ParameterKeyType: "pd-extreme", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyProvisionedIOPSOnCreate: "10000Gi"},
+			parameters: map[string]string{ParameterKeyType: "pd-extreme", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyProvisionedIOPSOnCreate: "10k"},
 			labels:     map[string]string{},
 			expectParams: DiskParameters{
 				DiskType:             "pd-extreme",

diff --git a/pkg/common/utils.go b/pkg/common/utils.go
@@ -251,24 +251,20 @@ func ValidateSnapshotType(snapshotType string) error {
 	}
 }
 
-// ConvertGiBStringToInt64 converts a GiB string to int64
-func ConvertGiBStringToInt64(str string) (int64, error) {
-	// Verify regex before
-	match, _ := regexp.MatchString("^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$", str)
-	if !match {
-		return 0, fmt.Errorf("invalid string %s", str)
+// ConvertStringToInt64 converts a string to int64
+func ConvertStringToInt64(str string) (int64, error) {
+	quantity, err := resource.ParseQuantity(str)
+	if err != nil {
+		return -1, err
 	}
-	quantity := resource.MustParse(str)
-	return volumehelpers.RoundUpToGiB(quantity)
+	return volumehelpers.RoundUpToB(quantity)
 }
 
 // ConvertMiBStringToInt64 converts a GiB string to int64
 func ConvertMiBStringToInt64(str string) (int64, error) {
-	// Verify regex before
-	match, _ := regexp.MatchString("^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$", str)
-	if !match {
-		return 0, fmt.Errorf("invalid string %s", str)
+	quantity, err := resource.ParseQuantity(str)
+	if err != nil {
+		return -1, err
 	}
-	quantity := resource.MustParse(str)
 	return volumehelpers.RoundUpToMiB(quantity)
 }
diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go
@@ -578,83 +578,113 @@ func TestSnapshotStorageLocations(t *testing.T) {
 	}
 }
 
-func TestConvertGiBStringToInt64(t *testing.T) {
+func TestConvertStringToInt64(t *testing.T) {
 	tests := []struct {
 		desc        string
 		inputStr    string
 		expInt64    int64
 		expectError bool
 	}{
 		{
-			"valid number string",
-			"10000",
-			1,
-			false,
+			desc:        "valid number string",
+			inputStr:    "10000",
+			expInt64:    10000,
+			expectError: false,
 		},
 		{
-			"round Ki to GiB",
-			"1000Ki",
-			1,
-			false,
+			desc:        "round M to number",
+			inputStr:    "1M",
+			expInt64:    1000000,
+			expectError: false,
 		},
 		{
-			"round k to GiB",
-			"1000k",
-			1,
-			false,
+			desc:        "round m to number",
+			inputStr:    "1m",
+			expInt64:    1,
+			expectError: false,
 		},
 		{
-			"round Mi to GiB",
-			"1000Mi",
-			1,
-			false,
+			desc:        "round k to number",
+			inputStr:    "1k",
+			expInt64:    1000,
+			expectError: false,
 		},
 		{
-			"round M to GiB",
-			"1000M",
-			1,
-			false,
+			desc:        "invalid empty string",
+			inputStr:    "",
+			expInt64:    10000,
+			expectError: true,
 		},
 		{
-			"round G to GiB",
-			"1000G",
-			932,
-			false,
+			desc:        "invalid string",
+			inputStr:    "ew%65",
+			expInt64:    10000,
+			expectError: true,
 		},
 		{
-			"round Gi to GiB",
-			"10000Gi",
-			10000,
-			false,
+			desc:        "invalid KiB string",
+			inputStr:    "10KiB",
+			expInt64:    10000,
+			expectError: true,
 		},
 		{
-			"round decimal to GiB",
-			"1.2Gi",
-			2,
-			false,
+			desc:        "invalid GB string",
+			inputStr:    "10GB",
+			expInt64:    10000,
+			expectError: true,
 		},
 		{
-			"round big value to GiB",
-			"8191Pi",
-			8588886016,
-			false,
+			desc:        "round Ki to number",
+			inputStr:    "1Ki",
+			expInt64:    1024,
+			expectError: false,
 		},
 		{
-			"invalid empty string",
-			"",
-			10000,
-			true,
+			desc:        "round k to number",
+			inputStr:    "10k",
+			expInt64:    10000,
+			expectError: false,
 		},
 		{
-			"invalid string",
-			"ew%65",
-			10000,
-			true,
+			desc:        "round Mi to number",
+			inputStr:    "10Mi",
+			expInt64:    10485760,
+			expectError: false,
+		},
+		{
+			desc:        "round M to number",
+			inputStr:    "10M",
+			expInt64:    10000000,
+			expectError: false,
+		},
+		{
+			desc:        "round G to number",
+			inputStr:    "10G",
+			expInt64:    10000000000,
+			expectError: false,
+		},
+		{
+			desc:        "round Gi to number",
+			inputStr:    "100Gi",
+			expInt64:    107374182400,
+			expectError: false,
+		},
+		{
+			desc:        "round decimal to number",
+			inputStr:    "1.2Gi",
+			expInt64:    1288490189,
+			expectError: false,
+		},
+		{
+			desc:        "round big value to number",
+			inputStr:    "8191Pi",
+			expInt64:    9222246136947933184,
+			expectError: false,
 		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
-			actualInt64, err := ConvertGiBStringToInt64(tc.inputStr)
+			actualInt64, err := ConvertStringToInt64(tc.inputStr)
 			if err != nil && !tc.expectError {
 				t.Errorf("Got error %v converting string to int64 %s; expect no error", err, tc.inputStr)
 			}

diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go
@@ -51,7 +51,7 @@ const (
 	readyState                       = "READY"
 	standardDiskType                 = "pd-standard"
 	extremeDiskType                  = "pd-extreme"
-	provisionedIOPSOnCreate          = "100000Gi"
+	provisionedIOPSOnCreate          = "100000"
 	provisionedIOPSOnCreateInt       = int64(100000)
 
 	defaultEpsilon = 500000000 // 500M

diff --git a/test/k8s-integration/config/sc-extreme.yaml b/test/k8s-integration/config/sc-extreme.yaml
@@ -5,7 +5,7 @@ metadata:
 provisioner: pd.csi.storage.gke.io
 parameters:
   type: pd-extreme
-  provisioned-iops-on-create: '10000Gi'
+  provisioned-iops-on-create: '10000'
   # Add labels for testing.
   labels: key1=value1,key2=value2
-volumeBindingMode: WaitForFirstConsumer
+volumeBindingMode: WaitForFirstConsumer