Merge pull request #1860 from torredil/bugfix-volume_limits

Bugfix: Instances listed under `maxVolumeLimits` not taking into account ENIs/Instance storage
kubernetes-sigs · Dec 14, 2023 · b5010a2 · b5010a2
2 parents d753d69 + 0862ff1
commit b5010a2
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 10 deletions.
diff --git a/pkg/cloud/volume_limits.go b/pkg/cloud/volume_limits.go
@@ -78,7 +78,7 @@ func GetMaxAttachments(nitro bool) int {
 // / https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/volume_limits.html
 var maxVolumeLimits = map[string]int{
 	"d3.8xlarge":    3,
-	"d3.12xlarge":   3,
+	"d3en.12xlarge": 3,
 	"g5.48xlarge":   9,
 	"inf1.xlarge":   26,
 	"inf1.2xlarge":  26,

diff --git a/pkg/driver/node.go b/pkg/driver/node.go
@@ -779,7 +779,10 @@ func (d *nodeService) getVolumesLimit() int64 {
 	availableAttachments := cloud.GetMaxAttachments(isNitro)
 	blockVolumes := d.metadata.GetNumBlockDeviceMappings()
 	dedicatedLimit := cloud.GetDedicatedLimitForInstanceType(instanceType)
-
+	maxEBSAttachments, ok := cloud.GetEBSLimitForInstanceType(instanceType)
+	if ok {
+		availableAttachments = min(maxEBSAttachments, availableAttachments)
+	}
 	// For special dedicated limit instance types, the limit is only for EBS volumes
 	// For (all other) Nitro instances, attachments are shared between EBS volumes, ENIs and NVMe instance stores
 	if dedicatedLimit != 0 {
@@ -794,11 +797,6 @@ func (d *nodeService) getVolumesLimit() int64 {
 		availableAttachments = 1
 	}
 
-	maxEBSAttachments, ok := cloud.GetEBSLimitForInstanceType(instanceType)
-	if ok {
-		availableAttachments = min(maxEBSAttachments, availableAttachments)
-	}
-
 	return int64(availableAttachments)
 }
 

diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go
@@ -2112,7 +2112,7 @@ func TestNodeGetInfo(t *testing.T) {
 			region:            "us-west-2",
 			volumeAttachLimit: -1,
 			attachedENIs:      1,
-			expMaxVolumes:     19,
+			expMaxVolumes:     17,
 			outpostArn:        emptyOutpostArn,
 		},
 		{
@@ -2123,7 +2123,7 @@ func TestNodeGetInfo(t *testing.T) {
 			region:            "us-west-2",
 			volumeAttachLimit: -1,
 			attachedENIs:      1,
-			expMaxVolumes:     16,
+			expMaxVolumes:     14,
 			outpostArn:        emptyOutpostArn,
 		},
 		{
@@ -2134,7 +2134,7 @@ func TestNodeGetInfo(t *testing.T) {
 			region:            "us-west-2",
 			volumeAttachLimit: -1,
 			attachedENIs:      1,
-			expMaxVolumes:     11,
+			expMaxVolumes:     9,
 			outpostArn:        emptyOutpostArn,
 		},
 		{
@@ -2196,6 +2196,28 @@ func TestNodeGetInfo(t *testing.T) {
 			expMaxVolumes:     127, // 128 (max) - 1 (root)
 			outpostArn:        emptyOutpostArn,
 		},
+		{
+			name:              "d3.8xlarge instance max EBS attachment limit",
+			instanceID:        "i-123456789abcdef01",
+			instanceType:      "d3.8xlarge",
+			availabilityZone:  "us-west-2b",
+			region:            "us-west-2",
+			volumeAttachLimit: -1,
+			attachedENIs:      1,
+			expMaxVolumes:     1,
+			outpostArn:        emptyOutpostArn,
+		},
+		{
+			name:              "d3en.12xlarge instance max EBS attachment limit",
+			instanceID:        "i-123456789abcdef01",
+			instanceType:      "d3en.12xlarge",
+			availabilityZone:  "us-west-2b",
+			region:            "us-west-2",
+			volumeAttachLimit: -1,
+			attachedENIs:      1,
+			expMaxVolumes:     1,
+			outpostArn:        emptyOutpostArn,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {