Cleanup ControllerModifyVolume logic and add e2e tests

Author: travisyx

Dockerfile

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM --platform=$BUILDPLATFORM golang:1.22.3 as builder
+FROM --platform=$BUILDPLATFORM golang:1.23.0 as builder
 
 ARG STAGINGVERSION
 ARG TARGETPLATFORM
@@ -22,13 +22,15 @@ ADD . .
 RUN GOARCH=$(echo $TARGETPLATFORM | cut -f2 -d '/') GCE_PD_CSI_STAGING_VERSION=$STAGINGVERSION make gce-pd-driver
 
 # Start from Kubernetes Debian base.
-FROM gke.gcr.io/debian-base:bullseye-v1.4.3-gke.5 as debian
+
+FROM gke.gcr.io/debian-base:bookworm-v1.0.3-gke.0 as debian
+
 # Install necessary dependencies
 # google_nvme_id script depends on the following packages: nvme-cli, xxd, bash
 RUN clean-install util-linux e2fsprogs mount ca-certificates udev xfsprogs nvme-cli xxd bash
 
 # Since we're leveraging apt to pull in dependencies, we use `gcr.io/distroless/base` because it includes glibc.
-FROM gcr.io/distroless/base-debian11 as distroless-base
+FROM gcr.io/distroless/base-debian12 as distroless-base
 
 # The distroless amd64 image has a target triplet of x86_64
 FROM distroless-base AS distroless-amd64
@@ -72,8 +74,7 @@ COPY --from=debian /bin/ln /bin/ln
 COPY --from=debian /bin/udevadm /bin/udevadm
 
 # Copy shared libraries into distroless base.
-COPY --from=debian /lib/${LIB_DIR_PREFIX}-linux-gnu/libpcre.so.3 \
-                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libselinux.so.1 \
+COPY --from=debian /lib/${LIB_DIR_PREFIX}-linux-gnu/libselinux.so.1 \
                    /lib/${LIB_DIR_PREFIX}-linux-gnu/libtinfo.so.6 \
                    /lib/${LIB_DIR_PREFIX}-linux-gnu/libe2p.so.2 \
                    /lib/${LIB_DIR_PREFIX}-linux-gnu/libcom_err.so.2 \
@@ -82,7 +83,21 @@ COPY --from=debian /lib/${LIB_DIR_PREFIX}-linux-gnu/libpcre.so.3 \
                    /lib/${LIB_DIR_PREFIX}-linux-gnu/libgcc_s.so.1 \
                    /lib/${LIB_DIR_PREFIX}-linux-gnu/liblzma.so.5 \
                    /lib/${LIB_DIR_PREFIX}-linux-gnu/libreadline.so.8 \
-                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libz.so.1 /lib/${LIB_DIR_PREFIX}-linux-gnu/
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libz.so.1 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libc.so.6 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/liburcu.so.8 \ 
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libcap.so.2 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libcrypto.so.3 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libdbus-1.so.3 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libgcrypt.so.20 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libjson-c.so.5 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/liblz4.so.1 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libm.so.6 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libnvme-mi.so.1 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libnvme.so.1 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libsystemd.so.0 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libgpg-error.so.0 \
+                   /lib/${LIB_DIR_PREFIX}-linux-gnu/libzstd.so.1 /lib/${LIB_DIR_PREFIX}-linux-gnu/
 
 COPY --from=debian /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libblkid.so.1 \
                    /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libbsd.so.0 \
@@ -93,9 +108,9 @@ COPY --from=debian /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libblkid.so.1 \
                    /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libacl.so.1 \
                    /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libattr.so.1 \
                    /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libedit.so.2 \
-                   /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libicudata.so.67 \
-                   /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libicui18n.so.67 \
-                   /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libicuuc.so.67 \
+                   /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libicudata.so.72 \
+                   /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libicui18n.so.72 \
+                   /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libicuuc.so.72 \
                    /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libkmod.so.2 \
                    /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libmd.so.0 \
                    /usr/lib/${LIB_DIR_PREFIX}-linux-gnu/libpcre2-8.so.0 \

