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

@@ -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

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:<partition>:outposts:<region>:<account>:outpost/<outpost-id>`
 	// There's a case to be made here to ignore the error so a failure here wouldn't affect non-outpost calls.

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
 }
 

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

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()
 		})