Skip to content

Commit

Permalink
Fix Volume Size Rounding Issue in PowerFlex (#357)
Browse files Browse the repository at this point in the history
  • Loading branch information
KshitijaKakde authored Dec 3, 2024
1 parent e9a00c0 commit 22bf0e5
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 15 deletions.
17 changes: 12 additions & 5 deletions service/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 17 additions & 1 deletion service/service_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions service/step_defs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions test/integration/features/integration.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions test/integration/step_defs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 22bf0e5

Please sign in to comment.