From ac5d95882824ca0b936cc04f4eb72cbdf6086372 Mon Sep 17 00:00:00 2001 From: Rishabh Singhvi Date: Wed, 9 Feb 2022 15:40:30 +0000 Subject: [PATCH] UPSTREAM: 1175: Fix nodeService.getVolumesLimit() adding more instance This commit fixes the `getVolumesLimit` function. Previously, the volume limit for Nitro instances was based off of a regex. This regex is insufficient for the wide variety of Nitro instances that AWS supports today. Nitro instances share attachments with ENIs and NVMe instance stores. The number of attached ENIs is queried through IMDS, against the `network/interfaces/macs` endpoint. The number of already attached block devices is also queried through IMDS, against the `block-device-mapping` endpoint. Currently, IMDS does not provide a way to query NVMe instance store volumes. So, all instances that support NVMe instance stores are listed in `pkg/cloud/volume_limits.go` This commit also added more tests to cover various scenarios. --- pkg/cloud/metadata.go | 32 +++- pkg/cloud/metadata_ec2.go | 27 ++- pkg/cloud/metadata_interface.go | 2 + pkg/cloud/metadata_k8s.go | 10 +- pkg/cloud/metadata_test.go | 56 ++++++- pkg/cloud/mock_metadata.go | 28 ++++ pkg/cloud/volume_limits.go | 283 ++++++++++++++++++++++++++++++++ pkg/driver/node.go | 31 +++- pkg/driver/node_test.go | 73 +++++++- 9 files changed, 517 insertions(+), 25 deletions(-) create mode 100644 pkg/cloud/volume_limits.go diff --git a/pkg/cloud/metadata.go b/pkg/cloud/metadata.go index bd737d5a56..2c3342805e 100644 --- a/pkg/cloud/metadata.go +++ b/pkg/cloud/metadata.go @@ -26,15 +26,25 @@ import ( // Metadata is info about the ec2 instance on which the driver is running type Metadata struct { - InstanceID string - InstanceType string - Region string - AvailabilityZone string - OutpostArn arn.ARN + InstanceID string + InstanceType string + Region string + AvailabilityZone string + NumAttachedENIs int + NumBlockDeviceMappings int + OutpostArn arn.ARN } -// OutpostArnEndpoint is the ec2 instance metadata endpoint to query to get the outpost arn -const OutpostArnEndpoint string = "outpost-arn" +const ( + // OutpostArnEndpoint is the ec2 instance metadata endpoint to query to get the outpost arn + outpostArnEndpoint string = "outpost-arn" + + // enisEndpoint is the ec2 instance metadata endpoint to query the number of attached ENIs + enisEndpoint string = "network/interfaces/macs" + + // blockDevicesEndpoint is the ec2 instance metadata endpoint to query the number of attached block devices + blockDevicesEndpoint string = "block-device-mapping" +) var _ MetadataService = &Metadata{} @@ -58,6 +68,14 @@ func (m *Metadata) GetAvailabilityZone() string { return m.AvailabilityZone } +func (m *Metadata) GetNumAttachedENIs() int { + return m.NumAttachedENIs +} + +func (m *Metadata) GetNumBlockDeviceMappings() int { + return m.NumBlockDeviceMappings +} + // GetOutpostArn returns outpost arn if instance is running on an outpost. empty otherwise. func (m *Metadata) GetOutpostArn() arn.ARN { return m.OutpostArn diff --git a/pkg/cloud/metadata_ec2.go b/pkg/cloud/metadata_ec2.go index f64f32f2e2..44f814bc47 100644 --- a/pkg/cloud/metadata_ec2.go +++ b/pkg/cloud/metadata_ec2.go @@ -41,14 +41,31 @@ func EC2MetadataInstanceInfo(svc EC2Metadata) (*Metadata, error) { return nil, fmt.Errorf("could not get valid EC2 availability zone") } + enis, err := svc.GetMetadata(enisEndpoint) + // the ENIs should not be empty; if (somehow) it is empty, return an error + if enis == "" || err != nil { + return nil, fmt.Errorf("could not get number of attached ENIs") + } + + attachedENIs := strings.Count(enis, "\n") + 1 + + mappings, err := svc.GetMetadata(blockDevicesEndpoint) + if err != nil { + return nil, fmt.Errorf("could not get number of block device mappings") + } + // The output contains 1 volume for the AMI. Any other block device contributes to the attachment limit + blockDevMappings := strings.Count(mappings, "\n") + instanceInfo := Metadata{ - InstanceID: doc.InstanceID, - InstanceType: doc.InstanceType, - Region: doc.Region, - AvailabilityZone: doc.AvailabilityZone, + InstanceID: doc.InstanceID, + InstanceType: doc.InstanceType, + Region: doc.Region, + AvailabilityZone: doc.AvailabilityZone, + NumAttachedENIs: attachedENIs, + NumBlockDeviceMappings: blockDevMappings, } - outpostArn, err := svc.GetMetadata(OutpostArnEndpoint) + outpostArn, err := svc.GetMetadata(outpostArnEndpoint) // "outpust-arn" returns 404 for non-outpost instances. note that the request is made to a link-local address. // it's guaranteed to be in the form `arn::outposts:::outpost/` // There's a case to be made here to ignore the error so a failure here wouldn't affect non-outpost calls. diff --git a/pkg/cloud/metadata_interface.go b/pkg/cloud/metadata_interface.go index 005a1ad7e4..f98c22a832 100644 --- a/pkg/cloud/metadata_interface.go +++ b/pkg/cloud/metadata_interface.go @@ -11,6 +11,8 @@ type MetadataService interface { GetInstanceType() string GetRegion() string GetAvailabilityZone() string + GetNumAttachedENIs() int + GetNumBlockDeviceMappings() int GetOutpostArn() arn.ARN } diff --git a/pkg/cloud/metadata_k8s.go b/pkg/cloud/metadata_k8s.go index dcfaac2bba..3e182a5ab2 100644 --- a/pkg/cloud/metadata_k8s.go +++ b/pkg/cloud/metadata_k8s.go @@ -73,10 +73,12 @@ func KubernetesAPIInstanceInfo(clientset kubernetes.Interface) (*Metadata, error } instanceInfo := Metadata{ - InstanceID: instanceID, - InstanceType: instanceType, - Region: region, - AvailabilityZone: availabilityZone, + InstanceID: instanceID, + InstanceType: instanceType, + Region: region, + AvailabilityZone: availabilityZone, + NumAttachedENIs: 1, // All nodes have at least 1 attached ENI, so we'll use that + NumBlockDeviceMappings: 0, } return &instanceInfo, nil diff --git a/pkg/cloud/metadata_test.go b/pkg/cloud/metadata_test.go index 9c645d7605..143ee78dcc 100644 --- a/pkg/cloud/metadata_test.go +++ b/pkg/cloud/metadata_test.go @@ -55,6 +55,10 @@ func TestNewMetadataService(t *testing.T) { invalidInstanceIdentityDocument bool getMetadataValue string getMetadataError error + imdsBlockDeviceOutput string + imdsENIOutput string + expectedENIs int + expectedBlockDevices int expectedOutpostArn arn.ARN expectedErr error node v1.Node @@ -69,6 +73,8 @@ func TestNewMetadataService(t *testing.T) { Region: stdRegion, AvailabilityZone: stdAvailabilityZone, }, + imdsENIOutput: "00:00:00:00:00:00", + expectedENIs: 1, }, { name: "success: outpost-arn is available", @@ -81,6 +87,8 @@ func TestNewMetadataService(t *testing.T) { }, getMetadataValue: validRawOutpostArn, expectedOutpostArn: validOutpostArn, + imdsENIOutput: "00:00:00:00:00:00", + expectedENIs: 1, }, { name: "success: outpost-arn is invalid", @@ -92,6 +100,8 @@ func TestNewMetadataService(t *testing.T) { AvailabilityZone: stdAvailabilityZone, }, getMetadataValue: "foo", + imdsENIOutput: "00:00:00:00:00:00", + expectedENIs: 1, }, { name: "success: outpost-arn is not found", @@ -103,6 +113,8 @@ func TestNewMetadataService(t *testing.T) { AvailabilityZone: stdAvailabilityZone, }, getMetadataError: fmt.Errorf("404"), + imdsENIOutput: "00:00:00:00:00:00", + expectedENIs: 1, }, { name: "success: metadata not available, used k8s api", @@ -123,6 +135,7 @@ func TestNewMetadataService(t *testing.T) { }, Status: v1.NodeStatus{}, }, + expectedENIs: 1, nodeNameEnvVar: nodeName, }, { @@ -270,9 +283,37 @@ func TestNewMetadataService(t *testing.T) { Region: stdRegion, AvailabilityZone: stdAvailabilityZone, }, + imdsENIOutput: "00:00:00:00:00:00", + expectedENIs: 1, getMetadataError: fmt.Errorf("405"), expectedErr: fmt.Errorf("something went wrong while getting EC2 outpost arn: 405"), }, + { + name: "success: GetMetadata() returns correct number of ENIs", + ec2metadataAvailable: true, + getInstanceIdentityDocumentValue: ec2metadata.EC2InstanceIdentityDocument{ + InstanceID: stdInstanceID, + InstanceType: stdInstanceType, + Region: stdRegion, + AvailabilityZone: stdAvailabilityZone, + }, + imdsENIOutput: "00:00:00:00:00:00\n00:00:00:00:00:01", + expectedENIs: 2, + }, + { + name: "success: GetMetadata() returns correct number of block device mappings", + ec2metadataAvailable: true, + getInstanceIdentityDocumentValue: ec2metadata.EC2InstanceIdentityDocument{ + InstanceID: stdInstanceID, + InstanceType: stdInstanceType, + Region: stdRegion, + AvailabilityZone: stdAvailabilityZone, + }, + imdsENIOutput: "00:00:00:00:00:00", + expectedENIs: 1, + imdsBlockDeviceOutput: "ami\nroot\nebs1\nebs2", + expectedBlockDevices: 3, + }, } for _, tc := range testCases { @@ -297,12 +338,17 @@ func TestNewMetadataService(t *testing.T) { // GetInstanceIdentityDocument returns an error or (somehow?) partial // output if tc.getInstanceIdentityDocumentError == nil && !tc.invalidInstanceIdentityDocument { + mockEC2Metadata.EXPECT().GetMetadata(enisEndpoint).Return(tc.imdsENIOutput, nil) + mockEC2Metadata.EXPECT().GetMetadata(blockDevicesEndpoint).Return(tc.imdsBlockDeviceOutput, nil) + if tc.getMetadataValue != "" || tc.getMetadataError != nil { - mockEC2Metadata.EXPECT().GetMetadata(OutpostArnEndpoint).Return(tc.getMetadataValue, tc.getMetadataError) + mockEC2Metadata.EXPECT().GetMetadata(outpostArnEndpoint).Return(tc.getMetadataValue, tc.getMetadataError) + } else { - mockEC2Metadata.EXPECT().GetMetadata(OutpostArnEndpoint).Return("", fmt.Errorf("404")) + mockEC2Metadata.EXPECT().GetMetadata(outpostArnEndpoint).Return("", fmt.Errorf("404")) } } + if clientsetInitialized == true { t.Errorf("kubernetes client was unexpectedly initialized when metadata is available!") if len(clientset.Actions()) > 0 { @@ -339,6 +385,12 @@ func TestNewMetadataService(t *testing.T) { if m.GetOutpostArn() != tc.expectedOutpostArn { t.Errorf("GetOutpostArn() failed: got %v, expected %v", m.GetOutpostArn(), tc.expectedOutpostArn) } + if m.GetNumAttachedENIs() != tc.expectedENIs { + t.Errorf("GetMetadata() failed for %s: got %v, expected %v", enisEndpoint, m.GetNumAttachedENIs(), tc.expectedENIs) + } + if m.GetNumBlockDeviceMappings() != tc.expectedBlockDevices { + t.Errorf("GetMetadata() failed for %s: got %v, expected %v", blockDevicesEndpoint, m.GetNumBlockDeviceMappings(), tc.expectedBlockDevices) + } } mockCtrl.Finish() }) diff --git a/pkg/cloud/mock_metadata.go b/pkg/cloud/mock_metadata.go index be2d1b219d..7eab9d0998 100644 --- a/pkg/cloud/mock_metadata.go +++ b/pkg/cloud/mock_metadata.go @@ -77,6 +77,34 @@ func (mr *MockMetadataServiceMockRecorder) GetInstanceType() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetInstanceType", reflect.TypeOf((*MockMetadataService)(nil).GetInstanceType)) } +// GetNumAttachedENIs mocks base method. +func (m *MockMetadataService) GetNumAttachedENIs() int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNumAttachedENIs") + ret0, _ := ret[0].(int) + return ret0 +} + +// GetNumAttachedENIs indicates an expected call of GetNumAttachedENIs. +func (mr *MockMetadataServiceMockRecorder) GetNumAttachedENIs() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNumAttachedENIs", reflect.TypeOf((*MockMetadataService)(nil).GetNumAttachedENIs)) +} + +// GetNumBlockDeviceMappings mocks base method. +func (m *MockMetadataService) GetNumBlockDeviceMappings() int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNumBlockDeviceMappings") + ret0, _ := ret[0].(int) + return ret0 +} + +// GetNumBlockDeviceMappings indicates an expected call of GetNumBlockDeviceMappings. +func (mr *MockMetadataServiceMockRecorder) GetNumBlockDeviceMappings() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNumBlockDeviceMappings", reflect.TypeOf((*MockMetadataService)(nil).GetNumBlockDeviceMappings)) +} + // GetOutpostArn mocks base method. func (m *MockMetadataService) GetOutpostArn() arn.ARN { m.ctrl.T.Helper() diff --git a/pkg/cloud/volume_limits.go b/pkg/cloud/volume_limits.go new file mode 100644 index 0000000000..90409c0f35 --- /dev/null +++ b/pkg/cloud/volume_limits.go @@ -0,0 +1,283 @@ +package cloud + +import ( + "regexp" + "strings" +) + +/// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances +const ( + highMemoryMetalInstancesMaxVolumes = 19 + highMemoryVirtualInstancesMaxVolumes = 27 + baremetalMaxVolumes = 31 + nonNitroMaxAttachments = 39 + nitroMaxAttachments = 28 +) + +/// It is possible to have an instance family where the virtualized instances are Nitro +/// and metal instances are not +var nonNitroInstances = map[string]struct{}{ + "c6i.metal": {}, + "g5g.metal": {}, +} + +/// List of nitro instance types can be found here: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances +var nonNitroInstanceFamilies = map[string]struct{}{ + "t2": {}, + "c7g": {}, + "c4": {}, + "r4": {}, + "x2idn": {}, + "x2iedn": {}, + "x2iezn": {}, + "x1e": {}, + "x1": {}, + "p2": {}, + "tr1n": {}, + "g4dn": {}, + "g3": {}, + "d2": {}, + "h1": {}, +} + +func IsNitroInstanceType(it string) bool { + if _, ok := nonNitroInstances[it]; ok { + return false + } + + strs := strings.Split(it, ".") + + if len(strs) != 2 { + panic("cannot determine family of instance type") + } + + family := strs[0] + _, ok := nonNitroInstanceFamilies[family] + return !ok +} + +func GetMaxAttachments(nitro bool) int { + if nitro { + return nitroMaxAttachments + } + return nonNitroMaxAttachments +} + +/// Some instance types have a maximum limit of EBS volumes +/// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances +var maxVolumeLimits = map[string]int{ + "d3.8xlarge": 3, + "d3.12xlarge": 3, + "inf1.xlarge": 26, + "inf1.2xlarge": 26, + "inf1.6xlarge": 23, + "inf1.24xlarge": 11, + "mac1.metal": 16, +} + +func GetEBSLimitForInstanceType(it string) (int, bool) { + if v, ok := maxVolumeLimits[it]; ok { + return v, ok + } + + highMemoryMetalRegex := `^u-[a-z0-9]+\.metal$` + re := regexp.MustCompile(highMemoryMetalRegex) + + if ok := re.MatchString(it); ok { + return highMemoryMetalInstancesMaxVolumes, true + } + + highMemoryVirtualRegex := `^u-[a-z0-9]+\.[a-z0-9]+` + re = regexp.MustCompile(highMemoryVirtualRegex) + + if ok := re.MatchString(it); ok { + return highMemoryVirtualInstancesMaxVolumes, true + } + + bareMetalRegex := `[a-z0-9]+\.metal$` + re = regexp.MustCompile(bareMetalRegex) + + if ok := re.MatchString(it); ok { + return baremetalMaxVolumes, true + } + + return 0, false +} + +func GetNVMeInstanceStoreVolumesForInstanceType(it string) int { + if v, ok := nvmeInstanceStoreVolumes[it]; ok { + return v + } + return 0 +} + +/// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/InstanceStorage.html#instance-store-volumes +/// IMDS does not provide NVMe instance store data; we'll just list all instances here +/// TODO: See if we can get these values from DescribeInstanceTypes API +var nvmeInstanceStoreVolumes = map[string]int{ + "c5ad.large": 1, + "c5ad.xlarge": 1, + "c5ad.2xlarge": 1, + "c5ad.4xlarge": 2, + "c5ad.8xlarge": 2, + "c5ad.12xlarge": 2, + "c5ad.16xlarge": 2, + "c5ad.24xlarge": 2, + "c5d.large": 1, + "c5d.xlarge": 1, + "c5d.2xlarge": 1, + "c5d.4xlarge": 1, + "c5d.9xlarge": 1, + "c5d.12xlarge": 2, + "c5d.18xlarge": 2, + "c5d.24xlarge": 4, + "c5d.metal": 4, + "c6gd.medium": 1, + "c6gd.large": 1, + "c6gd.xlarge": 1, + "c6gd.2xlarge": 1, + "c6gd.4xlarge": 1, + "c6gd.8xlarge": 1, + "c6gd.12xlarge": 2, + "c6gd.16xlarge": 2, + "c6gd.metal": 2, + "dl1.24xlarge": 4, + "f1.2xlarge": 1, + "f1.4xlarge": 1, + "f1.16xlarge": 4, + "g4ad.xlarge": 1, + "g4ad.2xlarge": 1, + "g4ad.4xlarge": 1, + "g4ad.8xlarge": 1, + "g4ad.16xlarge": 2, + "g4dn.xlarge": 1, + "g4dn.2xlarge": 1, + "g4dn.4xlarge": 1, + "g4dn.8xlarge": 1, + "g4dn.12xlarge": 1, + "g4dn.16xlarge": 1, + "g4dn.metal": 2, + "g5.xlarge": 1, + "g5.2xlarge": 1, + "g5.4xlarge": 1, + "g5.8xlarge": 1, + "g5.12xlarge": 1, + "g5.16xlarge": 1, + "g5.24xlarge": 1, + "g5.48xlarge": 2, + "i3.large": 1, + "i3.xlarge": 1, + "i3.2xlarge": 1, + "i3.4xlarge": 2, + "i3.8xlarge": 4, + "i3.16xlarge": 8, + "i3.metal": 8, + "i3en.large": 1, + "i3en.xlarge": 1, + "i3en.2xlarge": 2, + "i3en.3xlarge": 1, + "i3en.6xlarge": 2, + "i3en.12xlarge": 4, + "i3en.24xlarge": 8, + "i3en.metal": 8, + "im4gn.large": 1, + "im4gn.xlarge": 1, + "im4gn.2xlarge": 1, + "im4gn.4xlarge": 1, + "im4gn.8xlarge": 2, + "im4gn.16xlarge": 4, + "is4gen.medium": 1, + "is4gen.large": 1, + "is4gen.xlarge": 1, + "is4gen.2xlarge": 1, + "is4gen.4xlarge": 2, + "is4gen.8xlarge": 4, + "m5ad.large": 1, + "m5ad.xlarge": 1, + "m5ad.2xlarge": 1, + "m5ad.4xlarge": 2, + "m5ad.8xlarge": 2, + "m5ad.12xlarge": 2, + "m5ad.16xlarge": 4, + "m5ad.24xlarge": 4, + "m5d.large": 1, + "m5d.xlarge": 1, + "m5d.2xlarge": 1, + "m5d.4xlarge": 2, + "m5d.8xlarge": 2, + "m5d.12xlarge": 2, + "m5d.16xlarge": 4, + "m5d.24xlarge": 4, + "m5d.metal": 4, + "m5dn.large": 1, + "m5dn.xlarge": 1, + "m5dn.2xlarge": 1, + "m5dn.4xlarge": 2, + "m5dn.8xlarge": 2, + "m5dn.12xlarge": 2, + "m5dn.16xlarge": 4, + "m5dn.24xlarge": 4, + "m5dn.metal": 4, + "m6gd.medium": 1, + "m6gd.large": 1, + "m6gd.xlarge": 1, + "m6gd.2xlarge": 1, + "m6gd.4xlarge": 1, + "m6gd.8xlarge": 1, + "m6gd.12xlarge": 2, + "m6gd.16xlarge": 2, + "m6gd.metal": 2, + "p3dn.24xlarge": 2, + "p4d.24xlarge": 8, + "r5ad.large": 1, + "r5ad.xlarge": 1, + "r5ad.2xlarge": 1, + "r5ad.4xlarge": 2, + "r5ad.8xlarge": 2, + "r5ad.12xlarge": 2, + "r5ad.16xlarge": 4, + "r5ad.24xlarge": 4, + "r5d.large": 1, + "r5d.xlarge": 1, + "r5d.2xlarge": 1, + "r5d.4xlarge": 2, + "r5d.8xlarge": 2, + "r5d.12xlarge": 2, + "r5d.16xlarge": 4, + "r5d.24xlarge": 4, + "r5d.metal": 4, + "r5dn.large": 1, + "r5dn.xlarge": 1, + "r5dn.2xlarge": 1, + "r5dn.4xlarge": 2, + "r5dn.8xlarge": 2, + "r5dn.12xlarge": 2, + "r5dn.16xlarge": 4, + "r5dn.24xlarge": 4, + "r5dn.metal": 4, + "r6gd.medium": 1, + "r6gd.large": 1, + "r6gd.xlarge": 1, + "r6gd.2xlarge": 1, + "r6gd.4xlarge": 1, + "r6gd.8xlarge": 1, + "r6gd.12xlarge": 2, + "r6gd.16xlarge": 2, + "r6gd.metal": 2, + "x2gd.medium": 1, + "x2gd.large": 1, + "x2gd.xlarge": 1, + "x2gd.2xlarge": 1, + "x2gd.4xlarge": 1, + "x2gd.8xlarge": 1, + "x2gd.12xlarge": 2, + "x2gd.16xlarge": 2, + "x2gd.metal": 2, + "z1d.large": 1, + "z1d.xlarge": 1, + "z1d.2xlarge": 1, + "z1d.3xlarge": 1, + "z1d.6xlarge": 1, + "z1d.12xlarge": 2, + "z1d.metal": 2, +} diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 90403b30c1..1717639a96 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -21,7 +21,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "strings" csi "github.com/container-storage-interface/spec/lib/go/csi" @@ -693,12 +692,34 @@ func (d *nodeService) getVolumesLimit() int64 { if d.driverOptions.volumeAttachLimit >= 0 { return d.driverOptions.volumeAttachLimit } - ebsNitroInstanceTypeRegex := "^[cmr]5.*|t3|z1d" + instanceType := d.metadata.GetInstanceType() - if ok, _ := regexp.MatchString(ebsNitroInstanceTypeRegex, instanceType); ok { - return defaultMaxEBSNitroVolumes + + isNitro := cloud.IsNitroInstanceType(instanceType) + availableAttachments := cloud.GetMaxAttachments(isNitro) + blockVolumes := d.metadata.GetNumBlockDeviceMappings() + + // For Nitro instances, attachments are shared between EBS volumes, ENIs and NVMe instance stores + if isNitro { + enis := d.metadata.GetNumAttachedENIs() + nvmeInstanceStoreVolumes := cloud.GetNVMeInstanceStoreVolumesForInstanceType(instanceType) + availableAttachments = availableAttachments - enis - blockVolumes - nvmeInstanceStoreVolumes + } else { + availableAttachments -= blockVolumes + } + maxEBSAttachments, ok := cloud.GetEBSLimitForInstanceType(instanceType) + if ok { + availableAttachments = min(maxEBSAttachments, availableAttachments) + } + + return int64(availableAttachments) +} + +func min(x, y int) int { + if x <= y { + return x } - return defaultMaxEBSVolumes + return y } // hasMountOption returns a boolean indicating whether the given diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index e075b966b3..819cf27349 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -1907,17 +1907,20 @@ func TestNodeGetInfo(t *testing.T) { instanceID string instanceType string availabilityZone string + attachedENIs int + blockDevices int volumeAttachLimit int64 expMaxVolumes int64 outpostArn arn.ARN }{ { - name: "success normal", + name: "non-nitro instance success normal", instanceID: "i-123456789abcdef01", instanceType: "t2.medium", availabilityZone: "us-west-2b", volumeAttachLimit: -1, expMaxVolumes: 39, + attachedENIs: 1, outpostArn: emptyOutpostArn, }, { @@ -1930,11 +1933,22 @@ func TestNodeGetInfo(t *testing.T) { outpostArn: emptyOutpostArn, }, { - name: "success normal with NVMe", + name: "nitro instance success normal", + instanceID: "i-123456789abcdef01", + instanceType: "t3.xlarge", + availabilityZone: "us-west-2b", + volumeAttachLimit: -1, + attachedENIs: 2, + expMaxVolumes: 26, // 28 (max) - 2 (enis) + outpostArn: emptyOutpostArn, + }, + { + name: "nitro instance success normal with NVMe", instanceID: "i-123456789abcdef01", instanceType: "m5d.large", availabilityZone: "us-west-2b", volumeAttachLimit: -1, + attachedENIs: 2, expMaxVolumes: 25, outpostArn: emptyOutpostArn, }, @@ -1956,6 +1970,57 @@ func TestNodeGetInfo(t *testing.T) { expMaxVolumes: 30, outpostArn: validOutpostArn, }, + { + name: "baremetal instances max EBS attachment limit", + instanceID: "i-123456789abcdef01", + instanceType: "c6i.metal", + availabilityZone: "us-west-2b", + volumeAttachLimit: -1, + attachedENIs: 1, + expMaxVolumes: 31, + outpostArn: emptyOutpostArn, + }, + { + name: "high memory baremetal instances max EBS attachment limit", + instanceID: "i-123456789abcdef01", + instanceType: "u-12tb1.metal", + availabilityZone: "us-west-2b", + volumeAttachLimit: -1, + attachedENIs: 1, + expMaxVolumes: 19, + outpostArn: emptyOutpostArn, + }, + { + name: "mac instances max EBS attachment limit", + instanceID: "i-123456789abcdef01", + instanceType: "mac1.metal", + availabilityZone: "us-west-2b", + volumeAttachLimit: -1, + attachedENIs: 1, + expMaxVolumes: 16, + outpostArn: emptyOutpostArn, + }, + { + name: "inf1.24xlarge instace max EBS attachment limit", + instanceID: "i-123456789abcdef01", + instanceType: "inf1.24xlarge", + availabilityZone: "us-west-2b", + volumeAttachLimit: -1, + attachedENIs: 1, + expMaxVolumes: 11, + outpostArn: emptyOutpostArn, + }, + { + name: "nitro instances already attached EBS volumes", + instanceID: "i-123456789abcdef01", + instanceType: "t3.xlarge", + availabilityZone: "us-west-2b", + volumeAttachLimit: -1, + attachedENIs: 1, + blockDevices: 2, + expMaxVolumes: 25, + outpostArn: emptyOutpostArn, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -1976,6 +2041,10 @@ func TestNodeGetInfo(t *testing.T) { if tc.volumeAttachLimit < 0 { mockMetadata.EXPECT().GetInstanceType().Return(tc.instanceType) + mockMetadata.EXPECT().GetNumBlockDeviceMappings().Return(tc.blockDevices) + if cloud.IsNitroInstanceType(tc.instanceType) { + mockMetadata.EXPECT().GetNumAttachedENIs().Return(tc.attachedENIs) + } } awsDriver := &nodeService{