Skip to content

Commit 285b158

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

File tree

3 files changed

+316
-4
lines changed

3 files changed

+316
-4
lines changed

‎pkg/common/utils.go

Lines changed: 79 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,75 @@ 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+
// Sample formats:
783+
// https://www.googleapis.com/compute/v1/{gce.projectID}/zones/{disk.Zone}/diskTypes/{disk.Type}"
784+
// https://www.googleapis.com/compute/v1/{gce.projectID}/regions/{disk.Region}/diskTypes/{disk.Type}"
785+
return strings.Contains(disk.Type, "hyperdisk")
786+
}
787+
788+
// GetMinIopsThroughput calculates and returns the minimum required IOPS and throughput
789+
// based on the existing disk configuration and the requested new GiB.
790+
// The `needed` return value indicates whether either IOPS or throughput need to be updated.
791+
// https://cloud.google.com/compute/docs/disks/hyperdisks#limits-disk
792+
func GetMinIopsThroughput(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
793+
switch {
794+
case strings.Contains(disk.Type, "hyperdisk-balanced"):
795+
// This includes types "hyperdisk-balanced" and "hyperdisk-balanced-high-availability"
796+
return minIopsForBalanced(disk, requestGb)
797+
case strings.Contains(disk.Type, "hyperdisk-extreme"):
798+
return minIopsForExtreme(disk, requestGb)
799+
case strings.Contains(disk.Type, "hyperdisk-ml"):
800+
return minThroughputForML(disk, requestGb)
801+
case strings.Contains(disk.Type, "hyperdisk-throughput"):
802+
return minThroughputForThroughput(disk, requestGb)
803+
default:
804+
return false, 0, 0
805+
}
806+
}
807+
808+
// minIopsForBalanced calculates and returns the minimum required IOPS and throughput
809+
// for hyperdisk-balanced and hyperdisk-balanced-high-availability disks
810+
func minIopsForBalanced(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
811+
minRequiredIops := requestGb * HyperdiskBalancedIopsPerGB
812+
if minRequiredIops > HyperdiskBalancedMinIops {
813+
minRequiredIops = HyperdiskBalancedMinIops
814+
}
815+
if disk.ProvisionedIops < minRequiredIops {
816+
return true, minRequiredIops, 0
817+
}
818+
return false, 0, 0
819+
}
820+
821+
// minIopsForExtreme calculates and returns the minimum required IOPS and throughput
822+
// for hyperdisk-extreme disks
823+
func minIopsForExtreme(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
824+
minRequiredIops := requestGb * HyperdiskExtremeIopsPerGB
825+
if disk.ProvisionedIops < minRequiredIops {
826+
return true, minRequiredIops, 0
827+
}
828+
return false, 0, 0
829+
}
830+
831+
// minThroughputForML calculates and returns the minimum required IOPS and throughput
832+
// for hyperdisk-ml disks
833+
func minThroughputForML(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
834+
minRequiredThroughput := int64(float64(requestGb) * 0.12)
835+
if disk.ProvisionedThroughput < minRequiredThroughput {
836+
return true, 0, minRequiredThroughput
837+
}
838+
return false, 0, 0
839+
}
840+
841+
// minThroughputForThroughput calculates and returns the minimum required IOPS and throughput
842+
// for hyperdisk-throughput disks
843+
func minThroughputForThroughput(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
844+
minRequiredThroughput := requestGb * HyperdiskThroughputThroughputPerGB / BytesInGB
845+
if disk.ProvisionedThroughput < minRequiredThroughput {
846+
return true, 0, minRequiredThroughput
847+
}
848+
return false, 0, 0
849+
}

‎pkg/common/utils_test.go

