Skip to content

Fix Hyperdisk Resize That Requires Iops/Throughput Adjustment #2054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions pkg/common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"github.com/googleapis/gax-go/v2/apierror"
"golang.org/x/time/rate"
computev1 "google.golang.org/api/compute/v1"
"google.golang.org/api/googleapi"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -80,6 +81,12 @@ const (
// projects/{project}/zones/{zone}
zoneURIPattern = "projects/[^/]+/zones/([^/]+)$"
alphanums = "bcdfghjklmnpqrstvwxz2456789"

HyperdiskBalancedIopsPerGB = 500
HyperdiskBalancedMinIops = 3000
HyperdiskExtremeIopsPerGB = 2
HyperdiskThroughputThroughputPerGB = 10
BytesInGB = 1024
)

var (
Expand Down Expand Up @@ -768,3 +775,75 @@ func MapNumber(num int64) int64 {
func DiskTypeLabelKey(diskType string) string {
return fmt.Sprintf("%s/%s", DiskTypeKeyPrefix, diskType)
}

// IsUpdateIopsThroughputValuesAllowed checks if a disk type is hyperdisk,
// which implies that IOPS and throughput values can be updated.
func IsUpdateIopsThroughputValuesAllowed(disk *computev1.Disk) bool {
// Sample formats:
// https://www.googleapis.com/compute/v1/{gce.projectID}/zones/{disk.Zone}/diskTypes/{disk.Type}"
// https://www.googleapis.com/compute/v1/{gce.projectID}/regions/{disk.Region}/diskTypes/{disk.Type}"
return strings.Contains(disk.Type, "hyperdisk")
}

// GetMinIopsThroughput calculates and returns the minimum required IOPS and throughput
// based on the existing disk configuration and the requested new GiB.
// The `needed` return value indicates whether either IOPS or throughput need to be updated.
// https://cloud.google.com/compute/docs/disks/hyperdisks#limits-disk
func GetMinIopsThroughput(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
switch {
case strings.Contains(disk.Type, "hyperdisk-balanced"):
// This includes types "hyperdisk-balanced" and "hyperdisk-balanced-high-availability"
return minIopsForBalanced(disk, requestGb)
case strings.Contains(disk.Type, "hyperdisk-extreme"):
return minIopsForExtreme(disk, requestGb)
case strings.Contains(disk.Type, "hyperdisk-ml"):
return minThroughputForML(disk, requestGb)
case strings.Contains(disk.Type, "hyperdisk-throughput"):
return minThroughputForThroughput(disk, requestGb)
default:
return false, 0, 0
}
}

// minIopsForBalanced calculates and returns the minimum required IOPS and throughput
// for hyperdisk-balanced and hyperdisk-balanced-high-availability disks
func minIopsForBalanced(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
minRequiredIops := requestGb * HyperdiskBalancedIopsPerGB
if minRequiredIops > HyperdiskBalancedMinIops {
minRequiredIops = HyperdiskBalancedMinIops
}
if disk.ProvisionedIops < minRequiredIops {
return true, minRequiredIops, 0
}
return false, 0, 0
}

// minIopsForExtreme calculates and returns the minimum required IOPS and throughput
// for hyperdisk-extreme disks
func minIopsForExtreme(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
minRequiredIops := requestGb * HyperdiskExtremeIopsPerGB
if disk.ProvisionedIops < minRequiredIops {
return true, minRequiredIops, 0
}
return false, 0, 0
}

// minThroughputForML calculates and returns the minimum required IOPS and throughput
// for hyperdisk-ml disks
func minThroughputForML(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
minRequiredThroughput := int64(float64(requestGb) * 0.12)
if disk.ProvisionedThroughput < minRequiredThroughput {
return true, 0, minRequiredThroughput
}
return false, 0, 0
}

// minThroughputForThroughput calculates and returns the minimum required IOPS and throughput
// for hyperdisk-throughput disks
func minThroughputForThroughput(disk *computev1.Disk, requestGb int64) (needed bool, minIops int64, minThroughput int64) {
minRequiredThroughput := requestGb * HyperdiskThroughputThroughputPerGB / BytesInGB
if disk.ProvisionedThroughput < minRequiredThroughput {
return true, 0, minRequiredThroughput
}
return false, 0, 0
}
195 changes: 195 additions & 0 deletions pkg/common/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"github.com/google/go-cmp/cmp"
"github.com/googleapis/gax-go/v2/apierror"
computev1 "google.golang.org/api/compute/v1"
"google.golang.org/api/googleapi"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand All @@ -36,6 +37,7 @@ import (
const (
volIDZoneFmt = "projects/%s/zones/%s/disks/%s"
volIDRegionFmt = "projects/%s/regions/%s/disks/%s"
testDiskName = "test-disk"
)

func TestBytesToGbRoundDown(t *testing.T) {
Expand Down Expand Up @@ -1954,3 +1956,196 @@ func TestNewCombinedError(t *testing.T) {
})
}
}
func TestIsUpdateIopsThroughputValuesAllowed(t *testing.T) {
testcases := []struct {
name string
diskType string
expectResult bool
}{
{
name: "Hyperdisk returns true",
diskType: "hyperdisk-balanced",
expectResult: true,
},
{
name: "PD disk returns true",
diskType: "pd-ssd",
expectResult: false,
},
{
name: "Unknown disk type",
diskType: "not-a-disk-type-we-know",
expectResult: false,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
disk := &computev1.Disk{
Name: "test-disk",
Type: tc.diskType,
}
gotResult := IsUpdateIopsThroughputValuesAllowed(disk)
if gotResult != tc.expectResult {
t.Errorf("IsUpdateIopsThroughputValuesAllowed: got %v, want %v", gotResult, tc.expectResult)
}
})
}
}

