Skip to content

fix: update Hyperdisk attach limits to match GCP documentation for Ge… #2127

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.

Sign up for GitHub

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
k8s-ci-robot merged 1 commit into from
Jul 10, 2025
Merged

fix: update Hyperdisk attach limits to match GCP documentation for Ge… #2127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 35 additions & 8 deletions pkg/common/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,42 @@ const (
AttachLimitOverrideLabel = "gke-volume-attach-limit-override"
)

// doc https://cloud.google.com/compute/docs/disks/hyperdisks#max-total-disks-per-vm
var Gen4MachineHyperdiskAttachLimitMap = []struct {
// doc https://cloud.google.com/compute/docs/general-purpose-machines
// MachineHyperdiskLimit represents the mapping between max vCPUs and hyperdisk (balanced) attach limit
type MachineHyperdiskLimit struct {
max int64
value int64
}{
}

// C4 Machine Types - Hyperdisk Balanced Limits
var C4MachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
{max: 2, value: 7},
{max: 4, value: 15},
{max: 8, value: 23},
{max: 16, value: 31},
{max: 32, value: 49},
{max: 64, value: 63},
{max: 1024, value: 127},
{max: 24, value: 31},
{max: 48, value: 63},
{max: 96, value: 127},
}

// C4D Machine Types - Hyperdisk Balanced Limits
var C4DMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
{max: 2, value: 3},
{max: 4, value: 7},
{max: 8, value: 15},
{max: 96, value: 31},
{max: 192, value: 63},
{max: 384, value: 127},
}

// N4 Machine Types - Hyperdisk Balanced Limits
var N4MachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
{max: 8, value: 15},
{max: 80, value: 31},
}

// C4A Machine Types - Hyperdisk Balanced Limits
var C4AMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
{max: 2, value: 7},
{max: 8, value: 15},
{max: 48, value: 31},
{max: 72, value: 63},
}
37 changes: 31 additions & 6 deletions pkg/common/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,14 +762,39 @@ func ShortString(s string) string {
return string(short)
}

// MapNumber is a function to map input cpu number to the Hyperdisk attach limit
func MapNumber(num int64) int64 {
for _, r := range Gen4MachineHyperdiskAttachLimitMap {
if num <= r.max {
return r.value
// GetHyperdiskAttachLimit returns the hyperdisk attach limit based on machine type prefix and vCPUs
func GetHyperdiskAttachLimit(machineTypePrefix string, vCPUs int64) int64 {
var limitMap []MachineHyperdiskLimit

switch machineTypePrefix {
case "c4":
limitMap = C4MachineHyperdiskAttachLimitMap
case "c4d":
limitMap = C4DMachineHyperdiskAttachLimitMap
case "n4":
limitMap = N4MachineHyperdiskAttachLimitMap
case "c4a":
limitMap = C4AMachineHyperdiskAttachLimitMap
default:
// Fallback to the most conservative Gen4 map for unknown types
return MapNumber(vCPUs, C4DMachineHyperdiskAttachLimitMap)
}

return MapNumber(vCPUs, limitMap)
}

// mapNumber maps the vCPUs to the appropriate hyperdisk limit
func MapNumber(vCPUs int64, limitMap []MachineHyperdiskLimit) int64 {
for _, limit := range limitMap {
if vCPUs <= limit.max {
return limit.value
}
}
return 0
// Return the last value if vCPUs exceeds all max values
if len(limitMap) > 0 {
return limitMap[len(limitMap)-1].value
}
return 15
}

func DiskTypeLabelKey(diskType string) string {
Expand Down
16 changes: 9 additions & 7 deletions pkg/gce-pd-csi-driver/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,17 +858,19 @@ func (ns *GCENodeServer) GetVolumeLimits(ctx context.Context) (int64, error) {
if err != nil {
return volumeLimitBig, fmt.Errorf("invalid cpuString %s for machine type: %v", cpuString, machineType)
}
return common.MapNumber(cpus), nil
// Extract the machine type prefix (e.g., "c4", "c4a", "n4")
prefix := strings.TrimSuffix(gen4Prefix, "-")
return common.GetHyperdiskAttachLimit(prefix, cpus), nil
} else {
return volumeLimitBig, fmt.Errorf("unconventional machine type: %v", machineType)
}
}
if strings.HasPrefix(machineType, "x4-") {
return x4HyperdiskLimit, nil
}
if strings.HasPrefix(machineType, "a4-") {
return a4HyperdiskLimit, nil
}
}
if strings.HasPrefix(machineType, "x4-") {
Copy link
Copy Markdown
Contributor

@sunnylovestiramisu sunnylovestiramisu Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why moved this outside? Because if it is inside the machineType is x4 or a4 it does not need to go through the loop. But on the other hand it does not matter so much in the real world

Copy link
Copy Markdown
Contributor Author

@arsiesys arsiesys Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! Moving these checks outside was what I though could be a performance optimisation.

Since x4/a4 machine types don't match any of the gen4 prefixes (c4a-, c4-, n4-, c4d-), keeping them inside the loop means we'd unnecessarily check them 4 times (once per iteration) before ultimately returning the correct limit.

By moving them outside:

  • gen4 machines: Exit early when matched, never reaching these checks
  • Other machines: Only check x4/a4 once at the end

This reduces string comparisons from up to 12 (4 iterations × 3 checks) to at most 6 (4 gen4 + x4 + a4).

I also considered that x4 and a4 may be less frequently used than the ones in the loop. I will put it back if you think it would be better or not the subject of the current PR (would be understandable).

return x4HyperdiskLimit, nil
}
if strings.HasPrefix(machineType, "a4-") {
return a4HyperdiskLimit, nil
}

return volumeLimitBig, nil
Expand Down
11 changes: 8 additions & 3 deletions pkg/gce-pd-csi-driver/node_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,11 @@ func TestNodeGetVolumeLimits(t *testing.T) {
machineType: "c4-standard-48",
expVolumeLimit: 63,
},
{
name: "c4-standard-2",
machineType: "c4-standard-2",
expVolumeLimit: 7,
},
{
name: "c4a-standard-4",
machineType: "c4a-standard-4",
Expand All @@ -302,7 +307,7 @@ func TestNodeGetVolumeLimits(t *testing.T) {
{
name: "n4-custom-8-12345-ext",
machineType: "n4-custom-8-12345-ext",
expVolumeLimit: 23,
expVolumeLimit: 15,
},
{
name: "n4-custom-16-12345",
Expand Down Expand Up @@ -338,12 +343,12 @@ func TestNodeGetVolumeLimits(t *testing.T) {
{
name: "c4a-standard-32-lssd",
machineType: "c4a-standard-32-lssd",
expVolumeLimit: 49,
expVolumeLimit: 31,
},
{
name: "c4d-standard-32",
machineType: "c4d-standard-32",
expVolumeLimit: 49,
expVolumeLimit: 31,
},
}

Expand Down