fix4

kubernetes-sigs · Nov 11, 2022 · a1f9d19 · a1f9d19
1 parent acfe73c
commit a1f9d19
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 84 deletions.
diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
@@ -147,7 +147,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 		case *csi.VolumeContentSource_Volume:
 			diskDetails, _ := d.cloud.GetDiskByNamePrefix("clone-" + volName)
 			if diskDetails != nil {
-				err := verifyVolumeDetails(opts, diskDetails, false)
+				err := verifyVolumeDetails(opts, diskDetails)
 				if err != nil {
 					return nil, err
 				}
@@ -159,28 +159,18 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 				if err != nil {
 					return nil, status.Errorf(codes.Internal, "Could not get the source volume %q: %v", srcVolumeID, err)
 				}
-				err = verifyVolumeDetails(opts, diskDetails, true)
+				if util.GiBToBytes(diskDetails.CapacityGiB) != volSizeBytes {
+					return nil, status.Errorf(codes.Internal, "Cannot clone volume %v, source volume size is not equal to the clone volume", srcVolumeID)
+				}
+				err = verifyVolumeDetails(opts, diskDetails)
 				if err != nil {
 					return nil, err
 				}
 				diskFromSourceVolume, err := d.cloud.CloneDisk(srcVolumeID, volName)
 				if err != nil {
 					return nil, status.Errorf(codes.Internal, "Could not create volume %q: %v", volName, err)
 				}
-				if util.GiBToBytes(diskDetails.CapacityGiB) < volSizeBytes {
-					capRange := req.GetCapacityRange()
-					if capRange == nil {
-						return nil, status.Error(codes.InvalidArgument, "Capacity range not provided")
-					}
-					maxVolSize := capRange.GetLimitBytes()
-					if maxVolSize > 0 && maxVolSize < volSizeBytes {
-						return nil, status.Error(codes.InvalidArgument, "After round-up, volume size exceeds the limit specified")
-					}
-					_, err := d.cloud.ResizeDisk(diskFromSourceVolume.VolumeID, volSizeBytes)
-					if err != nil {
-						return nil, status.Errorf(codes.Internal, "Could not resize volume %q: %v", diskFromSourceVolume.VolumeID, err)
-					}
-				}
+
 				cloneDiskDetails, err := d.cloud.GetDiskByID(diskFromSourceVolume.VolumeID)
 				if err != nil {
 					return nil, status.Errorf(codes.Internal, "Could not create volume %q: %v", volName, err)
@@ -197,7 +187,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 	diskDetails, _ := d.cloud.GetDiskByName(volName)
 	if diskDetails != nil {
 		// wait for volume to be available as the volume already exists
-		err := verifyVolumeDetails(opts, diskDetails, false)
+		err := verifyVolumeDetails(opts, diskDetails)
 		if err != nil {
 			return nil, err
 		}
@@ -513,17 +503,15 @@ func getVolSizeBytes(req *csi.CreateVolumeRequest) (int64, error) {
 	return volSizeBytes, nil
 }
 
-func verifyVolumeDetails(payload *cloud.DiskOptions, diskDetails *cloud.Disk, isClone bool) error {
+func verifyVolumeDetails(payload *cloud.DiskOptions, diskDetails *cloud.Disk) error {
 	if payload.Shareable != diskDetails.Shareable {
 		return status.Errorf(codes.Internal, "shareable in payload and shareable in disk details don't match")
 	}
 	if payload.VolumeType != diskDetails.DiskType {
 		return status.Errorf(codes.Internal, "TYPE in payload and disktype in disk details don't match")
 	}
 	capacityGIB := util.BytesToGiB(payload.CapacityBytes)
-	if isClone && capacityGIB < diskDetails.CapacityGiB {
-		return status.Errorf(codes.Internal, "capacityBytes in payload(clone disk) is less than capacityGIB in the src disk details")
-	} else if !isClone && capacityGIB != diskDetails.CapacityGiB {
+	if capacityGIB != diskDetails.CapacityGiB {
 		return status.Errorf(codes.Internal, "capacityBytes in payload and capacityGIB in disk details don't match")
 	}
 	return nil

diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go
@@ -45,7 +45,6 @@ func TestCreateVolume(t *testing.T) {
 	}
 
 	stdVolSize := int64(5 * 1024 * 1024 * 1024)
-	resizeVolSize := int64(10 * 1024 * 1024 * 1024)
 	stdCapRange := &csi.CapacityRange{RequiredBytes: stdVolSize}
 	stdParams := map[string]string{
 		"type": cloud.DefaultVolumeType,
@@ -151,67 +150,7 @@ func TestCreateVolume(t *testing.T) {
 			},
 		},
 		{
-			name: "Create PVC with Data source capacity Smaller than New Volume",
-			testFunc: func(t *testing.T) {
-				req := &csi.CreateVolumeRequest{
-					Name:               "clone-volume-name",
-					CapacityRange:      &csi.CapacityRange{RequiredBytes: resizeVolSize},
-					VolumeCapabilities: stdVolCap,
-					Parameters:         stdParams,
-					VolumeContentSource: &csi.VolumeContentSource{
-						Type: &csi.VolumeContentSource_Volume{
-							Volume: &csi.VolumeContentSource_VolumeSource{
-								VolumeId: "test-volume-src-100",
-							},
-						},
-					},
-				}
-
-				ctx := context.Background()
-
-				mockDisk := &cloud.Disk{
-					VolumeID:    req.Name,
-					CapacityGiB: util.BytesToGiB(stdVolSize),
-					DiskType:    cloud.DefaultVolumeType,
-				}
-				mockSrcDisk := &cloud.Disk{
-					VolumeID:    "test-volume-src-100",
-					CapacityGiB: util.BytesToGiB(stdVolSize),
-					DiskType:    cloud.DefaultVolumeType,
-				}
-				resizedDisk := &cloud.Disk{
-					VolumeID:    req.Name,
-					CapacityGiB: util.BytesToGiB(resizeVolSize),
-					DiskType:    cloud.DefaultVolumeType,
-				}
-
-				mockCtl := gomock.NewController(t)
-				defer mockCtl.Finish()
-
-				mockCloud := mocks.NewMockCloud(mockCtl)
-				mockCloud.EXPECT().GetDiskByNamePrefix(gomock.Eq("clone-"+req.Name)).Return(nil, nil)
-				mockCloud.EXPECT().GetDiskByID(gomock.Eq(mockSrcDisk.VolumeID)).Return(mockSrcDisk, nil)
-				mockCloud.EXPECT().CloneDisk(gomock.Eq(mockSrcDisk.VolumeID), gomock.Eq(req.Name)).Return(mockDisk, nil)
-				mockCloud.EXPECT().ResizeDisk(gomock.Eq(mockDisk.VolumeID), resizeVolSize).Return(resizeVolSize, nil)
-				mockCloud.EXPECT().GetDiskByID(gomock.Eq(mockDisk.VolumeID)).Return(resizedDisk, nil)
-
-				powervsDriver := controllerService{
-					cloud:         mockCloud,
-					driverOptions: &Options{},
-					volumeLocks:   util.NewVolumeLocks(),
-				}
-
-				if _, err := powervsDriver.CreateVolume(ctx, req); err != nil {
-					srvErr, ok := status.FromError(err)
-					if !ok {
-						t.Fatalf("Could not get error status code from error: %v", srvErr)
-					}
-					t.Fatalf("Unexpected error: %v", srvErr.Code())
-				}
-			},
-		},
-		{
-			name: "Create PVC with Data source already exists",
+			name: "Create PVC with Data source - volume already exists",
 			testFunc: func(t *testing.T) {
 				req := &csi.CreateVolumeRequest{
 					Name:               "clone-volume-name",
@@ -240,7 +179,6 @@ func TestCreateVolume(t *testing.T) {
 
 				mockCloud := mocks.NewMockCloud(mockCtl)
 				mockCloud.EXPECT().GetDiskByNamePrefix(gomock.Eq("clone-"+req.Name)).Return(mockDisk, nil)
-				mockCloud.EXPECT().WaitForVolumeState(gomock.Eq(req.Name), gomock.Any()).Return(nil)
 
 				powervsDriver := controllerService{
 					cloud:         mockCloud,