From 22bf0e579034fd182dde4c8fb7fa3bb4dd538949 Mon Sep 17 00:00:00 2001 From: KshitijaKakde <111420075+KshitijaKakde@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:00:36 +0530 Subject: [PATCH] Fix Volume Size Rounding Issue in PowerFlex (#357) --- service/controller.go | 17 ++++++++++++----- service/service_unit_test.go | 18 +++++++++++++++++- service/step_defs_test.go | 12 ++++++------ test/integration/features/integration.feature | 9 +++++++++ test/integration/step_defs_test.go | 6 +++--- 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/service/controller.go b/service/controller.go index e8633503..31b5a2b3 100644 --- a/service/controller.go +++ b/service/controller.go @@ -930,17 +930,24 @@ func validateVolSize(cr *csi.CapacityRange) (int64, error) { } var ( - sizeGiB int64 - sizeKiB int64 - sizeB int64 + sizeGiBFloat float64 + sizeGiB int64 + sizeKiB int64 + sizeB int64 ) + // VxFlexOS creates volumes in multiples of 8GiB, rounding up. // Determine what actual size of volume will be, and check that // we do not exceed maxSize - sizeGiB = minSize / kiBytesInGiB + // Calculate size in GiB using float for precision + sizeGiBFloat = float64(minSize) / float64(kiBytesInGiB) + + // Use math.Ceil to round up to the nearest whole GiB + sizeGiB = int64(math.Ceil(sizeGiBFloat)) + // if the requested size was less than 1GB, set the request to 1GB // so it can be rounded to a 8GiB boundary correctly - if sizeGiB == 0 { + if sizeGiB < 1 { sizeGiB = 1 } mod := sizeGiB % VolSizeMultipleGiB diff --git a/service/service_unit_test.go b/service/service_unit_test.go index c6e8b622..114d3e7a 100644 --- a/service/service_unit_test.go +++ b/service/service_unit_test.go @@ -72,7 +72,7 @@ func TestGetVolSize(t *testing.T) { { // requesting a size less than 1GiB should result in a minimal size cr: &csi.CapacityRange{ - RequiredBytes: 300 * bytesInKiB, + RequiredBytes: 1, LimitBytes: 0, }, sizeKiB: 8 * kiBytesInGiB, @@ -105,6 +105,22 @@ func TestGetVolSize(t *testing.T) { }, sizeKiB: 0, }, + { + // Requesting a size with decimal part that rounds up to next multiple of 8 GiB + cr: &csi.CapacityRange{ + RequiredBytes: int64(9.5 * float64(bytesInGiB)), + LimitBytes: 0, + }, + sizeKiB: 16 * kiBytesInGiB, + }, + { + // Requesting a size of 8.5 GiB to test rounding up + cr: &csi.CapacityRange{ + RequiredBytes: int64(48.5 * float64(bytesInGiB)), + LimitBytes: 0, + }, + sizeKiB: 56 * kiBytesInGiB, + }, } for _, tt := range tests { diff --git a/service/step_defs_test.go b/service/step_defs_test.go index 9203e416..e772aebc 100644 --- a/service/step_defs_test.go +++ b/service/step_defs_test.go @@ -3421,28 +3421,28 @@ func (f *feature) aCorrectNodeGetVolumeStatsResponse() error { return nil } -func (f *feature) iCallNodeUnstageVolumeWith(error string) error { +func (f *feature) iCallNodeUnstageVolumeWith(errStr string) error { // Save the ephemeralStagingMountPath to restore below ephemeralPath := ephemeralStagingMountPath header := metadata.New(map[string]string{"csi.requestid": "1"}) - if error == "NoRequestID" { + if errStr == "NoRequestID" { header = metadata.New(map[string]string{"csi.requestid": ""}) } ctx := metadata.NewIncomingContext(context.Background(), header) req := new(csi.NodeUnstageVolumeRequest) req.VolumeId = goodVolumeID - if error == "NoVolumeID" { + if errStr == "NoVolumeID" { req.VolumeId = "" } req.StagingTargetPath = datadir - if error == "NoStagingTarget" { + if errStr == "NoStagingTarget" { req.StagingTargetPath = "" } - if error == "UnmountError" { + if errStr == "UnmountError" { req.StagingTargetPath = "/tmp" gofsutil.GOFSMock.InduceUnmountError = true } - if error == "EphemeralVolume" { + if errStr == "EphemeralVolume" { // Create an ephemeral volume id ephemeralStagingMountPath = "test/" err := os.MkdirAll("test"+"/"+goodVolumeID+"/id", 0o777) diff --git a/test/integration/features/integration.feature b/test/integration/features/integration.feature index 0c4a4969..0a8d06dd 100644 --- a/test/integration/features/integration.feature +++ b/test/integration/features/integration.feature @@ -37,6 +37,15 @@ Feature: VxFlex OS CSI interface And when I call DeleteVolume Then there are no errors + Scenario: Create and delete basic volume with float size + Given a VxFlexOS service + And a basic block volume request "integration1" "48.1" + When I call CreateVolume + When I call ListVolume + Then a valid ListVolumeResponse is returned + And when I call DeleteVolume + Then there are no errors + Scenario: Idempotent create and delete basic volume Given a VxFlexOS service And a basic block volume request "integration2" "8" diff --git a/test/integration/step_defs_test.go b/test/integration/step_defs_test.go index 180b456c..cc900ff5 100644 --- a/test/integration/step_defs_test.go +++ b/test/integration/step_defs_test.go @@ -331,7 +331,7 @@ func (f *feature) aVxFlexOSService() error { return nil } -func (f *feature) aBasicBlockVolumeRequest(name string, size int64) error { +func (f *feature) aBasicBlockVolumeRequest(name string, size float64) error { req := new(csi.CreateVolumeRequest) storagePool := os.Getenv("STORAGE_POOL") params := make(map[string]string) @@ -344,7 +344,7 @@ func (f *feature) aBasicBlockVolumeRequest(name string, size int64) error { makeAUniqueName(&name) req.Name = name capacityRange := new(csi.CapacityRange) - capacityRange.RequiredBytes = size * 1024 * 1024 * 1024 + capacityRange.RequiredBytes = int64(math.Ceil(size * 1024 * 1024 * 1024)) req.CapacityRange = capacityRange capability := new(csi.VolumeCapability) block := new(csi.VolumeCapability_BlockVolume) @@ -2737,7 +2737,7 @@ func (f *feature) checkNFS(_ context.Context, systemID string) (bool, error) { func FeatureContext(s *godog.ScenarioContext) { f := &feature{} s.Step(`^a VxFlexOS service$`, f.aVxFlexOSService) - s.Step(`^a basic block volume request "([^"]*)" "(\d+)"$`, f.aBasicBlockVolumeRequest) + s.Step(`^a basic block volume request "([^"]*)" "(\d+(\.\d+)?)"$`, f.aBasicBlockVolumeRequest) s.Step(`^Set System Name As "([^"]*)"$`, f.iSetSystemName) s.Step(`^Set Bad AllSystemNames$`, f.iSetBadAllSystemNames) s.Step(`^I call CreateVolume$`, f.iCallCreateVolume)