Dockerfile.Windows

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 ARG BASE_IMAGE
-FROM --platform=$BUILDPLATFORM golang:1.22.3 AS builder
+FROM --platform=$BUILDPLATFORM golang:1.23.0 AS builder
 
 ARG TARGETPLATFORM
 ARG STAGINGVERSION

Dockerfile.debug

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM golang:1.22.3 as builder
+FROM golang:1.23.0 as builder
 WORKDIR /go/src/sigs.k8s.io/gcp-compute-persistent-disk-csi-driver
 ADD . .
 

Makefile

@@ -126,7 +126,7 @@ init-buildx:
 	$(DOCKER) run --rm --privileged multiarch/qemu-user-static --reset --credential yes --persistent yes
 	# Ensure we use a builder that can leverage it (the default on linux will not)
 	-$(DOCKER) buildx rm multiarch-multiplatform-builder
-	$(DOCKER) buildx create --use --name=multiarch-multiplatform-builder --driver-opt network=host --driver-opt image=moby/buildkit:v0.11.3
+	$(DOCKER) buildx create --use --name=multiarch-multiplatform-builder --driver-opt network=host --driver-opt image=moby/buildkit:v0.14.1
 	# Register gcloud as a Docker credential helper.
 	# Required for "docker buildx build --push".
 	gcloud auth configure-docker --quiet

cmd/gce-pd-csi-driver/main.go

