From 6ba5fe007a9b6c1ba4ca077664a364c381c16e4c Mon Sep 17 00:00:00 2001 From: Jiawei Wang Date: Tue, 15 Dec 2020 15:55:30 -0800 Subject: [PATCH] Round up pdcsi driver size in CreateVolume --- pkg/common/utils.go | 10 +++- pkg/common/utils_test.go | 47 ++++++++++++++++++- pkg/gce-cloud-provider/compute/fake-gce.go | 8 ++-- pkg/gce-cloud-provider/compute/gce-compute.go | 6 +-- pkg/gce-pd-csi-driver/controller.go | 9 ++-- test/e2e/utils/utils.go | 2 +- 6 files changed, 68 insertions(+), 14 deletions(-) diff --git a/pkg/common/utils.go b/pkg/common/utils.go index d5fa8f897..c48045e33 100644 --- a/pkg/common/utils.go +++ b/pkg/common/utils.go @@ -49,11 +49,19 @@ const ( regionalDeviceNameSuffix = "_regional" ) -func BytesToGb(bytes int64) int64 { +func BytesToGbRoundDown(bytes int64) int64 { // TODO: Throw an error when div to 0 return bytes / (1024 * 1024 * 1024) } +func BytesToGbRoundUp(bytes int64) int64 { + re := bytes / (1024 * 1024 * 1024) + if (bytes % (1024 * 1024 * 1024)) != 0 { + re++ + } + return re +} + func GbToBytes(Gb int64) int64 { // TODO: Check for overflow return Gb * 1024 * 1024 * 1024 diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go index 240d0c3bd..57de58c30 100644 --- a/pkg/common/utils_test.go +++ b/pkg/common/utils_test.go @@ -29,7 +29,7 @@ const ( volIDRegionFmt = "projects/%s/regions/%s/disks/%s" ) -func TestBytesToGb(t *testing.T) { +func TestBytesToGbRoundDown(t *testing.T) { testCases := []struct { name string bytes int64 @@ -58,7 +58,50 @@ func TestBytesToGb(t *testing.T) { } for _, tc := range testCases { t.Logf("test case: %s", tc.name) - gotGB := BytesToGb(tc.bytes) + gotGB := BytesToGbRoundDown(tc.bytes) + + if gotGB != tc.expGB { + t.Errorf("got GB %v, expected %v", gotGB, tc.expGB) + } + + } +} + +func TestBytesToGbRoundUp(t *testing.T) { + testCases := []struct { + name string + bytes int64 + expGB int64 + }{ + { + name: "normal 5gb", + bytes: 5368709120, + expGB: 5, + }, + { + name: "slightly less than 5gb", + bytes: 5368709119, + expGB: 5, + }, + { + name: "slightly more than 5gb", + bytes: 5368709121, + expGB: 6, + }, + { + name: "1.5Gi", + bytes: 1610612736, + expGB: 2, + }, + { + name: "zero", + bytes: 0, + expGB: 0, + }, + } + for _, tc := range testCases { + t.Logf("test case: %s", tc.name) + gotGB := BytesToGbRoundUp(tc.bytes) if gotGB != tc.expGB { t.Errorf("got GB %v, expected %v", gotGB, tc.expGB) diff --git a/pkg/gce-cloud-provider/compute/fake-gce.go b/pkg/gce-cloud-provider/compute/fake-gce.go index 93da6fb5d..910d6bcf7 100644 --- a/pkg/gce-cloud-provider/compute/fake-gce.go +++ b/pkg/gce-cloud-provider/compute/fake-gce.go @@ -262,7 +262,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key computeDisk := &computev1.Disk{ Name: volKey.Name, - SizeGb: common.BytesToGb(capBytes), + SizeGb: common.BytesToGbRoundUp(capBytes), Description: "Disk created by GCE-PD CSI Driver", Type: cloud.GetDiskTypeURI(volKey, params.DiskType), SourceSnapshotId: snapshotID, @@ -414,9 +414,11 @@ func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, volKey *meta.Key return -1, notFoundError() } - disk.setSizeGb(common.BytesToGb(requestBytes)) + requestSizGb := common.BytesToGbRoundUp(requestBytes) - return common.BytesToGb(requestBytes), nil + disk.setSizeGb(requestSizGb) + + return requestSizGb, nil } diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index afca4bba2..760f5a40c 100644 --- a/pkg/gce-cloud-provider/compute/gce-compute.go +++ b/pkg/gce-cloud-provider/compute/gce-compute.go @@ -381,7 +381,7 @@ func (cloud *CloudProvider) insertRegionalDisk( diskToCreate := &computev1.Disk{ Name: volKey.Name, - SizeGb: common.BytesToGb(capBytes), + SizeGb: common.BytesToGbRoundUp(capBytes), Description: description, Type: cloud.GetDiskTypeURI(volKey, params.DiskType), } @@ -474,7 +474,7 @@ func (cloud *CloudProvider) insertZonalDisk( diskToCreate := &computev1.Disk{ Name: volKey.Name, - SizeGb: common.BytesToGb(capBytes), + SizeGb: common.BytesToGbRoundUp(capBytes), Description: description, Type: cloud.GetDiskTypeURI(volKey, params.DiskType), } @@ -815,7 +815,7 @@ func (cloud *CloudProvider) ResizeDisk(ctx context.Context, volKey *meta.Key, re } sizeGb := cloudDisk.GetSizeGb() - requestGb := common.BytesToGb(requestBytes) + requestGb := common.BytesToGbRoundUp(requestBytes) // If disk is already of size equal or greater than requested size, we // simply return the found size diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index 6974b18ae..5d164db21 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -182,7 +182,7 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre // If there is no validation error, immediately return success klog.V(4).Infof("CreateVolume succeeded for disk %v, it already exists and was compatible", volKey) - return generateCreateVolumeResponse(existingDisk, capBytes, zones), nil + return generateCreateVolumeResponse(existingDisk, zones), nil } snapshotID := "" @@ -234,7 +234,7 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre } klog.V(4).Infof("CreateVolume succeeded for disk %v", volKey) - return generateCreateVolumeResponse(disk, capBytes, zones), nil + return generateCreateVolumeResponse(disk, zones), nil } @@ -979,16 +979,17 @@ func getDefaultZonesInRegion(ctx context.Context, gceCS *GCEControllerServer, ex return ret, nil } -func generateCreateVolumeResponse(disk *gce.CloudDisk, capBytes int64, zones []string) *csi.CreateVolumeResponse { +func generateCreateVolumeResponse(disk *gce.CloudDisk, zones []string) *csi.CreateVolumeResponse { tops := []*csi.Topology{} for _, zone := range zones { tops = append(tops, &csi.Topology{ Segments: map[string]string{common.TopologyKeyZone: zone}, }) } + realDiskSizeBytes := common.GbToBytes(disk.GetSizeGb()) createResp := &csi.CreateVolumeResponse{ Volume: &csi.Volume{ - CapacityBytes: capBytes, + CapacityBytes: realDiskSizeBytes, VolumeId: cleanSelfLink(disk.GetSelfLink()), VolumeContext: nil, AccessibleTopology: tops, diff --git a/test/e2e/utils/utils.go b/test/e2e/utils/utils.go index f13c3195e..c901e2359 100644 --- a/test/e2e/utils/utils.go +++ b/test/e2e/utils/utils.go @@ -210,7 +210,7 @@ func GetBlockSizeInGb(instance *remote.InstanceInfo, devicePath string) (int64, if err != nil { return -1, fmt.Errorf("failed to parse size %s into int", output) } - return utilcommon.BytesToGb(n), nil + return utilcommon.BytesToGbRoundDown(n), nil } func Symlink(instance *remote.InstanceInfo, src, dest string) error {