diff --git a/pkg/volume/portworx/portworx_util.go b/pkg/volume/portworx/portworx_util.go index 420afd277d6..5885a1065d7 100644 --- a/pkg/volume/portworx/portworx_util.go +++ b/pkg/volume/portworx/portworx_util.go @@ -214,9 +214,9 @@ func (util *portworxVolumeUtil) ResizeVolume(spec *volume.Spec, newSize resource } vol := vols[0] - tBytes, ok := newSize.AsInt64() - if !ok { - return fmt.Errorf("quantity %v is too great, overflows int64", newSize) + tBytes, err := volumehelpers.RoundUpToB(newSize) + if err != nil { + return err } newSizeInBytes := uint64(tBytes) if vol.Spec.Size >= newSizeInBytes { diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/rounding.go b/staging/src/k8s.io/cloud-provider/volume/helpers/rounding.go index 7adf345007a..a50435c0fc2 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/rounding.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/rounding.go @@ -18,6 +18,8 @@ package helpers import ( "fmt" + "math" + "math/bits" "k8s.io/apimachinery/pkg/api/resource" ) @@ -41,139 +43,110 @@ const ( // RoundUpToGiB rounds up given quantity upto chunks of GiB func RoundUpToGiB(size resource.Quantity) (int64, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSize(requestBytes, GiB), nil + return roundUpSizeInt64(size, GiB) } // RoundUpToMB rounds up given quantity to chunks of MB func RoundUpToMB(size resource.Quantity) (int64, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSize(requestBytes, MB), nil + return roundUpSizeInt64(size, MB) } // RoundUpToMiB rounds up given quantity upto chunks of MiB func RoundUpToMiB(size resource.Quantity) (int64, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSize(requestBytes, MiB), nil + return roundUpSizeInt64(size, MiB) } // RoundUpToKB rounds up given quantity to chunks of KB func RoundUpToKB(size resource.Quantity) (int64, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSize(requestBytes, KB), nil + return roundUpSizeInt64(size, KB) } -// RoundUpToKiB rounds up given quantity upto chunks of KiB +// RoundUpToKiB rounds up given quantity to chunks of KiB func RoundUpToKiB(size resource.Quantity) (int64, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSize(requestBytes, KiB), nil + return roundUpSizeInt64(size, KiB) +} + +// RoundUpToB rounds up given quantity to chunks of bytes +func RoundUpToB(size resource.Quantity) (int64, error) { + return roundUpSizeInt64(size, 1) } // RoundUpToGiBInt rounds up given quantity upto chunks of GiB. It returns an // int instead of an int64 and an error if there's overflow func RoundUpToGiBInt(size resource.Quantity) (int, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSizeInt(requestBytes, GiB) + return roundUpSizeInt(size, GiB) } // RoundUpToMBInt rounds up given quantity to chunks of MB. It returns an // int instead of an int64 and an error if there's overflow func RoundUpToMBInt(size resource.Quantity) (int, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSizeInt(requestBytes, MB) + return roundUpSizeInt(size, MB) } // RoundUpToMiBInt rounds up given quantity upto chunks of MiB. It returns an // int instead of an int64 and an error if there's overflow func RoundUpToMiBInt(size resource.Quantity) (int, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSizeInt(requestBytes, MiB) + return roundUpSizeInt(size, MiB) } // RoundUpToKBInt rounds up given quantity to chunks of KB. It returns an // int instead of an int64 and an error if there's overflow func RoundUpToKBInt(size resource.Quantity) (int, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSizeInt(requestBytes, KB) + return roundUpSizeInt(size, KB) } // RoundUpToKiBInt rounds up given quantity upto chunks of KiB. It returns an // int instead of an int64 and an error if there's overflow func RoundUpToKiBInt(size resource.Quantity) (int, error) { - requestBytes := size.Value() - return roundUpSizeInt(requestBytes, KiB) + return roundUpSizeInt(size, KiB) } // RoundUpToGiBInt32 rounds up given quantity up to chunks of GiB. It returns an // int32 instead of an int64 and an error if there's overflow func RoundUpToGiBInt32(size resource.Quantity) (int32, error) { - requestBytes, ok := size.AsInt64() - if !ok { - return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) - } - return roundUpSizeInt32(requestBytes, GiB) + return roundUpSizeInt32(size, GiB) } // roundUpSizeInt calculates how many allocation units are needed to accommodate -// a volume of given size. It returns an int instead of an int64 and an error if -// there's overflow -func roundUpSizeInt(volumeSizeBytes int64, allocationUnitBytes int64) (int, error) { - roundedUp := roundUpSize(volumeSizeBytes, allocationUnitBytes) - roundedUpInt := int(roundedUp) - if int64(roundedUpInt) != roundedUp { - return 0, fmt.Errorf("capacity %v is too great, casting results in integer overflow", roundedUp) +// a volume of a given size. It returns an int and an error if there's overflow +func roundUpSizeInt(size resource.Quantity, allocationUnitBytes int64) (int, error) { + if bits.UintSize == 32 { + res, err := roundUpSizeInt32(size, allocationUnitBytes) + return int(res), err } - return roundedUpInt, nil + res, err := roundUpSizeInt64(size, allocationUnitBytes) + return int(res), err } // roundUpSizeInt32 calculates how many allocation units are needed to accommodate -// a volume of given size. It returns an int32 instead of an int64 and an error if -// there's overflow -func roundUpSizeInt32(volumeSizeBytes int64, allocationUnitBytes int64) (int32, error) { - roundedUp := roundUpSize(volumeSizeBytes, allocationUnitBytes) - roundedUpInt32 := int32(roundedUp) - if int64(roundedUpInt32) != roundedUp { - return 0, fmt.Errorf("quantity %v is too great, overflows int32", roundedUp) +// a volume of a given size. It returns an int32 and an error if there's overflow +func roundUpSizeInt32(size resource.Quantity, allocationUnitBytes int64) (int32, error) { + roundedUpInt32, err := roundUpSizeInt64(size, allocationUnitBytes) + if err != nil { + return 0, err } - return roundedUpInt32, nil + if roundedUpInt32 > math.MaxInt32 { + return 0, fmt.Errorf("quantity %s is too great, overflows int32", size.String()) + } + return int32(roundedUpInt32), nil } -// roundUpSize calculates how many allocation units are needed to accommodate -// a volume of given size. E.g. when user wants 1500MiB volume, while AWS EBS +// roundUpSize64 calculates how many allocation units are needed to accommodate +// a volume of a given size. E.g. when user wants 1500MiB volume, while AWS EBS // allocates volumes in gibibyte-sized chunks, -// RoundUpSize(1500 * 1024*1024, 1024*1024*1024) returns '2' +// roundUpSizeInt64(1500MiB, 1024*1024*1024) returns '2' // (2 GiB is the smallest allocatable volume that can hold 1500MiB) -func roundUpSize(volumeSizeBytes int64, allocationUnitBytes int64) int64 { +// It returns an int64 and an error if the size was too great to work with +func roundUpSizeInt64(size resource.Quantity, allocationUnitBytes int64) (int64, error) { + // CmpInt64() actually returns 0 when comparing an amount bigger than MaxInt64 + // with MaxInt64 + if size.CmpInt64(math.MaxInt64) >= 0 { + return 0, fmt.Errorf("quantity %s is too great, overflows int64", size.String()) + } + volumeSizeBytes := size.Value() roundedUp := volumeSizeBytes / allocationUnitBytes if volumeSizeBytes%allocationUnitBytes > 0 { roundedUp++ } - return roundedUp + return roundedUp, nil } diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/rounding_test.go b/staging/src/k8s.io/cloud-provider/volume/helpers/rounding_test.go index 24bc17a955e..3a10ac67e5d 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/rounding_test.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/rounding_test.go @@ -60,8 +60,18 @@ func Test_RoundUpToGiB(t *testing.T) { roundedVal: int64(1000), }, { - name: "round overflowed quantity to int64", - resource: resource.MustParse("73786976299133170k"), + name: "round decimal to GiB", + resource: resource.MustParse("1.2Gi"), + roundedVal: int64(2), + }, + { + name: "round big value to GiB", + resource: resource.MustParse("8191Pi"), + roundedVal: int64(8588886016), + }, + { + name: "round quantity to GiB that would lead to an int64 overflow", + resource: resource.MustParse("8192Pi"), roundedVal: int64(0), expectError: true, }, @@ -125,8 +135,18 @@ func Test_RoundUpToMB(t *testing.T) { roundedVal: int64(1073742), }, { - name: "round overflowed quantity to int64", - resource: resource.MustParse("73786976299133170k"), + name: "round decimal to MB", + resource: resource.MustParse("1.2Gi"), + roundedVal: int64(1289), + }, + { + name: "round big value to MB", + resource: resource.MustParse("8191Pi"), + roundedVal: int64(9222246136948), + }, + { + name: "round quantity to MB that would lead to an int64 overflow", + resource: resource.MustParse("8192Pi"), roundedVal: int64(0), expectError: true, }, @@ -190,8 +210,18 @@ func Test_RoundUpToMiB(t *testing.T) { roundedVal: int64(1024000), }, { - name: "round overflowed quantity to int64", - resource: resource.MustParse("73786976299133170k"), + name: "round decimal to MiB", + resource: resource.MustParse("1.2Gi"), + roundedVal: int64(1229), + }, + { + name: "round big value to MiB", + resource: resource.MustParse("8191Pi"), + roundedVal: int64(8795019280384), + }, + { + name: "round quantity to MiB that would lead to an int64 overflow", + resource: resource.MustParse("8192Pi"), roundedVal: int64(0), expectError: true, }, @@ -255,8 +285,18 @@ func Test_RoundUpToKB(t *testing.T) { roundedVal: int64(1073741824), }, { - name: "round overflowed quantity to int64", - resource: resource.MustParse("73786976299133170k"), + name: "round decimal to KB", + resource: resource.MustParse("1.2Gi"), + roundedVal: int64(1288491), + }, + { + name: "round big value to KB", + resource: resource.MustParse("8191Pi"), + roundedVal: int64(9222246136947934), + }, + { + name: "round quantity to KB that would lead to an int64 overflow", + resource: resource.MustParse("8192Pi"), roundedVal: int64(0), expectError: true, }, @@ -320,8 +360,18 @@ func Test_RoundUpToKiB(t *testing.T) { roundedVal: int64(1048576000), }, { - name: "round overflowed quantity to int64", - resource: resource.MustParse("73786976299133170k"), + name: "round decimal to KiB", + resource: resource.MustParse("1.2Gi"), + roundedVal: int64(1258292), + }, + { + name: "round big value to KiB", + resource: resource.MustParse("8191Pi"), + roundedVal: int64(9006099743113216), + }, + { + name: "round quantity to KiB that would lead to an int64 overflow", + resource: resource.MustParse("8192Pi"), roundedVal: int64(0), expectError: true, }, @@ -385,8 +435,18 @@ func Test_RoundUpToGiBInt32(t *testing.T) { roundedVal: int32(1000), }, { - name: "round overflowed quantity to int32", - resource: resource.MustParse("73786976299133170k"), + name: "round decimal to GiB", + resource: resource.MustParse("1.2Gi"), + roundedVal: int32(2), + }, + { + name: "round big value to GiB", + resource: resource.MustParse("2047Pi"), + roundedVal: int32(2146435072), + }, + { + name: "round quantity to GiB that would lead to an int32 overflow", + resource: resource.MustParse("2048Pi"), roundedVal: int32(0), expectError: true, }, @@ -411,3 +471,53 @@ func Test_RoundUpToGiBInt32(t *testing.T) { }) } } + +func Test_RoundUpToB(t *testing.T) { + testcases := []struct { + name string + resource resource.Quantity + roundedVal int64 + expectError bool + }{ + { + name: "round m to B", + resource: resource.MustParse("987m"), + roundedVal: int64(1), + }, + { + name: "round decimal to B", + resource: resource.MustParse("1.2Gi"), + roundedVal: int64(1288490189), + }, + { + name: "round big value to B", + resource: resource.MustParse("8191Pi"), + roundedVal: int64(9222246136947933184), + }, + { + name: "round quantity to B that would lead to an int64 overflow", + resource: resource.MustParse("8192Pi"), + roundedVal: int64(0), + expectError: true, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + val, err := RoundUpToB(test.resource) + if !test.expectError && err != nil { + t.Errorf("expected no error got: %v", err) + } + + if test.expectError && err == nil { + t.Errorf("expected error but got nothing") + } + + if val != test.roundedVal { + t.Logf("actual rounded value: %d", val) + t.Logf("expected rounded value: %d", test.roundedVal) + t.Error("unexpected rounded value") + } + }) + } +}