Repository: kubernetes-sigs/gcp-compute-persistent-disk-csi-driver
Pull request #1563 (2024-01-12 19:35:30)

Add command line flag for enabling Storage Pools feature
**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**:
Adds command line flag for enabling creating volumes in storage pools. This will allow us to control the rollout of the storage pools feature until we are ready to launch to customers.

**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
NONE
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/pull/1563#" title="Author self-approved">amacaskill</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fgcp-compute-persistent-disk-csi-driver).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/blob/master/OWNERS)~~ [amacaskill]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: @amacaskill: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-gcp-compute-persistent-disk-csi-driver-unit | 8abbe1830d544338f1c69e47bc8c44c6d7498803 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_gcp-compute-persistent-disk-csi-driver/1563/pull-gcp-compute-persistent-disk-csi-driver-unit/1745892286836772864) | true | `/test pull-gcp-compute-persistent-disk-csi-driver-unit`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=gcp-compute-persistent-disk-csi-driver&pr=1563). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Aamacaskill). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go
index 2f1e36a5..dcf94f41 100644
--- a/cmd/gce-pd-csi-driver/main.go
+++ b/cmd/gce-pd-csi-driver/main.go
@@ -70,6 +70,8 @@ var (
 
 	fallbackRequisiteZonesFlag = flag.String("fallback-requisite-zones", "", "Comma separated list of requisite zones that will be used if there are not sufficient zones present in requisite topologies when provisioning a disk")
 
+	enableStoragePoolsFlag = flag.Bool("enable-storage-pools", false, "If set to true, the CSI Driver will allow volumes to be provisioned in Storage Pools")
+
 	version string
 )
 
@@ -160,7 +162,7 @@ func handle() {
 		}
 		initialBackoffDuration := time.Duration(*errorBackoffInitialDurationMs) * time.Millisecond
 		maxBackoffDuration := time.Duration(*errorBackoffMaxDurationMs) * time.Millisecond
-		controllerServer = driver.NewControllerServer(gceDriver, cloudProvider, initialBackoffDuration, maxBackoffDuration, fallbackRequisiteZones)
+		controllerServer = driver.NewControllerServer(gceDriver, cloudProvider, initialBackoffDuration, maxBackoffDuration, fallbackRequisiteZones, *enableStoragePoolsFlag)
 	} else if *cloudConfigFilePath != "" {
 		klog.Warningf("controller service is disabled but cloud config given - it has no effect")
 	}
diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go
index e70639eb..9c2085b0 100644
--- a/pkg/common/parameters.go
+++ b/pkg/common/parameters.go
@@ -120,7 +120,7 @@ type StoragePool struct {
 // put them into a well defined struct making sure to default unspecified fields.
 // extraVolumeLabels are added as labels; if there are also labels specified in
 // parameters, any matching extraVolumeLabels will be overridden.
-func ExtractAndDefaultParameters(parameters map[string]string, driverName string, extraVolumeLabels map[string]string) (DiskParameters, error) {
+func ExtractAndDefaultParameters(parameters map[string]string, driverName string, extraVolumeLabels map[string]string, enableStoragePools bool) (DiskParameters, error) {
 	p := DiskParameters{
 		DiskType:             "pd-standard",           // Default
 		ReplicationType:      replicationTypeNone,     // Default
@@ -200,6 +200,9 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
 
 			p.EnableConfidentialCompute = paramEnableConfidentialCompute
 		case ParameterKeyStoragePools:
+			if !enableStoragePools {
+				return p, fmt.Errorf("parameters contains invalid option %q", ParameterKeyStoragePools)
+			}
 			storagePools, err := ParseStoragePools(v)
 			if err != nil {
 				return p, fmt.Errorf("parameters contain invalid value for %s parameter: %w", ParameterKeyStoragePools, err)
diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go
index 5749be43..99ded3a4 100644
--- a/pkg/gce-cloud-provider/compute/gce-compute.go
+++ b/pkg/gce-cloud-provider/compute/gce-compute.go
@@ -560,9 +560,7 @@ func (cloud *CloudProvider) insertRegionalDisk(
 		if IsGCEError(err, "alreadyExists") {
 			disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
 			if err != nil {
-				// failed to GetDisk, however the Disk may already exist
-				// the error code should be non-Final
-				return status.Error(codes.Unavailable, err.Error())
+				return err
 			}
 			err = cloud.ValidateExistingDisk(ctx, disk, params,
 				int64(capacityRange.GetRequiredBytes()),
@@ -574,19 +572,16 @@ func (cloud *CloudProvider) insertRegionalDisk(
 			klog.Warningf("GCE PD %s already exists, reusing", volKey.Name)
 			return nil
 		}
-		// if the error code is considered "final", RegionDisks.Insert might not be retried
-		return fmt.Errorf("unknown Insert Regional disk error: %w", err)
+		return status.Error(codes.Internal, fmt.Sprintf("unknown Insert disk error: %v", err.Error()))
 	}
 	klog.V(5).Infof("InsertDisk operation %s for disk %s", opName, diskToCreate.Name)
 
 	err = cloud.waitForRegionalOp(ctx, project, opName, volKey.Region)
-	// failed to wait for Op to finish, however, the Op possibly is still running as expected
-	// the error code returned should be non-final
 	if err != nil {
 		if IsGCEError(err, "alreadyExists") {
 			disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
 			if err != nil {
-				return status.Errorf(codes.Unavailable, "error when getting disk: %v", err.Error())
+				return err
 			}
 			err = cloud.ValidateExistingDisk(ctx, disk, params,
 				int64(capacityRange.GetRequiredBytes()),
@@ -598,7 +593,7 @@ func (cloud *CloudProvider) insertRegionalDisk(
 			klog.Warningf("GCE PD %s already exists after wait, reusing", volKey.Name)
 			return nil
 		}
-		return status.Errorf(codes.Unavailable, "unknown error when polling the operation: %v", err.Error())
+		return fmt.Errorf("unknown Insert disk operation error: %w", err)
 	}
 	return nil
 }
@@ -676,9 +671,9 @@ func (cloud *CloudProvider) insertZonalDisk(
 		var insertOp *computealpha.Operation
 		var storagePool *common.StoragePool
 		if storagePoolsEnabled {
-			storagePool = common.StoragePoolInZone(params.StoragePools, diskToCreate.Zone)
+			storagePool = common.StoragePoolInZone(params.StoragePools, volKey.Zone)
 			if storagePool == nil {
-				return status.Errorf(codes.InvalidArgument, "cannot create disk in zone %q: no Storage Pools exist in zone", diskToCreate.Zone)
+				return status.Errorf(codes.InvalidArgument, "cannot create disk in zone %q: no Storage Pools exist in zone", volKey.Zone)
 			}
 		}
 		alphaDiskToCreate := convertV1DiskToAlphaDisk(diskToCreate, params.ProvisionedThroughputOnCreate, storagePool)
@@ -700,9 +695,7 @@ func (cloud *CloudProvider) insertZonalDisk(
 		if IsGCEError(err, "alreadyExists") {
 			disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
 			if err != nil {
-				// failed to GetDisk, however the Disk may already exist
-				// the error code should be non-Final
-				return status.Error(codes.Unavailable, err.Error())
+				return err
 			}
 			err = cloud.ValidateExistingDisk(ctx, disk, params,
 				int64(capacityRange.GetRequiredBytes()),
@@ -714,7 +707,6 @@ func (cloud *CloudProvider) insertZonalDisk(
 			klog.Warningf("GCE PD %s already exists, reusing", volKey.Name)
 			return nil
 		}
-		// if the error code is considered "final", Disks.Insert might not be retried
 		return fmt.Errorf("unknown Insert disk error: %w", err)
 	}
 	klog.V(5).Infof("InsertDisk operation %s for disk %s", opName, diskToCreate.Name)
@@ -722,12 +714,10 @@ func (cloud *CloudProvider) insertZonalDisk(
 	err = cloud.waitForZonalOp(ctx, project, opName, volKey.Zone)
 
 	if err != nil {
-		// failed to wait for Op to finish, however, the Op possibly is still running as expected
-		// the error code returned should be non-final
 		if IsGCEError(err, "alreadyExists") {
 			disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
 			if err != nil {
-				return status.Errorf(codes.Unavailable, "error when getting disk: %v", err.Error())
+				return err
 			}
 			err = cloud.ValidateExistingDisk(ctx, disk, params,
 				int64(capacityRange.GetRequiredBytes()),
@@ -739,7 +729,7 @@ func (cloud *CloudProvider) insertZonalDisk(
 			klog.Warningf("GCE PD %s already exists after wait, reusing", volKey.Name)
 			return nil
 		}
-		return status.Errorf(codes.Unavailable, "unknown error when polling the operation: %v", err.Error())
+		return fmt.Errorf("unknown Insert disk operation error: %w", err)
 	}
 	return nil
 }
diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go
index 93306c33..44ffd399 100644
--- a/pkg/gce-pd-csi-driver/controller.go
+++ b/pkg/gce-pd-csi-driver/controller.go
@@ -100,6 +100,9 @@ type GCEControllerServer struct {
 	// provisioning in GKE Autopilot, where a GKE cluster to
 	// be scaled down to 1 zone.
 	fallbackRequisiteZones []string
+
+	// If set to true, the CSI Driver will allow volumes to be provisioned in Storage Pools.
+	enableStoragePools bool
 }
 
 type csiErrorBackoffId string
@@ -251,7 +254,7 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 
 	// Apply Parameters (case-insensitive). We leave validation of
 	// the values to the cloud provider.
-	params, err := common.ExtractAndDefaultParameters(req.GetParameters(), gceCS.Driver.name, gceCS.Driver.extraVolumeLabels)
+	params, err := common.ExtractAndDefaultParameters(req.GetParameters(), gceCS.Driver.name, gceCS.Driver.extraVolumeLabels, gceCS.enableStoragePools)
 	diskTypeForMetric = params.DiskType
 	enableConfidentialCompute = strconv.FormatBool(params.EnableConfidentialCompute)
 	hasStoragePools := len(params.StoragePools) > 0
@@ -324,8 +327,7 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 	existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, gceCS.CloudProvider.GetDefaultProject(), volKey, gceAPIVersion)
 	if err != nil {
 		if !gce.IsGCEError(err, "notFound") {
-			// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
-			return nil, common.LoggedError("CreateVolume, failed to getDisk when validating: ", status.Error(codes.Unavailable, err.Error()))
+			return nil, common.LoggedError("CreateVolume, failed to getDisk when validating: ", err)
 		}
 	}
 	if err == nil {
@@ -340,10 +342,10 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 
 		ready, err := isDiskReady(existingDisk)
 		if err != nil {
-			return nil, status.Errorf(codes.Aborted, "CreateVolume disk %q had error checking ready status: %v", volKey.String(), err.Error())
+			return nil, common.LoggedError("CreateVolume disk "+volKey.String()+" had error checking ready status: ", err)
 		}
 		if !ready {
-			return nil, status.Errorf(codes.Aborted, "CreateVolume existing disk %v is not ready", volKey)
+			return nil, status.Errorf(codes.Internal, "CreateVolume existing disk %v is not ready", volKey)
 		}
 
 		// If there is no validation error, immediately return success
@@ -418,10 +420,10 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
 			// Verify the source disk is ready.
 			ready, err := isDiskReady(diskFromSourceVolume)
 			if err != nil {
-				return nil, status.Errorf(codes.Aborted, "CreateVolume disk from source volume %q had error checking ready status: %v", sourceVolKey.String(), err.Error())
+				return nil, common.LoggedError("CreateVolume disk from source volume "+sourceVolKey.String()+"  had error checking ready status: ", err)
 			}
 			if !ready {
-				return nil, status.Errorf(codes.Aborted, "CreateVolume disk from source volume %v is not ready", sourceVolKey)
+				return nil, status.Errorf(codes.Internal, "CreateVolume disk from source volume %v is not ready", sourceVolKey)
 			}
 		}
 	} else { // if VolumeContentSource is nil, validate access mode is not read only
@@ -835,7 +837,7 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
 	}
 
 	// Validate the disk parameters match the disk we GET
-	params, err := common.ExtractAndDefaultParameters(req.GetParameters(), gceCS.Driver.name, gceCS.Driver.extraVolumeLabels)
+	params, err := common.ExtractAndDefaultParameters(req.GetParameters(), gceCS.Driver.name, gceCS.Driver.extraVolumeLabels, gceCS.enableStoragePools)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "failed to extract parameters: %v", err.Error())
 	}
@@ -1877,10 +1879,9 @@ func createRegionalDisk(ctx context.Context, cloudProvider gce.GCECompute, name
 	if multiWriter {
 		gceAPIVersion = gce.GCEAPIVersionBeta
 	}
-	// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
 	disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region), gceAPIVersion)
 	if err != nil {
-		return nil, status.Errorf(codes.Unavailable, "failed to get disk after creating regional disk: %v", err.Error())
+		return nil, fmt.Errorf("failed to get disk after creating regional disk: %w", err)
 	}
 	return disk, nil
 }
@@ -1900,10 +1901,9 @@ func createSingleZoneDisk(ctx context.Context, cloudProvider gce.GCECompute, nam
 	if multiWriter {
 		gceAPIVersion = gce.GCEAPIVersionBeta
 	}
-	// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
 	disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone), gceAPIVersion)
 	if err != nil {
-		return nil, status.Errorf(codes.Unavailable, "failed to get disk after creating zonal disk: %v", err.Error())
+		return nil, err
 	}
 	return disk, nil
 }
diff --git a/pkg/gce-pd-csi-driver/gce-pd-driver.go b/pkg/gce-pd-csi-driver/gce-pd-driver.go
index 8d5156e5..07db696d 100644
--- a/pkg/gce-pd-csi-driver/gce-pd-driver.go
+++ b/pkg/gce-pd-csi-driver/gce-pd-driver.go
@@ -152,7 +152,7 @@ func NewNodeServer(gceDriver *GCEDriver, mounter *mount.SafeFormatAndMount, devi
 	}
 }
 
-func NewControllerServer(gceDriver *GCEDriver, cloudProvider gce.GCECompute, errorBackoffInitialDuration, errorBackoffMaxDuration time.Duration, fallbackRequisiteZones []string) *GCEControllerServer {
+func NewControllerServer(gceDriver *GCEDriver, cloudProvider gce.GCECompute, errorBackoffInitialDuration, errorBackoffMaxDuration time.Duration, fallbackRequisiteZones []string, enableStoragePools bool) *GCEControllerServer {
 	return &GCEControllerServer{
 		Driver:                 gceDriver,
 		CloudProvider:          cloudProvider,
@@ -160,6 +160,7 @@ func NewControllerServer(gceDriver *GCEDriver, cloudProvider gce.GCECompute, err
 		volumeLocks:            common.NewVolumeLocks(),
 		errorBackoff:           newCsiErrorBackoff(errorBackoffInitialDuration, errorBackoffMaxDuration),
 		fallbackRequisiteZones: fallbackRequisiteZones,
+		enableStoragePools:     enableStoragePools,
 	}
 }
 


Test patch:
diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go
index 98f3ff39..24567403 100644
--- a/pkg/common/parameters_test.go
+++ b/pkg/common/parameters_test.go
@@ -25,11 +25,12 @@ import (
 
 func TestExtractAndDefaultParameters(t *testing.T) {
 	tests := []struct {
-		name         string
-		parameters   map[string]string
-		labels       map[string]string
-		expectParams DiskParameters
-		expectErr    bool
+		name               string
+		parameters         map[string]string
+		labels             map[string]string
+		enableStoragePools bool
+		expectParams       DiskParameters
+		expectErr          bool
 	}{
 		{
 			name:       "defaults",
@@ -202,9 +203,10 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 			},
 		},
 		{
-			name:       "storage pool parameters",
-			parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1,projects/my-project/zones/us-central1-b/storagePools/storagePool-2"},
-			labels:     map[string]string{},
+			name:               "storage pool parameters",
+			enableStoragePools: true,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1,projects/my-project/zones/us-central1-b/storagePools/storagePool-2"},
+			labels:             map[string]string{},
 			expectParams: DiskParameters{
 				DiskType:        "hyperdisk-balanced",
 				ReplicationType: "none",
@@ -227,46 +229,59 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 			},
 		},
 		{
-			name:       "invalid storage pool parameters, starts with /projects instead of projects",
-			parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "/projects/my-project/zones/us-central1-a/storagePools/storagePool-1"},
-			labels:     map[string]string{},
-			expectErr:  true,
+			name:               "invalid storage pool parameters, starts with /projects instead of projects",
+			enableStoragePools: true,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "/projects/my-project/zones/us-central1-a/storagePools/storagePool-1"},
+			labels:             map[string]string{},
+			expectErr:          true,
 		},
 		{
-			name:       "invalid storage pool parameters, missing projects",
-			parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "zones/us-central1-a/storagePools/storagePool-1"},
-			labels:     map[string]string{},
-			expectErr:  true,
+			name:               "invalid storage pool parameters, missing projects",
+			enableStoragePools: true,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "zones/us-central1-a/storagePools/storagePool-1"},
+			labels:             map[string]string{},
+			expectErr:          true,
 		},
 		{
-			name:       "invalid storage pool parameters, missing zones",
-			parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/storagePools/storagePool-1"},
-			labels:     map[string]string{},
-			expectErr:  true,
+			name:               "invalid storage pool parameters, missing zones",
+			enableStoragePools: true,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/storagePools/storagePool-1"},
+			labels:             map[string]string{},
+			expectErr:          true,
 		},
 		{
-			name:       "invalid storage pool parameters, duplicate projects",
-			parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/projects/my-project/storagePools/storagePool-1"},
-			labels:     map[string]string{},
-			expectErr:  true,
+			name:               "invalid storage pool parameters, duplicate projects",
+			enableStoragePools: true,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/projects/my-project/storagePools/storagePool-1"},
+			labels:             map[string]string{},
+			expectErr:          true,
 		},
 		{
-			name:       "invalid storage pool parameters, duplicate zones",
-			parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "zones/us-central1-a/zones/us-central1-a/storagePools/storagePool-1"},
-			labels:     map[string]string{},
-			expectErr:  true,
+			name:               "invalid storage pool parameters, duplicate zones",
+			enableStoragePools: true,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "zones/us-central1-a/zones/us-central1-a/storagePools/storagePool-1"},
+			labels:             map[string]string{},
+			expectErr:          true,
 		},
 		{
-			name:       "invalid storage pool parameters, duplicate storagePools",
-			parameters: map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/storagePools/us-central1-a/storagePools/storagePool-1"},
-			labels:     map[string]string{},
-			expectErr:  true,
+			name:               "invalid storage pool parameters, duplicate storagePools",
+			enableStoragePools: true,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/storagePools/us-central1-a/storagePools/storagePool-1"},
+			labels:             map[string]string{},
+			expectErr:          true,
+		},
+		{
+			name:               "storage pool parameters, enableStoragePools is false",
+			enableStoragePools: false,
+			parameters:         map[string]string{ParameterKeyType: "hyperdisk-balanced", ParameterKeyStoragePools: "projects/my-project/zones/us-central1-a/storagePools/storagePool-1,projects/my-project/zones/us-central1-b/storagePools/storagePool-2"},
+			labels:             map[string]string{},
+			expectErr:          true,
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			p, err := ExtractAndDefaultParameters(tc.parameters, "testDriver", tc.labels)
+			p, err := ExtractAndDefaultParameters(tc.parameters, "testDriver", tc.labels, tc.enableStoragePools)
 			if gotErr := err != nil; gotErr != tc.expectErr {
 				t.Fatalf("ExtractAndDefaultParameters(%+v) = %v; expectedErr: %v", tc.parameters, err, tc.expectErr)
 			}
diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go
index 18347f46..c9096990 100644
--- a/pkg/gce-pd-csi-driver/controller_test.go
+++ b/pkg/gce-pd-csi-driver/controller_test.go
@@ -444,10 +444,11 @@ func TestListSnapshotsArguments(t *testing.T) {
 
 func TestCreateVolumeArguments(t *testing.T) {
 	testCases := []struct {
-		name       string
-		req        *csi.CreateVolumeRequest
-		expVol     *csi.Volume
-		expErrCode codes.Code
+		name               string
+		req                *csi.CreateVolumeRequest
+		enableStoragePools bool
+		expVol             *csi.Volume
+		expErrCode         codes.Code
 	}{
 		{
 			name: "success default",
@@ -898,6 +899,7 @@ func TestCreateVolumeArguments(t *testing.T) {
 					},
 				},
 			},
+			enableStoragePools: true,
 			expVol: &csi.Volume{
 				CapacityBytes: common.GbToBytes(20),
 				VolumeId:      "projects/test-project/zones/us-central1-a/disks/test-name",
@@ -909,6 +911,29 @@ func TestCreateVolumeArguments(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "fail with storage pools parameter, enableStoragePools is false",
+			req: &csi.CreateVolumeRequest{
+				Name:               name,
+				CapacityRange:      stdCapRange,
+				VolumeCapabilities: stdVolCaps,
+				Parameters:         map[string]string{"storage-pools": "projects/test-project/zones/us-central1-a/storagePools/storagePool-1", "type": "hyperdisk-balanced"},
+				AccessibilityRequirements: &csi.TopologyRequirement{
+					Requisite: []*csi.Topology{
+						{
+							Segments: map[string]string{common.TopologyKeyZone: "us-central1-a"},
+						},
+					},
+					Preferred: []*csi.Topology{
+						{
+							Segments: map[string]string{common.TopologyKeyZone: "us-central1-a"},
+						},
+					},
+				},
+			},
+			enableStoragePools: false,
+			expErrCode:         codes.InvalidArgument,
+		},
 		{
 			name: "fail with invalid storage pools parameter",
 			req: &csi.CreateVolumeRequest{
@@ -929,7 +954,8 @@ func TestCreateVolumeArguments(t *testing.T) {
 					},
 				},
 			},
-			expErrCode: codes.InvalidArgument,
+			enableStoragePools: true,
+			expErrCode:         codes.InvalidArgument,
 		},
 	}
 
@@ -938,7 +964,7 @@ func TestCreateVolumeArguments(t *testing.T) {
 		t.Logf("test case: %s", tc.name)
 		// Setup new driver each time so no interference
 		gceDriver := initGCEDriver(t, nil)
-
+		gceDriver.cs.enableStoragePools = tc.enableStoragePools
 		// Start Test
 		resp, err := gceDriver.cs.CreateVolume(context.Background(), tc.req)
 		if err != nil {
@@ -1374,6 +1400,7 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
 		requestCapacityRange *csi.CapacityRange
 		sourceTopology       *csi.TopologyRequirement
 		requestTopology      *csi.TopologyRequirement
+		enableStoragePools   bool
 		expCloneKey          *meta.Key
 		// Accessible topologies validates that the replica zones are valid for regional disk clones.
 		expAccessibleTop []*csi.Topology
@@ -1461,6 +1488,7 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
 			sourceVolumeID:       testZonalVolumeSourceID,
 			requestCapacityRange: stdCapRange,
 			sourceCapacityRange:  stdCapRange,
+			enableStoragePools:   true,
 			reqParameters: map[string]string{
 				common.ParameterKeyType:                 "test-type",
 				common.ParameterKeyReplicationType:      replicationTypeNone,
@@ -1880,6 +1908,7 @@ func TestCreateVolumeWithVolumeSourceFromVolume(t *testing.T) {
 	for _, tc := range testCases {
 		t.Logf("test case: %s", tc.name)
 		gceDriver := initGCEDriver(t, nil)
+		gceDriver.cs.enableStoragePools = tc.enableStoragePools
 
 		req := &csi.CreateVolumeRequest{
 			Name:               testCloneVolumeName,
diff --git a/pkg/gce-pd-csi-driver/gce-pd-driver_test.go b/pkg/gce-pd-csi-driver/gce-pd-driver_test.go
index 32613b5f..881e5ae8 100644
--- a/pkg/gce-pd-csi-driver/gce-pd-driver_test.go
+++ b/pkg/gce-pd-csi-driver/gce-pd-driver_test.go
@@ -47,8 +47,9 @@ func initGCEDriverWithCloudProvider(t *testing.T, cloudProvider gce.GCECompute)
 	errorBackoffInitialDuration := 200 * time.Millisecond
 	errorBackoffMaxDuration := 5 * time.Minute
 	fallbackRequisiteZones := []string{}
+	enableStoragePools := false
 
-	controllerServer := NewControllerServer(gceDriver, cloudProvider, errorBackoffInitialDuration, errorBackoffMaxDuration, fallbackRequisiteZones)
+	controllerServer := NewControllerServer(gceDriver, cloudProvider, errorBackoffInitialDuration, errorBackoffMaxDuration, fallbackRequisiteZones, enableStoragePools)
 	err := gceDriver.SetupGCEDriver(driver, vendorVersion, nil, nil, controllerServer, nil)
 	if err != nil {
 		t.Fatalf("Failed to setup GCE Driver: %v", err)
diff --git a/test/sanity/sanity_test.go b/test/sanity/sanity_test.go
index e0d742db..b0d88315 100644
--- a/test/sanity/sanity_test.go
+++ b/test/sanity/sanity_test.go
@@ -62,13 +62,14 @@ func TestSanity(t *testing.T) {
 	}
 
 	fallbackRequisiteZones := []string{}
+	enableStoragePools := false
 
 	mounter := mountmanager.NewFakeSafeMounter()
 	deviceUtils := deviceutils.NewFakeDeviceUtils(true)
 
 	//Initialize GCE Driver
 	identityServer := driver.NewIdentityServer(gceDriver)
-	controllerServer := driver.NewControllerServer(gceDriver, cloudProvider, 0, 5*time.Minute, fallbackRequisiteZones)
+	controllerServer := driver.NewControllerServer(gceDriver, cloudProvider, 0, 5*time.Minute, fallbackRequisiteZones, enableStoragePools)
 	nodeServer := driver.NewNodeServer(gceDriver, mounter, deviceUtils, metadataservice.NewFakeService(), mountmanager.NewFakeStatter(mounter))
 	err = gceDriver.SetupGCEDriver(driverName, vendorVersion, extraLabels, identityServer, controllerServer, nodeServer)
 	if err != nil {