From a1f9d19a0ac128873c03f5c4089b8872c02360b8 Mon Sep 17 00:00:00 2001 From: Madhan-SWE Date: Fri, 11 Nov 2022 11:58:05 +0530 Subject: [PATCH] fix4 --- pkg/driver/controller.go | 30 +++++----------- pkg/driver/controller_test.go | 64 +---------------------------------- 2 files changed, 10 insertions(+), 84 deletions(-) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 2a08645cc..0fa6273da 100644 --- 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,7 +159,10 @@ 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 } @@ -167,20 +170,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol 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,7 +503,7 @@ 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") } @@ -521,9 +511,7 @@ func verifyVolumeDetails(payload *cloud.DiskOptions, diskDetails *cloud.Disk, is 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 index f29c6aba3..4d26ebd2c 100644 --- 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,