Skip to content

Commit 94bbd44

Browse files
sunnylovestiramisusunnylovestiramisu
committed
Fix Hyperdisk Resize That Requires Iops/Throughput Adjustment
1 parent c08bad5 commit 94bbd44

File tree

3 files changed

+313
-4
lines changed

3 files changed

+313
-4
lines changed

‎pkg/common/utils.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
3131
"github.com/googleapis/gax-go/v2/apierror"
3232
"golang.org/x/time/rate"
33+
computev1 "google.golang.org/api/compute/v1"
3334
"google.golang.org/api/googleapi"
3435
"google.golang.org/grpc/codes"
3536
"google.golang.org/grpc/status"
@@ -80,6 +81,12 @@ const (
8081
// projects/{project}/zones/{zone}
8182
zoneURIPattern = "projects/[^/]+/zones/([^/]+)$"
8283
alphanums = "bcdfghjklmnpqrstvwxz2456789"
84+
85+
HyperdiskBalancedIopsPerGB = 500
86+
HyperdiskBalancedMinIops = 3000
87+
HyperdiskExtremeIopsPerGB = 2
88+
HyperdiskThroughputThroughputPerGB = 10
89+
BytesInGB = 1024
8390
)
8491

8592
var (
@@ -768,3 +775,80 @@ func MapNumber(num int64) int64 {
768775
func DiskTypeLabelKey(diskType string) string {
769776
return fmt.Sprintf("%s/%s", DiskTypeKeyPrefix, diskType)
770777
}
778+
779+
// IsUpdateIopsThroughputValuesAllowed checks if a disk type is hyperdisk,
780+
// which implies that IOPS and throughput values can be updated.
781+
func IsUpdateIopsThroughputValuesAllowed(disk *computev1.Disk) bool {
782+
return strings.HasPrefix(disk.Type, "hyperdisk")
783+
}
784+
785+
// GetMinIopsThroughput calculates and returns the minimum required IOPS and throughput
786+
// based on the existing disk configuration and the requested new GiB.
787+
// The `needed` return value indicates whether either IOPS or throughput need to be updated.
788+
// https://cloud.google.com/compute/docs/disks/hyperdisks#limits-disk
789+
func GetMinIopsThroughput(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
790+
switch disk.Type {
791+
case "hyperdisk-balanced", "hyperdisk-balanced-high-availability":
792+
return minIopsForBalanced(disk, requestGb)
793+
case "hyperdisk-extreme":
794+
return minIopsForExtreme(disk, requestGb)
795+
case "hyperdisk-ml":
796+
return minIopsForML(disk, requestGb)
797+
case "hyperdisk-throughput":
798+
return minThroughputForThroughput(disk, requestGb)
799+
default:
800+
return false, 0, 0
801+
}
802+
}
803+
804+
// minIopsForBalanced calculates and returns the minimum required IOPS and throughput
805+
// for hyperdisk-balanced and hyperdisk-balanced-high-availability disks
806+
func minIopsForBalanced(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
807+
minRequiredIops := requestGb * HyperdiskBalancedIopsPerGB
808+
if minRequiredIops > HyperdiskBalancedMinIops {
809+
minRequiredIops = HyperdiskBalancedMinIops
810+
}
811+
if disk.ProvisionedIops < minRequiredIops {
812+
return true, minRequiredIops, 0
813+
}
814+
return false, 0, 0
815+
}
816+
817+
// minIopsForExtreme calculates and returns the minimum required IOPS and throughput
818+
// for hyperdisk-extreme disks
819+
func minIopsForExtreme(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
820+
minRequiredIops := requestGb * HyperdiskExtremeIopsPerGB
821+
if disk.ProvisionedIops < minRequiredIops {
822+
return true, minRequiredIops, 0
823+
}
824+
return false, 0, 0
825+
}
826+
827+
// minIopsForExtreme calculates and returns the minimum required IOPS and throughput
828+
// for hyperdisk-ml disks
829+
func minIopsForML(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
830+
needed = false
831+
resultProvisionedThroughput := int64(0)
832+
resultProvisionedIops := int64(0)
833+
minRequiredThroughput := int64(float64(requestGb) * 0.12)
834+
if disk.ProvisionedThroughput < minRequiredThroughput {
835+
needed = true
836+
resultProvisionedThroughput = minRequiredThroughput
837+
}
838+
minRequiredIops := 16 * minRequiredThroughput
839+
if disk.ProvisionedIops < minRequiredIops {
840+
needed = true
841+
resultProvisionedIops = minRequiredIops
842+
}
843+
return needed, resultProvisionedIops, resultProvisionedThroughput
844+
}
845+
846+
// minIopsForExtreme calculates and returns the minimum required IOPS and throughput
847+
// for hyperdisk-throughput disks
848+
func minThroughputForThroughput(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
849+
minRequiredThroughput := requestGb * HyperdiskThroughputThroughputPerGB / BytesInGB
850+
if disk.ProvisionedThroughput < minRequiredThroughput {
851+
return true, 0, minRequiredThroughput
852+
}
853+
return false, 0, 0
854+
}

‎pkg/common/utils_test.go

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
2929
"github.com/google/go-cmp/cmp"
3030
"github.com/googleapis/gax-go/v2/apierror"
31+
computev1 "google.golang.org/api/compute/v1"
3132
"google.golang.org/api/googleapi"
3233
"google.golang.org/grpc/codes"
3334
"google.golang.org/grpc/status"
@@ -36,6 +37,7 @@ import (
3637
const (
3738
volIDZoneFmt = "projects/%s/zones/%s/disks/%s"
3839
volIDRegionFmt = "projects/%s/regions/%s/disks/%s"
40+
testDiskName = "test-disk"
3941
)
4042

4143
func TestBytesToGbRoundDown(t *testing.T) {
@@ -1954,3 +1956,145 @@ func TestNewCombinedError(t *testing.T) {
19541956
})
19551957
}
19561958
}
1959+
func TestIsUpdateIopsThroughputValuesAllowed(t *testing.T) {
1960+
testcases := []struct {
1961+
name string
1962+
diskType string
1963+
expectResult bool
1964+
}{
1965+
{
1966+
name: "Hyperdisk returns true",
1967+
diskType: "hyperdisk-balanced",
1968+
expectResult: true,
1969+
},
1970+
{
1971+
name: "PD disk returns true",
1972+
diskType: "pd-ssd",
1973+
expectResult: false,
1974+
},
1975+
{
1976+
name: "Unknown disk type",
1977+
diskType: "not-a-disk-type-we-know",
1978+
expectResult: false,
1979+
},
1980+
}
1981+
for _, tc := range testcases {
1982+
t.Run(tc.name, func(t *testing.T) {
1983+
disk := &computev1.Disk{
1984+
Name: "test-disk",
1985+
Type: tc.diskType,
1986+
}
1987+
gotResult := IsUpdateIopsThroughputValuesAllowed(disk)
1988+
if gotResult != tc.expectResult {
1989+
t.Errorf("IsUpdateIopsThroughputValuesAllowed: got %v, want %v", gotResult, tc.expectResult)
1990+
}
1991+
})
1992+
}
1993+
}
1994+
1995+
func TestGetMinIopsThroughput(t *testing.T) {
1996+
testcases := []struct {
1997+
name string
1998+
existingDisk *computev1.Disk
1999+
reqGb int64
2000+
expectResult bool
2001+
expectMinIops int64
2002+
expectMinThroughput int64
2003+
}{
2004+
{
2005+
name: "Hyperdisk Balanced 4 GiB to 5GiB",
2006+
existingDisk: &computev1.Disk{
2007+
Name: testDiskName,
2008+
Type: "hyperdisk-balanced",
2009+
ProvisionedIops: 2000,
2010+
ProvisionedThroughput: 140,
2011+
SizeGb: 4,
2012+
},
2013+
reqGb: 5,
2014+
expectResult: true,
2015+
expectMinIops: 2500,
2016+
expectMinThroughput: 0, // 0 indicates no change to throughput
2017+
},
2018+
{
2019+
name: "Hyperdisk Balanced 5 GiB to 6GiB",
2020+
existingDisk: &computev1.Disk{
2021+
Name: testDiskName,
2022+
Type: "hyperdisk-balanced",
2023+
ProvisionedIops: 2500,
2024+
ProvisionedThroughput: 145,
2025+
SizeGb: 5,
2026+
},
2027+
reqGb: 6,
2028+
expectResult: true,
2029+
expectMinIops: 3000,
2030+
expectMinThroughput: 0, // 0 indicates no change to throughput
2031+
},
2032+
{
2033+
name: "Hyperdisk Extreme with min IOPS value as 2 will adjust IOPs",
2034+
existingDisk: &computev1.Disk{
2035+
Name: testDiskName,
2036+
Type: "hyperdisk-extreme",
2037+
ProvisionedIops: 128,
2038+
SizeGb: 64,
2039+
},
2040+
reqGb: 65,
2041+
expectResult: true,
2042+
expectMinIops: 130,
2043+
expectMinThroughput: 0, // 0 indicates no change to throughput
2044+
},
2045+
{
2046+
name: "Hyperdisk ML with min IOPS/throughput will adjust both",
2047+
existingDisk: &computev1.Disk{
2048+
Name: testDiskName,
2049+
Type: "hyperdisk-ml",
2050+
ProvisionedIops: 6400,
2051+
ProvisionedThroughput: 400,
2052+
SizeGb: 3334,
2053+
},
2054+
reqGb: 3400,
2055+
expectResult: true,
2056+
expectMinIops: 6528,
2057+
expectMinThroughput: 408,
2058+
},
2059+
{
2060+
name: "Hyperdisk throughput with min throughput per GB will adjust throughput",
2061+
existingDisk: &computev1.Disk{
2062+
Name: testDiskName,
2063+
Type: "hyperdisk-throughput",
2064+
ProvisionedThroughput: 20,
2065+
SizeGb: 2048,
2066+
},
2067+
reqGb: 3072,
2068+
expectResult: true,
2069+
expectMinIops: 0,
2070+
expectMinThroughput: 30,
2071+
},
2072+
{
2073+
name: "Unknown disk type, no need to update",
2074+
existingDisk: &computev1.Disk{
2075+
Name: testDiskName,
2076+
Type: "unknown-type",
2077+
},
2078+
reqGb: 5,
2079+
expectResult: false,
2080+
expectMinIops: 0,
2081+
expectMinThroughput: 0, // 0 indicates no change to throughput
2082+
},
2083+
}
2084+
for _, tc := range testcases {
2085+
t.Run(tc.name, func(t *testing.T) {
2086+
gotNeeded, gotMinIops, gotMinThroughput := GetMinIopsThroughput(tc.existingDisk, tc.reqGb)
2087+
if gotNeeded != tc.expectResult {
2088+
t.Errorf("GetMinIopsThroughput: got %v, want %v", gotNeeded, tc.expectResult)
2089+
}
2090+
2091+
if gotMinIops != tc.expectMinIops {
2092+
t.Errorf("GetMinIopsThroughput Iops: got %v, want %v", gotMinIops, tc.expectMinIops)
2093+
}
2094+
2095+
if gotMinThroughput != tc.expectMinThroughput {
2096+
t.Errorf("GetMinIopsThroughput Throughput: got %v, want %v", gotMinThroughput, tc.expectMinThroughput)
2097+
}
2098+
})
2099+
}
2100+
}

‎pkg/gce-cloud-provider/compute/gce-compute.go

Lines changed: 85 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,10 +1366,52 @@ func (cloud *CloudProvider) resizeZonalDisk(ctx context.Context, project string,
13661366
resizeReq := &computev1.DisksResizeRequest{
13671367
SizeGb: requestGb,
13681368
}
1369-
op, err := cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do()
1369+
1370+
// Get Disk info of disk type, iops and throughput
1371+
disk, err := cloud.service.Disks.Get(project, volKey.Zone, volKey.Name).Context(ctx).Do()
13701372
if err != nil {
1371-
return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err)
1373+
return -1, err
1374+
}
1375+
1376+
var op *computev1.Operation
1377+
if common.IsUpdateIopsThroughputValuesAllowed(disk) {
1378+
// Only Hyperdisks can update iops/throughput
1379+
updateNeeded, minIopsUpdate, minThroughputUpdate := common.GetMinIopsThroughput(disk, requestGb)
1380+
// If an update is needed, IOPS and/or throughput values must be provided for the GCE Disk Update call
1381+
if updateNeeded {
1382+
updatedDisk := &computev1.Disk{
1383+
Name: disk.Name,
1384+
}
1385+
paths := []string{}
1386+
if minIopsUpdate != 0 {
1387+
updatedDisk.ProvisionedIops = minIopsUpdate
1388+
paths = append(paths, "provisionedIops")
1389+
}
1390+
if minThroughputUpdate != 0 {
1391+
updatedDisk.ProvisionedThroughput = minThroughputUpdate
1392+
paths = append(paths, "provisionedThroughput")
1393+
}
1394+
diskUpdateOp := cloud.service.Disks.Update(project, volKey.Zone, volKey.Name, updatedDisk)
1395+
diskUpdateOp.Paths(paths...)
1396+
_, err := diskUpdateOp.Context(ctx).Do()
1397+
if err != nil {
1398+
return -1, fmt.Errorf("failed to resize zonal volume via update %v: %w", volKey.String(), err)
1399+
}
1400+
} else {
1401+
// No updates to iops or throughput for Hyperdisk, using Resize operation
1402+
op, err = cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do()
1403+
if err != nil {
1404+
return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err)
1405+
}
1406+
}
1407+
} else {
1408+
// Iops and throughput are not applicable to PD disk updates
1409+
op, err = cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do()
1410+
if err != nil {
1411+
return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err)
1412+
}
13721413
}
1414+
13731415
klog.V(5).Infof("ResizeDisk operation %s for disk %s", op.Name, volKey.Name)
13741416

