diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index 3db30787f60..451f4f3ca3f 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -183,7 +183,13 @@ func (plugin *azureFilePlugin) ExpandVolumeDevice( return oldSize, err } - if err := azure.ResizeFileShare(resourceGroup, accountName, shareName, int(volumehelpers.RoundUpToGiB(newSize))); err != nil { + requestGiB, err := volumehelpers.RoundUpToGiBInt(newSize) + + if err != nil { + return oldSize, err + } + + if err := azure.ResizeFileShare(resourceGroup, accountName, shareName, requestGiB); err != nil { return oldSize, err } diff --git a/pkg/volume/azure_file/azure_provision.go b/pkg/volume/azure_file/azure_provision.go index abb5bb8750b..2db069ca27f 100644 --- a/pkg/volume/azure_file/azure_provision.go +++ b/pkg/volume/azure_file/azure_provision.go @@ -158,7 +158,11 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie var sku, resourceGroup, location, account, shareName string capacity := a.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] - requestGiB := int(volumehelpers.RoundUpToGiB(capacity)) + requestGiB, err := volumehelpers.RoundUpToGiBInt(capacity) + if err != nil { + return nil, err + } + secretNamespace := a.options.PVC.Namespace // Apply ProvisionerParameters (case-insensitive). We leave validation of // the values to the cloud provider. diff --git a/pkg/volume/gcepd/gce_util.go b/pkg/volume/gcepd/gce_util.go index ce18270a1b1..c1a607948e2 100644 --- a/pkg/volume/gcepd/gce_util.go +++ b/pkg/volume/gcepd/gce_util.go @@ -101,7 +101,10 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1. name := volumeutil.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 63) // GCE PD name can have up to 63 characters capacity := c.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] // GCE PDs are allocated in chunks of GiBs - requestGB := volumehelpers.RoundUpToGiB(capacity) + requestGB, err := volumehelpers.RoundUpToGiB(capacity) + if err != nil { + return "", 0, nil, "", err + } // Apply Parameters. // Values for parameter "replication-type" are canonicalized to lower case. @@ -159,7 +162,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1. name, diskType, selectedZones, - int64(requestGB), + requestGB, *c.options.CloudTags) if err != nil { klog.V(2).Infof("Error creating regional GCE PD volume: %v", err) @@ -176,7 +179,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1. name, diskType, selectedZone, - int64(requestGB), + requestGB, *c.options.CloudTags) if err != nil { klog.V(2).Infof("Error creating single-zone GCE PD volume: %v", err) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index 698c4e895f0..673f4a13d73 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -1212,11 +1212,18 @@ func (plugin *glusterfsPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize res } // Find out delta size - expansionSize := resource.NewScaledQuantity((newSize.Value() - oldSize.Value()), 0) - expansionSizeGiB := int(volumehelpers.RoundUpToGiB(*expansionSize)) + expansionSize := newSize + expansionSize.Sub(oldSize) + expansionSizeGiB, err := volumehelpers.RoundUpToGiBInt(expansionSize) + if err != nil { + return oldSize, err + } // Find out requested Size - requestGiB := volumehelpers.RoundUpToGiB(newSize) + requestGiB, err := volumehelpers.RoundUpToGiB(newSize) + if err != nil { + return oldSize, err + } //Check the existing volume size currentVolumeInfo, err := cli.VolumeInfo(volumeID) diff --git a/pkg/volume/portworx/portworx_util.go b/pkg/volume/portworx/portworx_util.go index e76031132ab..a8373d9ff2a 100644 --- a/pkg/volume/portworx/portworx_util.go +++ b/pkg/volume/portworx/portworx_util.go @@ -25,7 +25,7 @@ import ( volumeclient "github.com/libopenstorage/openstorage/api/client/volume" osdspec "github.com/libopenstorage/openstorage/api/spec" volumeapi "github.com/libopenstorage/openstorage/volume" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" volumehelpers "k8s.io/cloud-provider/volume/helpers" @@ -60,7 +60,10 @@ func (util *portworxVolumeUtil) CreateVolume(p *portworxVolumeProvisioner) (stri capacity := p.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] // Portworx Volumes are specified in GiB - requestGiB := volumehelpers.RoundUpToGiB(capacity) + requestGiB, err := volumehelpers.RoundUpToGiB(capacity) + if err != nil { + return "", 0, nil, err + } // Perform a best-effort parsing of parameters. Portworx 1.2.9 and later parses volume parameters from // spec.VolumeLabels. So even if below SpecFromOpts() fails to parse certain parameters or @@ -211,7 +214,11 @@ func (util *portworxVolumeUtil) ResizeVolume(spec *volume.Spec, newSize resource } vol := vols[0] - newSizeInBytes := uint64(volumehelpers.RoundUpToGiB(newSize) * volumehelpers.GiB) + tBytes, ok := newSize.AsInt64() + if !ok { + return fmt.Errorf("quantity %v is too great, overflows int64", newSize) + } + newSizeInBytes := uint64(tBytes) if vol.Spec.Size >= newSizeInBytes { klog.Infof("Portworx volume: %s already at size: %d greater than or equal to new "+ "requested size: %d. Skipping resize.", spec.Name(), vol.Spec.Size, newSizeInBytes) diff --git a/pkg/volume/rbd/rbd_util.go b/pkg/volume/rbd/rbd_util.go index 94b409aa88a..7bb6c3e6945 100644 --- a/pkg/volume/rbd/rbd_util.go +++ b/pkg/volume/rbd/rbd_util.go @@ -649,7 +649,11 @@ func (util *rbdUtil) ExpandImage(rbdExpander *rbdVolumeExpander, oldSize resourc var err error // Convert to MB that rbd defaults on. - sz := int(volumehelpers.RoundUpToMiB(newSize)) + sz, err := volumehelpers.RoundUpToMiBInt(newSize) + if err != nil { + return oldSize, err + } + newVolSz := fmt.Sprintf("%d", sz) newSizeQuant := resource.MustParse(fmt.Sprintf("%dMi", sz)) diff --git a/pkg/volume/scaleio/BUILD b/pkg/volume/scaleio/BUILD index f1410f50657..156e91f99c0 100644 --- a/pkg/volume/scaleio/BUILD +++ b/pkg/volume/scaleio/BUILD @@ -23,6 +23,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/util/testing:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/github.com/thecodeteam/goscaleio/types/v1:go_default_library", "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/utils/exec/testing:go_default_library", diff --git a/pkg/volume/scaleio/sio_volume.go b/pkg/volume/scaleio/sio_volume.go index 0d9efeb181b..139ec9e37c1 100644 --- a/pkg/volume/scaleio/sio_volume.go +++ b/pkg/volume/scaleio/sio_volume.go @@ -55,6 +55,10 @@ type sioVolume struct { volume.MetricsNil } +const ( + minimumVolumeSizeGiB = 8 +) + // ******************* // volume.Volume Impl var _ volume.Volume = &sioVolume{} @@ -272,21 +276,17 @@ func (v *sioVolume) Provision(selectedNode *api.Node, allowedTopologies []api.To // setup volume attrributes genName := v.generateName("k8svol", 11) - eightGig := int64(8 * volumehelpers.GiB) capacity := v.options.PVC.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)] - volSizeBytes := capacity.Value() - volSizeGB := int64(volumehelpers.RoundUpToGiB(capacity)) - - if volSizeBytes == 0 { - return nil, fmt.Errorf("invalid volume size of 0 specified") + volSizeGiB, err := volumehelpers.RoundUpToGiB(capacity) + if err != nil { + return nil, err } - if volSizeBytes < eightGig { - eightGiBCapacity := resource.NewQuantity(eightGig, resource.BinarySI) - volSizeGB = int64(volumehelpers.RoundUpToGiB(*eightGiBCapacity)) - klog.V(4).Info(log("capacity less than 8Gi found, adjusted to %dGi", volSizeGB)) + if volSizeGiB < minimumVolumeSizeGiB { + volSizeGiB = minimumVolumeSizeGiB + klog.V(4).Info(log("capacity less than 8Gi found, adjusted to %dGi", volSizeGiB)) } @@ -298,7 +298,7 @@ func (v *sioVolume) Provision(selectedNode *api.Node, allowedTopologies []api.To // create volume volName := genName - vol, err := v.sioMgr.CreateVolume(volName, volSizeGB) + vol, err := v.sioMgr.CreateVolume(volName, volSizeGiB) if err != nil { klog.Error(log("provision failed while creating volume: %v", err)) return nil, err @@ -333,7 +333,7 @@ func (v *sioVolume) Provision(selectedNode *api.Node, allowedTopologies []api.To AccessModes: v.options.PVC.Spec.AccessModes, Capacity: api.ResourceList{ api.ResourceName(api.ResourceStorage): resource.MustParse( - fmt.Sprintf("%dGi", volSizeGB), + fmt.Sprintf("%dGi", volSizeGiB), ), }, PersistentVolumeSource: api.PersistentVolumeSource{ diff --git a/pkg/volume/scaleio/sio_volume_test.go b/pkg/volume/scaleio/sio_volume_test.go index ca871817589..3968704366d 100644 --- a/pkg/volume/scaleio/sio_volume_test.go +++ b/pkg/volume/scaleio/sio_volume_test.go @@ -23,6 +23,7 @@ import ( "strings" "testing" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/klog" api "k8s.io/api/core/v1" @@ -425,7 +426,7 @@ func TestVolumeProvisionerWithIncompleteConfig(t *testing.T) { } } -func TestVolumeProvisionerWithZeroCapacity(t *testing.T) { +func TestVolumeProvisionerWithMinimumCapacity(t *testing.T) { plugMgr, tmpDir := newPluginMgr(t, makeScaleIOSecret(testSecret, testns)) defer os.RemoveAll(tmpDir) @@ -441,7 +442,7 @@ func TestVolumeProvisionerWithZeroCapacity(t *testing.T) { options := volume.VolumeOptions{ ClusterName: "testcluster", PVName: "pvc-sio-dynamic-vol", - PVC: volumetest.CreateTestPVC("0Mi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce}), + PVC: volumetest.CreateTestPVC("100Mi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce}), PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete, } options.PVC.Namespace = testns @@ -466,11 +467,25 @@ func TestVolumeProvisionerWithZeroCapacity(t *testing.T) { } sioVol.sioMgr.client = sio - _, err = provisioner.Provision(nil, nil) - if err == nil { - t.Fatalf("call to Provision() should fail with invalid capacity") + pv, err := + provisioner.Provision(nil, nil) + if err != nil { + t.Fatalf("call to Provision() failed %v", err) } + pvSize := pv.Spec.Capacity.Storage() + if pvSize == nil { + t.Fatalf("unexpected pv size: nil") + } + + gibSize, err := volumehelpers.RoundUpToGiB(*pvSize) + if err != nil { + t.Fatalf("unexpected error while converting size to GiB: %v", err) + } + + if gibSize != minimumVolumeSizeGiB { + t.Fatalf("expected GiB size to be %v got %v", minimumVolumeSizeGiB, gibSize) + } } func TestVolumeProvisionerWithSecretNamespace(t *testing.T) { 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 a4ccaa03480..a3b8ec57a8e 100644 --- a/staging/src/k8s.io/cloud-provider/volume/helpers/rounding.go +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/rounding.go @@ -39,74 +39,88 @@ const ( KiB = 1024 ) -// RoundUpToGB rounds up given quantity to chunks of GB -func RoundUpToGB(size resource.Quantity) int64 { - requestBytes := size.Value() - return roundUpSize(requestBytes, GB) -} - // RoundUpToGiB rounds up given quantity upto chunks of GiB -func RoundUpToGiB(size resource.Quantity) int64 { - requestBytes := size.Value() - return roundUpSize(requestBytes, GiB) +func RoundUpToGiB(size resource.Quantity) (int64, error) { + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } + return roundUpSize(requestBytes, GiB), nil } // RoundUpToMB rounds up given quantity to chunks of MB -func RoundUpToMB(size resource.Quantity) int64 { - requestBytes := size.Value() - return roundUpSize(requestBytes, MB) +func RoundUpToMB(size resource.Quantity) (int64, error) { + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } + return roundUpSize(requestBytes, MB), nil } // RoundUpToMiB rounds up given quantity upto chunks of MiB -func RoundUpToMiB(size resource.Quantity) int64 { - requestBytes := size.Value() - return roundUpSize(requestBytes, MiB) +func RoundUpToMiB(size resource.Quantity) (int64, error) { + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } + return roundUpSize(requestBytes, MiB), nil } // RoundUpToKB rounds up given quantity to chunks of KB -func RoundUpToKB(size resource.Quantity) int64 { - requestBytes := size.Value() - return roundUpSize(requestBytes, KB) +func RoundUpToKB(size resource.Quantity) (int64, error) { + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } + return roundUpSize(requestBytes, KB), nil } // RoundUpToKiB rounds up given quantity upto chunks of KiB -func RoundUpToKiB(size resource.Quantity) int64 { - requestBytes := size.Value() - return roundUpSize(requestBytes, KiB) -} - -// RoundUpToGBInt rounds up given quantity to chunks of GB. It returns an -// int instead of an int64 and an error if there's overflow -func RoundUpToGBInt(size resource.Quantity) (int, error) { - requestBytes := size.Value() - return roundUpSizeInt(requestBytes, GB) +func RoundUpToKiB(size resource.Quantity) (int64, error) { + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } + return roundUpSize(requestBytes, KiB), nil } // 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 := size.Value() + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } return roundUpSizeInt(requestBytes, 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 := size.Value() + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } return roundUpSizeInt(requestBytes, 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 := size.Value() + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } return roundUpSizeInt(requestBytes, 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 := size.Value() + requestBytes, ok := size.AsInt64() + if !ok { + return 0, fmt.Errorf("quantity %v is too great, overflows int64", size) + } return roundUpSizeInt(requestBytes, KB) } @@ -117,6 +131,16 @@ func RoundUpToKiBInt(size resource.Quantity) (int, error) { return roundUpSizeInt(requestBytes, 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 %v is too great, overflows int64", size) + } + return roundUpSizeInt32(requestBytes, 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 @@ -129,6 +153,18 @@ func roundUpSizeInt(volumeSizeBytes int64, allocationUnitBytes int64) (int, erro return roundedUpInt, nil } +// 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) + } + return 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 // allocates volumes in gibibyte-sized chunks, 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 6b2fa12b1b7..24bc17a955e 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 @@ -22,61 +22,12 @@ import ( "k8s.io/apimachinery/pkg/api/resource" ) -func Test_RoundUpToGB(t *testing.T) { - testcases := []struct { - name string - resource resource.Quantity - roundedVal int64 - }{ - { - name: "round Ki to GB", - resource: resource.MustParse("1000Ki"), - roundedVal: int64(1), - }, - { - name: "round k to GB", - resource: resource.MustParse("1000k"), - roundedVal: int64(1), - }, - { - name: "round Mi to GB", - resource: resource.MustParse("1000Mi"), - roundedVal: int64(2), - }, - { - name: "round M to GB", - resource: resource.MustParse("1000M"), - roundedVal: int64(1), - }, - { - name: "round G to GB", - resource: resource.MustParse("1000G"), - roundedVal: int64(1000), - }, - { - name: "round Gi to GB", - resource: resource.MustParse("1000Gi"), - roundedVal: int64(1074), - }, - } - - for _, test := range testcases { - t.Run(test.name, func(t *testing.T) { - val := RoundUpToGB(test.resource) - if val != test.roundedVal { - t.Logf("actual rounded value: %d", val) - t.Logf("expected rounded value: %d", test.roundedVal) - t.Error("unexpected rounded value") - } - }) - } -} - func Test_RoundUpToGiB(t *testing.T) { testcases := []struct { - name string - resource resource.Quantity - roundedVal int64 + name string + resource resource.Quantity + roundedVal int64 + expectError bool }{ { name: "round Ki to GiB", @@ -108,11 +59,25 @@ func Test_RoundUpToGiB(t *testing.T) { resource: resource.MustParse("1000Gi"), roundedVal: int64(1000), }, + { + name: "round overflowed quantity to int64", + resource: resource.MustParse("73786976299133170k"), + roundedVal: int64(0), + expectError: true, + }, } for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - val := RoundUpToGiB(test.resource) + val, err := RoundUpToGiB(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) @@ -124,9 +89,10 @@ func Test_RoundUpToGiB(t *testing.T) { func Test_RoundUpToMB(t *testing.T) { testcases := []struct { - name string - resource resource.Quantity - roundedVal int64 + name string + resource resource.Quantity + roundedVal int64 + expectError bool }{ { name: "round Ki to MB", @@ -158,11 +124,25 @@ func Test_RoundUpToMB(t *testing.T) { resource: resource.MustParse("1000Gi"), roundedVal: int64(1073742), }, + { + name: "round overflowed quantity to int64", + resource: resource.MustParse("73786976299133170k"), + roundedVal: int64(0), + expectError: true, + }, } for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - val := RoundUpToMB(test.resource) + val, err := RoundUpToMB(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) @@ -174,9 +154,10 @@ func Test_RoundUpToMB(t *testing.T) { func Test_RoundUpToMiB(t *testing.T) { testcases := []struct { - name string - resource resource.Quantity - roundedVal int64 + name string + resource resource.Quantity + roundedVal int64 + expectError bool }{ { name: "round Ki to MiB", @@ -208,11 +189,25 @@ func Test_RoundUpToMiB(t *testing.T) { resource: resource.MustParse("1000Gi"), roundedVal: int64(1024000), }, + { + name: "round overflowed quantity to int64", + resource: resource.MustParse("73786976299133170k"), + roundedVal: int64(0), + expectError: true, + }, } for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - val := RoundUpToMiB(test.resource) + val, err := RoundUpToMiB(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) @@ -224,9 +219,10 @@ func Test_RoundUpToMiB(t *testing.T) { func Test_RoundUpToKB(t *testing.T) { testcases := []struct { - name string - resource resource.Quantity - roundedVal int64 + name string + resource resource.Quantity + roundedVal int64 + expectError bool }{ { name: "round Ki to KB", @@ -258,11 +254,25 @@ func Test_RoundUpToKB(t *testing.T) { resource: resource.MustParse("1000Gi"), roundedVal: int64(1073741824), }, + { + name: "round overflowed quantity to int64", + resource: resource.MustParse("73786976299133170k"), + roundedVal: int64(0), + expectError: true, + }, } for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - val := RoundUpToKB(test.resource) + val, err := RoundUpToKB(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) @@ -274,9 +284,10 @@ func Test_RoundUpToKB(t *testing.T) { func Test_RoundUpToKiB(t *testing.T) { testcases := []struct { - name string - resource resource.Quantity - roundedVal int64 + name string + resource resource.Quantity + roundedVal int64 + expectError bool }{ { name: "round Ki to KiB", @@ -308,11 +319,90 @@ func Test_RoundUpToKiB(t *testing.T) { resource: resource.MustParse("1000Gi"), roundedVal: int64(1048576000), }, + { + name: "round overflowed quantity to int64", + resource: resource.MustParse("73786976299133170k"), + roundedVal: int64(0), + expectError: true, + }, } for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - val := RoundUpToKiB(test.resource) + val, err := RoundUpToKiB(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") + } + }) + } +} + +func Test_RoundUpToGiBInt32(t *testing.T) { + testcases := []struct { + name string + resource resource.Quantity + roundedVal int32 + expectError bool + }{ + { + name: "round Ki to GiB", + resource: resource.MustParse("1000Ki"), + roundedVal: int32(1), + }, + { + name: "round k to GiB", + resource: resource.MustParse("1000k"), + roundedVal: int32(1), + }, + { + name: "round Mi to GiB", + resource: resource.MustParse("1000Mi"), + roundedVal: int32(1), + }, + { + name: "round M to GiB", + resource: resource.MustParse("1000M"), + roundedVal: int32(1), + }, + { + name: "round G to GiB", + resource: resource.MustParse("1000G"), + roundedVal: int32(932), + }, + { + name: "round Gi to GiB", + resource: resource.MustParse("1000Gi"), + roundedVal: int32(1000), + }, + { + name: "round overflowed quantity to int32", + resource: resource.MustParse("73786976299133170k"), + roundedVal: int32(0), + expectError: true, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + val, err := RoundUpToGiBInt32(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) diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go index 89c529d4bc2..8e198fdb9c3 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go @@ -2815,7 +2815,10 @@ func (c *Cloud) ResizeDisk( return oldSize, descErr } // AWS resizes in chunks of GiB (not GB) - requestGiB := volumehelpers.RoundUpToGiB(newSize) + requestGiB, err := volumehelpers.RoundUpToGiB(newSize) + if err != nil { + return oldSize, err + } newSizeQuant := resource.MustParse(fmt.Sprintf("%dGi", requestGiB)) // If disk already if of greater or equal size than requested we return diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go index 13f6c59c38d..6df0f317095 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go @@ -284,7 +284,11 @@ func (c *ManagedDiskController) ResizeDisk(diskURI string, oldSize resource.Quan } // Azure resizes in chunks of GiB (not GB) - requestGiB := int32(volumehelpers.RoundUpToGiB(newSize)) + requestGiB, err := volumehelpers.RoundUpToGiBInt32(newSize) + if err != nil { + return oldSize, err + } + newSizeQuant := resource.MustParse(fmt.Sprintf("%dGi", requestGiB)) klog.V(2).Infof("azureDisk - begin to resize disk(%s) with new size(%d), old size(%v)", diskName, requestGiB, oldSize) diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go index 0600f087172..37b244b03f7 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go @@ -25,7 +25,7 @@ import ( "net/http" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" @@ -809,7 +809,11 @@ func (g *Cloud) ResizeDisk(diskToResize string, oldSize resource.Quantity, newSi } // GCE resizes in chunks of GiBs - requestGIB := volumehelpers.RoundUpToGiB(newSize) + requestGIB, err := volumehelpers.RoundUpToGiB(newSize) + if err != nil { + return oldSize, err + } + newSizeQuant := resource.MustParse(fmt.Sprintf("%dGi", requestGIB)) // If disk is already of size equal or greater than requested size, we simply return