Merge pull request kubernetes-sigs#213 from gnufied/backport-attachab…

…le-count

OCPBUGS-4185: Fix nodeService.getVolumesLimit() adding more instance

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