Lines changed: 142 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,143 @@ 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+
ProvisionedThroughput: 400,
2051+
SizeGb: 3334,
2052+
},
2053+
reqGb: 3400,
2054+
expectResult: true,
2055+
expectMinThroughput: 408,
2056+
},
2057+
{
2058+
name: "Hyperdisk throughput with min throughput per GB will adjust throughput",
2059+
existingDisk: &computev1.Disk{
2060+
Name: testDiskName,
2061+
Type: "hyperdisk-throughput",
2062+
ProvisionedThroughput: 20,
2063+
SizeGb: 2048,
2064+
},
2065+
reqGb: 3072,
2066+
expectResult: true,
2067+
expectMinIops: 0,
2068+
expectMinThroughput: 30,
2069+
},
2070+
{
2071+
name: "Unknown disk type, no need to update",
2072+
existingDisk: &computev1.Disk{
2073+
Name: testDiskName,
2074+
Type: "unknown-type",
2075+
},
2076+
reqGb: 5,
2077+
expectResult: false,
2078+
expectMinIops: 0,
2079+
expectMinThroughput: 0, // 0 indicates no change to throughput
2080+
},
2081+
}
2082+
for _, tc := range testcases {
2083+
t.Run(tc.name, func(t *testing.T) {
2084+
gotNeeded, gotMinIops, gotMinThroughput := GetMinIopsThroughput(tc.existingDisk, tc.reqGb)
2085+
if gotNeeded != tc.expectResult {
2086+
t.Errorf("GetMinIopsThroughput: got %v, want %v", gotNeeded, tc.expectResult)
2087+
}
2088+
2089+
if gotMinIops != tc.expectMinIops {
2090+
t.Errorf("GetMinIopsThroughput Iops: got %v, want %v", gotMinIops, tc.expectMinIops)
2091+
}
2092+
2093+
if gotMinThroughput != tc.expectMinThroughput {
2094+
t.Errorf("GetMinIopsThroughput Throughput: got %v, want %v", gotMinThroughput, tc.expectMinThroughput)
2095+
}
2096+
})
2097+
}
2098+
}

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

Lines changed: 95 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,10 +1366,58 @@ 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
13721374
}
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+
1387+
updatedDisk.SizeGb = requestGb
1388+
paths = append(paths, "sizeGb")
1389+
1390+
if minIopsUpdate != 0 {
1391+
updatedDisk.ProvisionedIops = minIopsUpdate
1392+
paths = append(paths, "provisionedIops")
1393+
}
1394+
if minThroughputUpdate != 0 {
1395+
updatedDisk.ProvisionedThroughput = minThroughputUpdate
1396+
paths = append(paths, "provisionedThroughput")
1397+
}
1398+
diskUpdateOp, err := cloud.service.Disks.Update(project, volKey.Zone, volKey.Name, updatedDisk).Context(ctx).Paths(paths...).Do()
1399+
if err != nil {
1400+
return -1, fmt.Errorf("failed to resize zonal volume via update %v: %w", volKey.String(), err)
1401+
}
1402+
err = cloud.waitForZonalOp(ctx, project, diskUpdateOp.Name, volKey.Zone)
1403+
if err != nil {
1404+
return -1, fmt.Errorf("failed waiting for op for zonal disk update for %v: %w", volKey, err)
1405+
}
1406+
} else {
1407+
// No updates to iops or throughput for Hyperdisk, using Resize operation
1408+
op, err = cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do()
1409+
if err != nil {
1410+
return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err)
1411+
}
1412+
}
1413+
} else {
1414+
// Iops and throughput are not applicable to PD disk updates
1415+
op, err = cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do()
1416+
if err != nil {
1417+
return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err)
1418+
}
1419+
}
1420+
13731421
klog.V(5).Infof("ResizeDisk operation %s for disk %s", op.Name, volKey.Name)
13741422

13751423
err = cloud.waitForZonalOp(ctx, project, op.Name, volKey.Zone)
@@ -1385,10 +1433,53 @@ func (cloud *CloudProvider) resizeRegionalDisk(ctx context.Context, project stri
13851433
SizeGb: requestGb,
13861434
}
13871435

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

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

0 commit comments

Comments
 (0)