Repository: kubernetes-sigs/gcp-compute-persistent-disk-csi-driver

Issue:
N4 instances have wrong attach limit
N4 instances can use only hyperdisk-balanced disks and they have a different attach limit than regular "persistent disks": https://cloud.google.com/compute/docs/general-purpose-machines#disk_and_capacity_limits_2

The CSI driver should report 20 disks per n4-standard-2, now it reports 127.

Discussion:
@k8s-triage-robot: The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle stale`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle stale

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@k8s-triage-robot: The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle rotten`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle rotten

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@jsafrane: /remove-lifecycle rotten@sunnylovestiramisu: This is fixed in master

/close@k8s-ci-robot: @sunnylovestiramisu: Closing this issue.

<details>

In response to [this](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues/1819#issuecomment-2776317759):

>This is fixed in master
>
>/close


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>@jsafrane: /reopen

It's much better, just off-by-one :-).
The driver reports 16 attachments on n4-standard-4, but it forgets the root disk. Only 15 Pods with one volume each can run on the machine, the 16th one shows:

`AttachVolume.Attach failed for volume "pvc-c1673c2a-7749-4b9b-b270-3544d9b04cef" : rpc error: code = InvalidArgument desc = Failed to Attach: failed when waiting for zonal op: rpc error: code = InvalidArgument desc = operation operation-1744802898292-632e394f4ad33-7301bb9b-ce176f79 failed (UNSUPPORTED_OPERATION): Maximum hyperdisk-balanced disks count should be less than or equal to [16], Requested : [17]`

The driver reports 127 attachments for pd-disks, so it should report 15 for hyperdisks.@k8s-ci-robot: @jsafrane: Reopened this issue.

<details>

In response to [this](https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues/1819#issuecomment-2809295748):

>/reopen
>
>It's much better, just off-by-one :-).
>The driver reports 16 attachments on n4-standard-4, but it forgets the root disk. Only 15 Pods with one volume each can run on the machine, 16th shows:
>
>```
>AttachVolume.Attach failed for volume "pvc-c1673c2a-7749-4b9b-b270-3544d9b04cef" : rpc error: code = InvalidArgument desc = Failed to Attach: failed when waiting for zonal op: rpc error: code = InvalidArgument desc = operation operation-1744802898292-632e394f4ad33-7301bb9b-ce176f79 failed (UNSUPPORTED_OPERATION): Maximum hyperdisk-balanced disks count should be less than or equal to [16], Requested : [17]
>```
>
>The driver reports 127 attachments for pd-disks, so it should report 15 for hyperdisks.


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>@jsafrane: Proposing a fix in https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/pull/2057

Solution patch:
diff --git a/pkg/common/constants.go b/pkg/common/constants.go
index a6003b65..d5e5c101 100644
--- a/pkg/common/constants.go
+++ b/pkg/common/constants.go
@@ -57,3 +57,16 @@ const (
 	NodeLabelPrefix         = "cloud.google.com/%s"
 	DataCacheLssdCountLabel = "gke-data-cache-disk"
 )
+
+// doc https://cloud.google.com/compute/docs/disks/hyperdisks#max-total-disks-per-vm
+var Gen4MachineHyperdiskAttachLimitMap = []struct {
+	max   int64
+	value int64
+}{
+	{max: 4, value: 16},
+	{max: 8, value: 24},
+	{max: 16, value: 32},
+	{max: 32, value: 48},
+	{max: 64, value: 64},
+	{max: 1024, value: 128},
+}
diff --git a/pkg/common/utils.go b/pkg/common/utils.go
index 3abe5dde..719dcb1d 100644
--- a/pkg/common/utils.go
+++ b/pkg/common/utils.go
@@ -754,3 +754,13 @@ 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
+		}
+	}
+	return 0
+}
diff --git a/pkg/gce-pd-csi-driver/node.go b/pkg/gce-pd-csi-driver/node.go
index 52abeb8e..f45d159b 100644
--- a/pkg/gce-pd-csi-driver/node.go
+++ b/pkg/gce-pd-csi-driver/node.go
@@ -23,6 +23,7 @@ import (
 	"regexp"
 	"runtime"
 	"strconv"
+	"strings"
 	"time"
 
 	"google.golang.org/grpc/codes"
@@ -95,8 +96,12 @@ var _ csi.NodeServer = &GCENodeServer{}
 // node boot disk is considered an attachable disk so effective attach limit is
 // one less.
 const (
-	volumeLimitSmall     int64 = 15
-	volumeLimitBig       int64 = 127
+	volumeLimitSmall int64 = 15
+	volumeLimitBig   int64 = 127
+	// doc https://cloud.google.com/compute/docs/memory-optimized-machines#x4_disks
+	x4HyperdiskLimit int64 = 40
+	// doc https://cloud.google.com/compute/docs/accelerator-optimized-machines#a4-disks
+	a4HyperdiskLimit     int64 = 128
 	defaultLinuxFsType         = "ext4"
 	defaultWindowsFsType       = "ntfs"
 	fsTypeExt3                 = "ext3"
@@ -567,6 +572,9 @@ func (ns *GCENodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRe
 	nodeID := common.CreateNodeID(ns.MetadataService.GetProject(), ns.MetadataService.GetZone(), ns.MetadataService.GetName())
 
 	volumeLimits, err := ns.GetVolumeLimits()
+	if err != nil {
+		klog.Errorf("GetVolumeLimits failed: %v", err.Error())
+	}
 
 	resp := &csi.NodeGetInfoResponse{
 		NodeId:             nodeID,
@@ -733,5 +741,24 @@ func (ns *GCENodeServer) GetVolumeLimits() (int64, error) {
 			return volumeLimitSmall, nil
 		}
 	}
+	gen4MachineTypesPrefix := []string{"c4a-", "c4-", "n4-"}
+	for _, gen4Prefix := range gen4MachineTypesPrefix {
+		if strings.HasPrefix(machineType, gen4Prefix) {
+			cpuString := machineType[strings.LastIndex(machineType, "-")+1:]
+			cpus, err := strconv.ParseInt(cpuString, 10, 64)
+			if err != nil {
+				return volumeLimitSmall, fmt.Errorf("invalid cpuString %s for machine type: %v", cpuString, machineType)
+			}
+			return common.MapNumber(cpus), nil
+
+		}
+		if strings.HasPrefix(machineType, "x4-") {
+			return x4HyperdiskLimit, nil
+		}
+		if strings.HasPrefix(machineType, "a4-") {
+			return a4HyperdiskLimit, nil
+		}
+	}
+
 	return volumeLimitBig, nil
 }



Test patch:
diff --git a/pkg/gce-pd-csi-driver/node_test.go b/pkg/gce-pd-csi-driver/node_test.go
index ee016df0..57c3e82b 100644
--- a/pkg/gce-pd-csi-driver/node_test.go
+++ b/pkg/gce-pd-csi-driver/node_test.go
@@ -224,6 +224,7 @@ func TestNodeGetVolumeLimits(t *testing.T) {
 		name           string
 		machineType    string
 		expVolumeLimit int64
+		expectError    bool
 	}{
 		{
 			name:           "Predifined standard machine",
@@ -255,13 +256,54 @@ func TestNodeGetVolumeLimits(t *testing.T) {
 			machineType:    "e2-micro",
 			expVolumeLimit: volumeLimitSmall,
 		},
+		{
+			name:           "c4-standard-192",
+			machineType:    "c4-standard-192",
+			expVolumeLimit: 128,
+		},
+		{
+			name:           "c4-standard-48",
+			machineType:    "c4-standard-48",
+			expVolumeLimit: 64,
+		},
+		{
+			name:           "c4a-standard-4",
+			machineType:    "c4a-standard-4",
+			expVolumeLimit: 16,
+		},
+		{
+			name:           "n4-standard-16",
+			machineType:    "n4-standard-16",
+			expVolumeLimit: 32,
+		},
+		{
+			name:           "n4-highcpu-4",
+			machineType:    "n4-highcpu-4",
+			expVolumeLimit: 16,
+		},
+		{
+			name:           "invalid gen4 machine type",
+			machineType:    "n4-highcpu-4xyz",
+			expVolumeLimit: volumeLimitSmall,
+			expectError:    true,
+		},
+		{
+			name:           "x4-megamem-960-metal",
+			machineType:    "x4-megamem-960-metal",
+			expVolumeLimit: x4HyperdiskLimit,
+		},
+		{
+			name:           "a4-highgpu-8g",
+			machineType:    "a4-highgpu-8g",
+			expVolumeLimit: a4HyperdiskLimit,
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Logf("Test case: %s", tc.name)
 		metadataservice.SetMachineType(tc.machineType)
 		res, err := ns.NodeGetInfo(context.Background(), req)
-		if err != nil {
+		if err != nil && !tc.expectError {
 			t.Fatalf("Failed to get node info: %v", err)
 		} else {
 			volumeLimit := res.GetMaxVolumesPerNode()