func TestGetMinIopsThroughput(t *testing.T) {
testcases := []struct {
name string
existingDisk *computev1.Disk
reqGb int64
expectResult bool
expectMinIops int64
expectMinThroughput int64
}{
{
name: "Hyperdisk Balanced 4 GiB to 5GiB",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-balanced",
ProvisionedIops: 2000,
ProvisionedThroughput: 140,
SizeGb: 4,
},
reqGb: 5,
expectResult: true,
expectMinIops: 2500,
expectMinThroughput: 0, // 0 indicates no change to throughput
},
{
name: "Hyperdisk Balanced 5 GiB to 6GiB",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-balanced",
ProvisionedIops: 2500,
ProvisionedThroughput: 145,
SizeGb: 5,
},
reqGb: 6,
expectResult: true,
expectMinIops: 3000,
expectMinThroughput: 0, // 0 indicates no change to throughput
},
{
name: "Hyperdisk Balanced 6 GiB to 10GiB - no adjustment",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-balanced",
ProvisionedIops: 3000,
ProvisionedThroughput: 145,
SizeGb: 6,
},
reqGb: 10,
expectResult: false,
expectMinIops: 0, // 0 indicates no change to iops
expectMinThroughput: 0, // 0 indicates no change to throughput
},
{
name: "Hyperdisk Extreme with min IOPS value as 2 will adjust IOPs",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-extreme",
ProvisionedIops: 128,
SizeGb: 64,
},
reqGb: 65,
expectResult: true,
expectMinIops: 130,
expectMinThroughput: 0, // 0 indicates no change to throughput
},
{
name: "Hyperdisk Extreme 64GiB to 70 GiB - no adjustment",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-extreme",
ProvisionedIops: 3000,
SizeGb: 64,
},
reqGb: 70,
expectResult: false,
expectMinIops: 0, // 0 indicates no change to iops
expectMinThroughput: 0, // 0 indicates no change to throughput
},
{
name: "Hyperdisk ML with min throughput per GB will adjust throughput",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-ml",
ProvisionedThroughput: 400,
SizeGb: 3334,
},
reqGb: 3400,
expectResult: true,
expectMinThroughput: 408,
},
{
name: "Hyperdisk ML 64GiB to 100 GiB - no adjustment",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-ml",
ProvisionedThroughput: 6400,
SizeGb: 64,
},
reqGb: 100,
expectResult: false,
expectMinIops: 0, // 0 indicates no change to iops
expectMinThroughput: 0, // 0 indicates no change to throughput
},
{
name: "Hyperdisk throughput with min throughput per GB will adjust throughput",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-throughput",
ProvisionedThroughput: 20,
SizeGb: 2048,
},
reqGb: 3072,
expectResult: true,
expectMinIops: 0,
expectMinThroughput: 30,
},
{
name: "Hyperdisk throughput 2TiB to 4TiB - no adjustment",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "hyperdisk-throughput",
ProvisionedThroughput: 567,
SizeGb: 2048,
},
reqGb: 4096,
expectResult: false,
expectMinIops: 0, // 0 indicates no change to iops
expectMinThroughput: 0, // 0 indicates no change to throughput
},
{
name: "Unknown disk type, no need to update",
existingDisk: &computev1.Disk{
Name: testDiskName,
Type: "unknown-type",
},
reqGb: 5,
expectResult: false,
expectMinIops: 0,
expectMinThroughput: 0, // 0 indicates no change to throughput
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
gotNeeded, gotMinIops, gotMinThroughput := GetMinIopsThroughput(tc.existingDisk, tc.reqGb)
if gotNeeded != tc.expectResult {
t.Errorf("GetMinIopsThroughput: got %v, want %v", gotNeeded, tc.expectResult)
}

if gotMinIops != tc.expectMinIops {
t.Errorf("GetMinIopsThroughput Iops: got %v, want %v", gotMinIops, tc.expectMinIops)
}

if gotMinThroughput != tc.expectMinThroughput {
t.Errorf("GetMinIopsThroughput Throughput: got %v, want %v", gotMinThroughput, tc.expectMinThroughput)
}
})
}
}
Loading