13751417
err = cloud.waitForZonalOp(ctx, project, op.Name, volKey.Zone)
@@ -1385,10 +1427,49 @@ func (cloud *CloudProvider) resizeRegionalDisk(ctx context.Context, project stri
13851427
SizeGb: requestGb,
13861428
}
13871429

1388-
op, err := cloud.service.RegionDisks.Resize(project, volKey.Region, volKey.Name, resizeReq).Context(ctx).Do()
1430+
// Get Disk info of disk type, iops and throughput
1431+
disks, err := cloud.service.RegionDisks.Get(project, volKey.Zone, volKey.Name).Context(ctx).Do()
13891432
if err != nil {
1390-
return -1, fmt.Errorf("failed to resize regional volume %v: %w", volKey.String(), err)
1433+
return -1, err
1434+
}
1435+
var op *computev1.Operation
1436+
if common.IsUpdateIopsThroughputValuesAllowed(disks) {
1437+
// Only Hyperdisks can update iops/throughput
1438+
updateNeeded, minIopsUpdate, minThroughputUpdate := common.GetMinIopsThroughput(disks, requestGb)
1439+
// If an update is needed, IOPS and/or throughput values must be provided for the GCE Disk Update call
1440+
if updateNeeded {
1441+
updatedDisk := &computev1.Disk{
1442+
Name: disks.Name,
1443+
}
1444+
paths := []string{}
1445+
if minIopsUpdate != 0 {
1446+
updatedDisk.ProvisionedIops = minIopsUpdate
1447+
paths = append(paths, "provisionedIops")
1448+
}
1449+
if minThroughputUpdate != 0 {
1450+
updatedDisk.ProvisionedThroughput = minThroughputUpdate
1451+
paths = append(paths, "provisionedThroughput")
1452+
}
1453+
disksUpdateOp := cloud.service.RegionDisks.Update(project, volKey.Zone, volKey.Name, updatedDisk)
1454+
disksUpdateOp.Paths(paths...)
1455+
_, err := disksUpdateOp.Context(ctx).Do()
1456+
if err != nil {
1457+
return -1, fmt.Errorf("failed to resize regional volume via update %v: %w", volKey.String(), err)
1458+
}
1459+
} else {
1460+
// No updates to iops or throughput for Hyperdisk, using Resize operation
1461+
op, err = cloud.service.RegionDisks.Resize(project, volKey.Region, volKey.Name, resizeReq).Context(ctx).Do()
1462+
if err != nil {
1463+
return -1, fmt.Errorf("failed to resize regional volume %v: %w", volKey.String(), err)
1464+
}
1465+
}
1466+
} else {
1467+
op, err = cloud.service.RegionDisks.Resize(project, volKey.Region, volKey.Name, resizeReq).Context(ctx).Do()
1468+
if err != nil {
1469+
return -1, fmt.Errorf("failed to resize regional volume %v: %w", volKey.String(), err)
1470+
}
13911471
}
1472+
13921473
klog.V(5).Infof("ResizeDisk operation %s for disk %s", op.Name, volKey.Name)
13931474

13941475
err = cloud.waitForRegionalOp(ctx, project, op.Name, volKey.Region)

0 commit comments

Comments
 (0)