Repository: kubernetes-sigs/gcp-compute-persistent-disk-csi-driver
Pull request #1875 (2024-11-19 17:26:31)

Require VACs to use SI units
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>
> /kind api-change
> /kind bug

/kind cleanup
> /kind design
> /kind documentation
> /kind failing-test
> /kind feature
> /kind flake

**What this PR does / why we need it**:
This PR updates the logic to accept SI units for throughput for VolumeAttributesClasses. This makes the VACs consistent with SI being used in other areas, such as in StorageClasses.

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
This feature changes VolumeAttributesClass, so throughput specifications will need "Mi".
```



Patch:
diff --git a/examples/kubernetes/demo-vol-create.yaml b/examples/kubernetes/demo-vol-create.yaml
index 1ca0e055..5fa53604 100644
--- a/examples/kubernetes/demo-vol-create.yaml
+++ b/examples/kubernetes/demo-vol-create.yaml
@@ -16,7 +16,7 @@ metadata:
 driverName: pd.csi.storage.gke.io
 parameters:
   iops: "3000"
-  throughput: "150"
+  throughput: "150Mi"
 ---
 apiVersion: storage.k8s.io/v1beta1
 kind: VolumeAttributesClass
@@ -25,7 +25,7 @@ metadata:
 driverName: pd.csi.storage.gke.io
 parameters:
   iops: "3013"
-  throughput: "151"
+  throughput: "151Mi"
 ---
 apiVersion: v1
 kind: PersistentVolumeClaim
diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go
index e80346ed..87283719 100644
--- a/pkg/common/parameters.go
+++ b/pkg/common/parameters.go
@@ -343,7 +343,7 @@ func ExtractModifyVolumeParameters(parameters map[string]string) (ModifyVolumePa
 			}
 			modifyVolumeParams.IOPS = &iops
 		case "throughput":
-			throughput, err := ConvertStringToInt64(value)
+			throughput, err := ConvertMiStringToInt64(value)
 			if err != nil {
 				return ModifyVolumeParameters{}, fmt.Errorf("parameters contain invalid throughput parameter: %w", err)
 			}


Test patch:
diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go
index cb60ecff..35a11632 100644
--- a/pkg/common/parameters_test.go
+++ b/pkg/common/parameters_test.go
@@ -485,7 +485,7 @@ func TestSnapshotParameters(t *testing.T) {
 func TestExtractModifyVolumeParameters(t *testing.T) {
 	parameters := map[string]string{
 		"iops":       "1000",
-		"throughput": "500",
+		"throughput": "500Mi",
 	}
 
 	iops := int64(1000)
diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go
index 0e79a366..06db7222 100644
--- a/pkg/gce-pd-csi-driver/controller_test.go
+++ b/pkg/gce-pd-csi-driver/controller_test.go
@@ -1789,7 +1789,7 @@ func TestCreateVolumeWithVolumeAttributeClassParameters(t *testing.T) {
 						},
 					},
 				},
-				MutableParameters: map[string]string{"iops": "20000", "throughput": "600"},
+				MutableParameters: map[string]string{"iops": "20000", "throughput": "600Mi"},
 			},
 			expIops:       20000,
 			expThroughput: 600,
@@ -1822,7 +1822,7 @@ func TestCreateVolumeWithVolumeAttributeClassParameters(t *testing.T) {
 						},
 					},
 				},
-				MutableParameters: map[string]string{"iops": "20000", "throughput": "600"},
+				MutableParameters: map[string]string{"iops": "20000", "throughput": "600Mi"},
 			},
 			expIops:       0,
 			expThroughput: 0,
@@ -1890,7 +1890,7 @@ func TestVolumeModifyOperation(t *testing.T) {
 			name: "Update volume with valid parameters",
 			req: &csi.ControllerModifyVolumeRequest{
 				VolumeId:          testVolumeID,
-				MutableParameters: map[string]string{"iops": "20000", "throughput": "600"},
+				MutableParameters: map[string]string{"iops": "20000", "throughput": "600Mi"},
 			},
 			diskType: "hyperdisk-balanced",
 			params: &common.DiskParameters{
@@ -1906,7 +1906,7 @@ func TestVolumeModifyOperation(t *testing.T) {
 			name: "Update volume with invalid parameters",
 			req: &csi.ControllerModifyVolumeRequest{
 				VolumeId:          testVolumeID,
-				MutableParameters: map[string]string{"iops": "0", "throughput": "0"},
+				MutableParameters: map[string]string{"iops": "0", "throughput": "0Mi"},
 			},
 			diskType: "hyperdisk-balanced",
 			params: &common.DiskParameters{
@@ -1922,7 +1922,7 @@ func TestVolumeModifyOperation(t *testing.T) {
 			name: "Update volume with valid parameters but invalid disk type",
 			req: &csi.ControllerModifyVolumeRequest{
 				VolumeId:          testVolumeID,
-				MutableParameters: map[string]string{"iops": "20000", "throughput": "600"},
+				MutableParameters: map[string]string{"iops": "20000", "throughput": "600Mi"},
 			},
 			diskType: "pd-ssd",
 			params: &common.DiskParameters{
@@ -2053,7 +2053,7 @@ func TestVolumeModifyErrorHandling(t *testing.T) {
 				},
 			},
 			modifyReq: &csi.ControllerModifyVolumeRequest{
-				MutableParameters: map[string]string{"iops": "3001", "throughput": "151"},
+				MutableParameters: map[string]string{"iops": "3001", "throughput": "151Mi"},
 			},
 			modifyVolumeErrors: map[*meta.Key]error{
 				meta.ZonalKey(name, "us-central1-a"): &googleapi.Error{
@@ -2089,7 +2089,7 @@ func TestVolumeModifyErrorHandling(t *testing.T) {
 				},
 			},
 			modifyReq: &csi.ControllerModifyVolumeRequest{
-				MutableParameters: map[string]string{"iops": "10000", "throughput": "2400"},
+				MutableParameters: map[string]string{"iops": "10000", "throughput": "2400Mi"},
 			},
 			modifyVolumeErrors: map[*meta.Key]error{
 				meta.ZonalKey(name, "us-central1-a"): &googleapi.Error{Code: int(codes.InvalidArgument), Message: "InvalidArgument"},
diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go
index cd60d8b7..c77c7b91 100644
--- a/test/e2e/tests/single_zone_e2e_test.go
+++ b/test/e2e/tests/single_zone_e2e_test.go
@@ -1622,7 +1622,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
 			stringPtr(provisionedIOPSOnCreateHdb),
 			stringPtr(provisionedThroughputOnCreateHdb),
 			stringPtr("3013"),
-			stringPtr("181"),
+			stringPtr("181Mi"),
 		),
 		Entry(
 			"for hyperdisk-extreme",
@@ -1640,7 +1640,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
 			nil,
 			stringPtr(provisionedThroughputOnCreate),
 			nil,
-			stringPtr("70"),
+			stringPtr("70Mi"),
 		),
 	)
 })
diff --git a/test/k8s-integration/config/hdb-volumeattributesclass.yaml b/test/k8s-integration/config/hdb-volumeattributesclass.yaml
index f032912d..73d5ff4f 100644
--- a/test/k8s-integration/config/hdb-volumeattributesclass.yaml
+++ b/test/k8s-integration/config/hdb-volumeattributesclass.yaml
@@ -5,4 +5,4 @@ metadata:
 driverName: pd.csi.storage.gke.io
 parameters:
   iops: "3600"
-  throughput: "290"
\ No newline at end of file
+  throughput: "290Mi"
\ No newline at end of file