Merge pull request #1718 from hungnguyen243/master

Improve error messages for ControllerExpandVolume / CreateSnapshot of multi-zone PV.

Author: k8s-ci-robot

pkg/gce-pd-csi-driver/controller.go

@@ -1160,6 +1160,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)
 	}
@@ -1205,6 +1210,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)
@@ -1484,6 +1490,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
@@ -1506,12 +1513,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)

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