Repository: kubernetes-sigs/gcp-compute-persistent-disk-csi-driver
Pull request #1718 (2024-05-21 20:53:13)

Improve error messages for ControllerExpandVolume / CreateSnapshot of multi-zone PV.
#### What type of PR is this?
/kind cleanup
#### What this PR does / why we need it:
Improve error messages for ControllerExpandVolume / CreateSnapshot of multi-zone PV.

Before:
CreateSnapshot: "Failed to create snapshot: failed to take snapshot of the volume projects/psch-gke-dev/zones/multi-zone/disks/hdml-llama2-70b-hf: "rpc error: code = Unknown desc = CreateSnapshot, failed 
to getDisk: googleapi: Error 400: Invalid value for field 'zone': 'multi-zone'. Unknown zone., invalid""
ControllerExpandVolume: "resize volume "my-disk-pv" by resizer "pd.csi.storage.gke.io" failed: rpc error: code = Unknown desc = ControllerExpandVolume failed to resize disk: failed to get disk: googleapi: Error 400: In
valid value for field 'zone': 'multi-zone'. Unknown zone., invalid"

After:
CreateSnapshot: "Snapshots are not supported with the `multi-zone` PV volumeHandle feature"
ControllerExpandVolume: "Resize operation is not supported with the `multi-zone` PVC volumeHandle feature. Please re-create the disk from source if you want a larger size."
#### Which issue(s) this PR fixes:
Fixes #

#### Special notes for your reviewer:

#### Does this PR introduce a user-facing change?
```release-note
Update logging on multi-zone feature support for volume snapshot and resize
```

#### Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:



Patch:
diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go
index 1bcfdf25..aabde35c 100644
--- a/pkg/gce-pd-csi-driver/controller.go
+++ b/pkg/gce-pd-csi-driver/controller.go
@@ -1158,6 +1158,11 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
 		return nil, status.Errorf(codes.InvalidArgument, "CreateSnapshot Volume ID is invalid: %v", err.Error())
 	}
 
+	volumeIsMultiZone := isMultiZoneVolKey(volKey)
+	if gceCS.multiZoneVolumeHandleConfig.Enable && volumeIsMultiZone {
+		return nil, status.Errorf(codes.InvalidArgument, "CreateSnapshot for volume %v failed. Snapshots are not supported with the multi-zone PV volumeHandle feature", volumeID)
+	}
+
 	if acquired := gceCS.volumeLocks.TryAcquire(volumeID); !acquired {
 		return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, volumeID)
 	}
@@ -1203,6 +1208,7 @@ func (gceCS *GCEControllerServer) createPDSnapshot(ctx context.Context, project
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "Invalid volume key: %v", volKey)
 	}
+
 	// Check if PD snapshot already exists
 	var snapshot *compute.Snapshot
 	snapshot, err = gceCS.CloudProvider.GetSnapshot(ctx, project, snapshotName)
@@ -1482,6 +1488,7 @@ func (gceCS *GCEControllerServer) ListSnapshots(ctx context.Context, req *csi.Li
 }
 
 func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) {
+
 	var err error
 	diskTypeForMetric := metrics.DefaultDiskTypeForMetric
 	enableConfidentialCompute := metrics.DefaultEnableConfidentialCompute
@@ -1504,12 +1511,19 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re
 		return nil, status.Errorf(codes.InvalidArgument, "ControllerExpandVolume Volume ID is invalid: %v", err.Error())
 	}
 	project, volKey, err = gceCS.CloudProvider.RepairUnderspecifiedVolumeKey(ctx, project, volKey)
+
 	if err != nil {
 		if gce.IsGCENotFoundError(err) {
 			return nil, status.Errorf(codes.NotFound, "ControllerExpandVolume could not find volume with ID %v: %v", volumeID, err.Error())
 		}
 		return nil, common.LoggedError("ControllerExpandVolume error repairing underspecified volume key: ", err)
 	}
+
+	volumeIsMultiZone := isMultiZoneVolKey(volKey)
+	if gceCS.multiZoneVolumeHandleConfig.Enable && volumeIsMultiZone {
+		return nil, status.Errorf(codes.InvalidArgument, "ControllerExpandVolume is not supported with the multi-zone PVC volumeHandle feature. Please re-create the volume %v from source if you want a larger size", volumeID)
+	}
+
 	sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
 	diskTypeForMetric, enableConfidentialCompute, enableStoragePools = metrics.GetMetricParameters(sourceDisk)
 	resizedGb, err := gceCS.CloudProvider.ResizeDisk(ctx, project, volKey, reqBytes)


Test patch:
diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go
index 17f9946a..bee2e1d2 100644
--- a/pkg/gce-pd-csi-driver/controller_test.go
+++ b/pkg/gce-pd-csi-driver/controller_test.go
@@ -255,6 +255,77 @@ func TestCreateSnapshotArguments(t *testing.T) {
 	}
 }
 
+func TestUnsupportedMultiZoneCreateSnapshot(t *testing.T) {
+	testCase := struct {
+		name       string
+		req        *csi.CreateSnapshotRequest
+		expErrCode codes.Code
+	}{
+		name: "failed create snapshot for multi-zone PV", // Example values
+		req: &csi.CreateSnapshotRequest{
+			Name:           name,
+			SourceVolumeId: multiZoneVolumeID,
+		},
+		expErrCode: codes.InvalidArgument,
+	}
+
+	t.Logf("test case: %s", testCase.name)
+
+	gceDriver := initGCEDriver(t, nil)
+	gceDriver.cs.multiZoneVolumeHandleConfig = MultiZoneVolumeHandleConfig{
+		Enable: true,
+	}
+
+	// Start Test
+	_, err := gceDriver.cs.CreateSnapshot(context.Background(), testCase.req)
+	if err != nil {
+		serverError, ok := status.FromError(err)
+		if !ok {
+			t.Fatalf("Could not get error status code from err: %v", serverError)
+		}
+		if serverError.Code() != testCase.expErrCode {
+			t.Fatalf("Expected error code: %v, got: %v. err : %v", testCase.expErrCode, serverError.Code(), err)
+		}
+	} else {
+		t.Fatalf("Expected error: %v, got no error", testCase.expErrCode)
+	}
+}
+
+func TestUnsupportedMultiZoneControllerExpandVolume(t *testing.T) {
+	testCase := struct {
+		name       string
+		req        *csi.ControllerExpandVolumeRequest
+		expErrCode codes.Code
+	}{
+		name: "failed create snapshot for multi-zone PV", // Example values
+		req: &csi.ControllerExpandVolumeRequest{
+			VolumeId: multiZoneVolumeID,
+		},
+		expErrCode: codes.InvalidArgument,
+	}
+
+	t.Logf("test case: %s", testCase.name)
+
+	gceDriver := initGCEDriver(t, nil)
+	gceDriver.cs.multiZoneVolumeHandleConfig = MultiZoneVolumeHandleConfig{
+		Enable: true,
+	}
+
+	// Start Test
+	_, err := gceDriver.cs.ControllerExpandVolume(context.Background(), testCase.req)
+	if err != nil {
+		serverError, ok := status.FromError(err)
+		if !ok {
+			t.Fatalf("Could not get error status code from err: %v", serverError)
+		}
+		if serverError.Code() != testCase.expErrCode {
+			t.Fatalf("Expected error code: %v, got: %v. err : %v", testCase.expErrCode, serverError.Code(), err)
+		}
+	} else {
+		t.Fatalf("Expected error: %v, got no error", testCase.expErrCode)
+	}
+}
+
 func TestDeleteSnapshot(t *testing.T) {
 	testCases := []struct {
 		name       string