diff --git a/pkg/common/utils.go b/pkg/common/utils.go index 719dcb1d3..e2bd766a4 100644 --- a/pkg/common/utils.go +++ b/pkg/common/utils.go @@ -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" @@ -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 ( @@ -764,3 +771,75 @@ func MapNumber(num int64) int64 { } return 0 } + +// 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 +} diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go index 58629dd82..1d4cef0e3 100644 --- a/pkg/common/utils_test.go +++ b/pkg/common/utils_test.go @@ -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" @@ -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) { @@ -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) + } + }) + } +} diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index c03ceeac9..182b7b9f2 100644 --- a/pkg/gce-cloud-provider/compute/gce-compute.go +++ b/pkg/gce-cloud-provider/compute/gce-compute.go @@ -1366,10 +1366,52 @@ func (cloud *CloudProvider) resizeZonalDisk(ctx context.Context, project string, resizeReq := &computev1.DisksResizeRequest{ SizeGb: requestGb, } - op, err := cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do() + + // Get Disk info of disk type, iops and throughput + disk, err := cloud.service.Disks.Get(project, volKey.Zone, volKey.Name).Context(ctx).Do() if err != nil { - return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err) + return -1, err + } + + var op *computev1.Operation + if common.IsUpdateIopsThroughputValuesAllowed(disk) { + // Only Hyperdisks can update iops/throughput + updateNeeded, minIopsUpdate, minThroughputUpdate := common.GetMinIopsThroughput(disk, requestGb) + // If an update is needed, IOPS and/or throughput values must be provided for the GCE Disk Update call + if updateNeeded { + updatedDisk := &computev1.Disk{ + Name: disk.Name, + } + paths := []string{} + updatedDisk.SizeGb = requestGb + paths = append(paths, "sizeGb") + if minIopsUpdate != 0 { + updatedDisk.ProvisionedIops = minIopsUpdate + paths = append(paths, "provisionedIops") + } + if minThroughputUpdate != 0 { + updatedDisk.ProvisionedThroughput = minThroughputUpdate + paths = append(paths, "provisionedThroughput") + } + op, err = cloud.service.Disks.Update(project, volKey.Zone, volKey.Name, updatedDisk).Context(ctx).Paths(paths...).Do() + if err != nil { + return -1, fmt.Errorf("failed to resize zonal volume via update %v: %w", volKey.String(), err) + } + } else { + // No updates to iops or throughput for Hyperdisk, using Resize operation + op, err = cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do() + if err != nil { + return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err) + } + } + } else { + // Iops and throughput are not applicable to PD disk updates + op, err = cloud.service.Disks.Resize(project, volKey.Zone, volKey.Name, resizeReq).Context(ctx).Do() + if err != nil { + return -1, fmt.Errorf("failed to resize zonal volume %v: %w", volKey.String(), err) + } } + klog.V(5).Infof("ResizeDisk operation %s for disk %s", op.Name, volKey.Name) err = cloud.waitForZonalOp(ctx, project, op.Name, volKey.Zone) @@ -1385,10 +1427,49 @@ func (cloud *CloudProvider) resizeRegionalDisk(ctx context.Context, project stri SizeGb: requestGb, } - op, err := cloud.service.RegionDisks.Resize(project, volKey.Region, volKey.Name, resizeReq).Context(ctx).Do() + // Get Disk info of disk type, iops and throughput + disks, err := cloud.service.RegionDisks.Get(project, volKey.Region, volKey.Name).Context(ctx).Do() if err != nil { - return -1, fmt.Errorf("failed to resize regional volume %v: %w", volKey.String(), err) + return -1, err + } + var op *computev1.Operation + if common.IsUpdateIopsThroughputValuesAllowed(disks) { + // Only Hyperdisks can update iops/throughput + updateNeeded, minIopsUpdate, minThroughputUpdate := common.GetMinIopsThroughput(disks, requestGb) + // If an update is needed, IOPS and/or throughput values must be provided for the GCE Disk Update call + if updateNeeded { + updatedDisk := &computev1.Disk{ + Name: disks.Name, + } + paths := []string{} + updatedDisk.SizeGb = requestGb + paths = append(paths, "sizeGb") + if minIopsUpdate != 0 { + updatedDisk.ProvisionedIops = minIopsUpdate + paths = append(paths, "provisionedIops") + } + if minThroughputUpdate != 0 { + updatedDisk.ProvisionedThroughput = minThroughputUpdate + paths = append(paths, "provisionedThroughput") + } + op, err = cloud.service.RegionDisks.Update(project, volKey.Region, volKey.Name, updatedDisk).Context(ctx).Paths(paths...).Do() + if err != nil { + return -1, fmt.Errorf("failed to resize regional volume via update %v: %w", volKey.String(), err) + } + } else { + // No updates to iops or throughput for Hyperdisk, using Resize operation + op, err = cloud.service.RegionDisks.Resize(project, volKey.Region, volKey.Name, resizeReq).Context(ctx).Do() + if err != nil { + return -1, fmt.Errorf("failed to resize regional volume %v: %w", volKey.String(), err) + } + } + } else { + op, err = cloud.service.RegionDisks.Resize(project, volKey.Region, volKey.Name, resizeReq).Context(ctx).Do() + if err != nil { + return -1, fmt.Errorf("failed to resize regional volume %v: %w", volKey.String(), err) + } } + klog.V(5).Infof("ResizeDisk operation %s for disk %s", op.Name, volKey.Name) err = cloud.waitForRegionalOp(ctx, project, op.Name, volKey.Region)