@@ -28,6 +28,7 @@ import (
 	"time"
 
 	"k8s.io/klog/v2"
+	"k8s.io/utils/strings/slices"
 
 	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
 	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/deviceutils"
@@ -83,6 +84,9 @@ var (
 	useInstanceAPIForListVolumesPublishedNodesFlag = flag.Bool("use-instance-api-to-list-volumes-published-nodes", false, "Enables using the instances.list API to determine published_node_ids in ListVolumes. When false (default), the disks.list API is used")
 	instancesListFiltersFlag                       = flag.String("instances-list-filters", "", "Comma separated list of filters to use when calling the instances.list API. By default instances.list fetches all instances in a region")
 
+	diskSupportsIopsChangeFlag       = flag.String("supports-dynamic-iops-provisioning", "", "Comma separated list of disk types that support dynamic IOPS provisioning")
+	diskSupportsThroughputChangeFlag = flag.String("supports-dynamic-throughput-provisioning", "", "Comma separated list of disk types that support dynamic throughput provisioning")
+
 	extraTagsStr = flag.String("extra-tags", "", "Extra tags to attach to each Compute Disk, Image, Snapshot created. It is a comma separated list of parent id, key and value like '<parent_id1>/<tag_key1>/<tag_value1>,...,<parent_idN>/<tag_keyN>/<tag_valueN>'. parent_id is the Organization or the Project ID or Project name where the tag key and the tag value resources exist. A maximum of 50 tags bindings is allowed for a resource. See https://cloud.google.com/resource-manager/docs/tags/tags-overview, https://cloud.google.com/resource-manager/docs/tags/tags-creating-and-managing for details")
 
 	version string
@@ -98,7 +102,7 @@ func init() {
 	// Use V(4) for general debug information logging
 	// Use V(5) for GCE Cloud Provider Call informational logging
 	// Use V(6) for extra repeated/polling information
-	enumFlag(&computeEnvironment, "compute-environment", allowedComputeEnvironment, "Operating compute environment")
+	stringEnumFlag(&computeEnvironment, "compute-environment", allowedComputeEnvironment, "Operating compute environment")
 	urlFlag(&computeEndpoint, "compute-endpoint", "Compute endpoint")
 	klog.InitFlags(flag.CommandLine)
 	flag.Set("logtostderr", "true")
@@ -175,30 +179,38 @@ func handle() {
 	identityServer := driver.NewIdentityServer(gceDriver)
 
 	// Initialize requisite zones
-	fallbackRequisiteZones := strings.Split(*fallbackRequisiteZonesFlag, ",")
+	fallbackRequisiteZones := parseCSVFlag(*fallbackRequisiteZonesFlag)
 
 	// Initialize multi-zone disk types
-	multiZoneVolumeHandleDiskTypes := strings.Split(*multiZoneVolumeHandleDiskTypesFlag, ",")
+	multiZoneVolumeHandleDiskTypes := parseCSVFlag(*multiZoneVolumeHandleDiskTypesFlag)
 	multiZoneVolumeHandleConfig := driver.MultiZoneVolumeHandleConfig{
 		Enable:    *multiZoneVolumeHandleEnableFlag,
 		DiskTypes: multiZoneVolumeHandleDiskTypes,
 	}
 
 	// Initialize waitForAttach config
-	useInstanceAPIOnWaitForAttachDiskTypes := strings.Split(*useInstanceAPIOnWaitForAttachDiskTypesFlag, ",")
+	useInstanceAPIOnWaitForAttachDiskTypes := parseCSVFlag(*useInstanceAPIOnWaitForAttachDiskTypesFlag)
 	waitForAttachConfig := gce.WaitForAttachConfig{
 		UseInstancesAPIForDiskTypes: useInstanceAPIOnWaitForAttachDiskTypes,
 	}
 
 	// Initialize listVolumes config
-	instancesListFilters := strings.Split(*instancesListFiltersFlag, ",")
+	instancesListFilters := parseCSVFlag(*instancesListFiltersFlag)
 	listInstancesConfig := gce.ListInstancesConfig{
 		Filters: instancesListFilters,
 	}
 	listVolumesConfig := driver.ListVolumesConfig{
 		UseInstancesAPIForPublishedNodes: *useInstanceAPIForListVolumesPublishedNodesFlag,
 	}
 
+	// Initialize provisionableDisks config
+	supportsIopsChange := parseCSVFlag(*diskSupportsIopsChangeFlag)
+	supportsThroughputChange := parseCSVFlag(*diskSupportsThroughputChangeFlag)
+	provisionableDisksConfig := driver.ProvisionableDisksConfig{
+		SupportsIopsChange:       supportsIopsChange,
+		SupportsThroughputChange: supportsThroughputChange,
+	}
+
 	// Initialize requirements for the controller service
 	var controllerServer *driver.GCEControllerServer
 	if *runControllerService {
@@ -208,7 +220,7 @@ func handle() {
 		}
 		initialBackoffDuration := time.Duration(*errorBackoffInitialDurationMs) * time.Millisecond
 		maxBackoffDuration := time.Duration(*errorBackoffMaxDurationMs) * time.Millisecond
-		controllerServer = driver.NewControllerServer(gceDriver, cloudProvider, initialBackoffDuration, maxBackoffDuration, fallbackRequisiteZones, *enableStoragePoolsFlag, multiZoneVolumeHandleConfig, listVolumesConfig)
+		controllerServer = driver.NewControllerServer(gceDriver, cloudProvider, initialBackoffDuration, maxBackoffDuration, fallbackRequisiteZones, *enableStoragePoolsFlag, multiZoneVolumeHandleConfig, listVolumesConfig, provisionableDisksConfig)
 	} else if *cloudConfigFilePath != "" {
 		klog.Warningf("controller service is disabled but cloud config given - it has no effect")
 	}
@@ -252,18 +264,48 @@ func handle() {
 	gceDriver.Run(*endpoint, *grpcLogCharCap, *enableOtelTracing)
 }
 
-func enumFlag(target *gce.Environment, name string, allowedComputeEnvironment []gce.Environment, usage string) {
+func notEmpty(v string) bool {
+	return v != ""
+}
+
+func parseCSVFlag(list string) []string {
+	return slices.Filter(nil, strings.Split(list, ","), notEmpty)
+}
+
+type enumConverter[T any] interface {
+	convert(v string) (T, error)
+	eq(a, b T) bool
+}
+
+type stringConverter[T ~string] struct{}
+
+func (s stringConverter[T]) convert(v string) (T, error) {
+	return T(v), nil
+}
+
+func (s stringConverter[T]) eq(a, b T) bool {
+	return a == b
+}
+
+func stringEnumFlag[T ~string](target *T, name string, allowed []T, usage string) {
+	enumFlag(target, name, stringConverter[T]{}, allowed, usage)
+}
+
+func enumFlag[T any](target *T, name string, converter enumConverter[T], allowed []T, usage string) {
 	flag.Func(name, usage, func(flagValue string) error {
-		for _, allowedValue := range allowedComputeEnvironment {
-			if gce.Environment(flagValue) == allowedValue {
-				*target = gce.Environment(flagValue)
+		tValue, err := converter.convert(flagValue)
+		if err != nil {
+			return err
+		}
+		for _, allowedValue := range allowed {
+			if converter.eq(allowedValue, tValue) {
+				*target = tValue
 				return nil
 			}
 		}
 		errMsg := fmt.Sprintf(`must be one of %v`, allowedComputeEnvironment)
 		return errors.New(errMsg)
 	})
-
 }
 
 func urlFlag(target **url.URL, name string, usage string) {

deploy/kubernetes/base/controller/cluster_setup.yaml

@@ -34,6 +34,9 @@ rules:
   - apiGroups: ["storage.k8s.io"]
     resources: ["storageclasses"]
     verbs: ["get", "list", "watch"]
+  - apiGroups: ["storage.k8s.io"]
+    resources: ["volumeattributesclasses"]
+    verbs: ["get", "list", "watch"]
   - apiGroups: [""]
     resources: ["events"]
     verbs: ["list", "watch", "create", "update", "patch"]
@@ -69,7 +72,7 @@ roleRef:
   kind: ClusterRole
   name: csi-gce-pd-provisioner-role
   apiGroup: rbac.authorization.k8s.io
-  
+
 ---
 # xref: https://github.com/kubernetes-csi/external-attacher/blob/master/deploy/kubernetes/rbac.yaml
 kind: ClusterRole
@@ -143,6 +146,9 @@ rules:
   - apiGroups: [""]
     resources: ["persistentvolumeclaims/status"]
     verbs: ["update", "patch"]
+  - apiGroups: ["storage.k8s.io"]
+    resources: ["volumeattributesclasses"]
+    verbs: ["get", "list", "watch"]
   - apiGroups: [""]
     resources: ["events"]
     verbs: ["list", "watch", "create", "update", "patch"]
@@ -312,4 +318,4 @@ subjects:
 roleRef:
   kind: Role
   name: csi-gce-pd-leaderelection-role
-  apiGroup: rbac.authorization.k8s.io
+  apiGroup: rbac.authorization.k8s.io

deploy/kubernetes/base/controller/controller.yaml

@@ -37,6 +37,7 @@ spec:
             - "--leader-election"
             - "--default-fstype=ext4"
             - "--controller-publish-readonly=true"
+            - "--feature-gates=VolumeAttributesClass=true"
           env:
             - name: PDCSI_NAMESPACE
               valueFrom:
@@ -95,6 +96,7 @@ spec:
             - "--leader-election"
             - "--leader-election-namespace=$(PDCSI_NAMESPACE)"
             - "--handle-volume-inuse-error=false"
+            - "--feature-gates=VolumeAttributesClass=true"
           env:
             - name: PDCSI_NAMESPACE
               valueFrom:

deploy/kubernetes/images/prow-stable-sidecar-rc-master/image.yaml

@@ -48,6 +48,6 @@ metadata:
 imageTag:
   name: registry.k8s.io/cloud-provider-gcp/gcp-compute-persistent-disk-csi-driver
   newName: gcr.io/k8s-staging-cloud-provider-gcp/gcp-compute-persistent-disk-csi-driver
-  newTag: "v1.13.3-rc1"
+  newTag: "v1.14.2-rc1"
 ---
 

deploy/kubernetes/images/stable-master/image.yaml

@@ -4,7 +4,7 @@ metadata:
   name: imagetag-csi-provisioner
 imageTag:
   name: registry.k8s.io/sig-storage/csi-provisioner
-  newTag: "v3.6.3"
+  newTag: "v5.1.0"
 
 ---
 apiVersion: builtin
@@ -22,7 +22,7 @@ metadata:
   name: imagetag-csi-resizer
 imageTag:
   name: registry.k8s.io/sig-storage/csi-resizer
-  newTag: "v1.9.3"
+  newTag: "v1.11.1"
 ---
 
 apiVersion: builtin

deploy/kubernetes/overlays/dev/controller_always_pull.yaml

@@ -7,5 +7,4 @@ spec:
     spec:
       containers:
         - name: gce-pd-driver
-          imagePullPolicy: Always
-
+          imagePullPolicy: Always

deploy/kubernetes/overlays/dev/driver-args.yaml

@@ -0,0 +1,7 @@
+- op: add
+  path: /spec/template/spec/containers/0/args/-
+  value: --supports-dynamic-throughput-provisioning=hyperdisk-balanced,hyperdisk-throughput,hyperdisk-ml
+
+- op: add
+  path: /spec/template/spec/containers/0/args/-
+  value: --supports-dynamic-iops-provisioning=hyperdisk-balanced,hyperdisk-extreme

deploy/kubernetes/overlays/dev/kustomization.yaml

@@ -9,6 +9,14 @@ resources:
 # Here dev overlay is using the same image as alpha
 transformers:
 - ../../images/stable-master
+# Apply patches to support dynamic provisioning for hyperdisks
+patches:
+- path: ./driver-args.yaml
+  target:
+    group: apps
+    version: v1
+    kind: Deployment
+    name: csi-gce-pd-controller
 # To change the dev image, add something like the following.
 #images:
 #- name: gke.gcr.io/gcp-compute-persistent-disk-csi-driver

docs/kubernetes/user-guides/storage-pools.md

@@ -0,0 +1,37 @@
+# Hyperdisk Storage Pools User Guide
+
+Provisioning attached disks in Storage Pools is supported in the managed version of our driver which is automatically enabled on new GKE clusters, and in a manually deployed GCE PD CSI Driver. We recommend using this feature with the managed GCE PD CSI Driver. See the public documentation [here](https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/hyperdisk-storage-pools). 
+
+If you would like to use Storage Pools with a manual deployment of the GCE PD CSI driver, you will need to do a couple additional things to enable the feature as described below.
+
+### Enabling Storage Pools for a Manual Deployment of GCE PD CSI driver.
+
+>**Attention:** Note that Storage Pools is only available in the driver version [v1.13.0+](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/releases/tag/v1.13.0).
+
+In addition to the install instructions [here](driver-install.md), you need to specify the CSI driver command line flags, `--enable-storage-pools=true`. 
+
+### Provision Volumes in a Storage Pool
+
+To provision volumes in a Storage Pool, follow the instructions [here](https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/hyperdisk-storage-pools#provision-attached-disk). When using the feature in a manual deployment of the GCE PD CSI Driver, in the [Create a StorageClass](https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/hyperdisk-storage-pools#create-storageclass) section, you must additionally set [`allowedTopologies`](https://kubernetes.io/docs/concepts/storage/storage-classes/#allowed-topologies) to restrict the topology of provisioned volumes to specific zones where Storage Pools exist as specified in the `storage-pools` StorageClass parameter. 
+
+For example, looking at the example in [Create a StorageClass](https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/hyperdisk-storage-pools#create-storageclass), assuming you already created a Storage Pool in `us-east4-c` according to [Create a Hyperdisk Storage Pool](https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/hyperdisk-storage-pools#create-sp), your `StorageClass` would need to specify `allowedTopologies` to restrict the topology of provisioned volumes to us-east4-c, where the Storage Pool exists.
+
+    ```yaml
+    apiVersion: storage.k8s.io/v1
+    kind: StorageClass
+    metadata:
+        name: storage-pools-sc
+    provisioner: pd.csi.storage.gke.io
+    volumeBindingMode: WaitForFirstConsumer
+    allowVolumeExpansion: true
+    parameters:
+        type: hyperdisk-balanced
+        provisioned-throughput-on-create: "140Mi"
+        provisioned-iops-on-create: "3000"
+        storage-pools: projects/my-project/zones/us-east4-c/storagePools/pool-us-east4-c
+    allowedTopologies:
+    - matchLabelExpressions:
+        - key: topology.gke.io/zone
+            values:
+            - us-east4-c
+    ```