From 32f40c723d6fa5c4c37a91bdfe38b920c6fc56e7 Mon Sep 17 00:00:00 2001 From: Eddie Torres Date: Tue, 12 Dec 2023 02:30:42 +0000 Subject: [PATCH 1/2] Bugfix: instances listed under maxVolumeLimits not taking into account ENIs/Instance storage Signed-off-by: Eddie Torres --- pkg/driver/node.go | 10 ++++------ pkg/driver/node_test.go | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/driver/node.go b/pkg/driver/node.go index c2753e8b67..ee8463da7d 100644 --- 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 index 397c931e48..e35c436a3a 100644 --- 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,17 @@ 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, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From 0862ff10368630ad75493965565c6dfeb4aadcc3 Mon Sep 17 00:00:00 2001 From: Eddie Torres Date: Tue, 12 Dec 2023 02:32:55 +0000 Subject: [PATCH 2/2] Fix typo in maxVolumeLimits map Signed-off-by: Eddie Torres --- pkg/cloud/volume_limits.go | 2 +- pkg/driver/node_test.go | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/cloud/volume_limits.go b/pkg/cloud/volume_limits.go index 1775353b87..6f1e1f8c60 100644 --- 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_test.go b/pkg/driver/node_test.go index e35c436a3a..1767fd965f 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -2207,6 +2207,17 @@ func TestNodeGetInfo(t *testing.T) { 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) {