Add GetInstanceENIs method and improve ENI creation logging

Implements a new GetInstanceENIs method to retrieve all ENIs attached to an EC2 instance, returning a map of device index to ENI ID. This helps prevent race conditions during ENI creation.

Enhances logging in the NodeENI controller to better track ENI attachment operations, particularly during the subnet processing and device index determination phases.

Updates the NodeENI controller to immediately update status after ENI attachment to prevent race conditions in subsequent reconciliations.

Updates container image tags to the latest version with cleanup fixes.

Author: John Lam

charts/aws-multi-eni-controller/values.yaml

@@ -5,7 +5,7 @@
 # Image configuration
 image:
   repository: johnlam90/aws-multi-eni-controller
-  tag: fix-al2023-dpdk-setup-7101ae5
+  tag: fix-cleanup-case-01-3614101
   pullPolicy: Always
 
 # Namespace to deploy the controller

deploy/deployment.yaml

@@ -173,7 +173,7 @@ spec:
       #   - "kubernetes"
       containers:
       - name: manager
-        image: johnlam90/aws-multi-eni-controller:fix-al2023-dpdk-setup-7101ae5
+        image: johnlam90/aws-multi-eni-controller:fix-cleanup-case-01-3614101
         env:
         - name: COMPONENT
           value: "eni-controller"

deploy/eni-manager-daemonset.yaml

@@ -20,7 +20,7 @@ spec:
         ng: multi-eni
       initContainers:
       - name: dpdk-setup
-        image: johnlam90/aws-multi-eni-controller:fix-al2023-dpdk-setup-7101ae5
+        image: johnlam90/aws-multi-eni-controller:fix-cleanup-case-01-3614101
         command: ["/bin/sh", "-c"]
         args:
         - |
@@ -180,7 +180,7 @@ spec:
           readOnly: true
       containers:
       - name: eni-manager
-        image: johnlam90/aws-multi-eni-controller:fix-al2023-dpdk-setup-7101ae5
+        image: johnlam90/aws-multi-eni-controller:fix-cleanup-case-01-3614101
         env:
         - name: COMPONENT
           value: "eni-manager"

pkg/aws/ec2.go

@@ -577,6 +577,73 @@ func (c *EC2Client) DescribeInstance(ctx context.Context, instanceID string) (*E
 	return ec2Instance, nil
 }
 
+// GetInstanceENIs gets all ENIs attached to an instance
+func (c *EC2Client) GetInstanceENIs(ctx context.Context, instanceID string) (map[int]string, error) {
+	log := c.Logger.WithValues("instanceID", instanceID)
+	log.V(1).Info("Getting all ENIs attached to instance")
+
+	// Use DescribeNetworkInterfaces with a filter to get all ENIs attached to this instance
+	input := &ec2.DescribeNetworkInterfacesInput{
+		Filters: []types.Filter{
+			{
+				Name:   aws.String("attachment.instance-id"),
+				Values: []string{instanceID},
+			},
+			{
+				Name:   aws.String("attachment.status"),
+				Values: []string{"attached"},
+			},
+		},
+	}
+
+	// Use exponential backoff for API rate limiting
+	backoff := wait.Backoff{
+		Steps:    5,
+		Duration: 1 * time.Second,
+		Factor:   2.0,
+		Jitter:   0.1,
+	}
+
+	var result *ec2.DescribeNetworkInterfacesOutput
+	var lastErr error
+	err := wait.ExponentialBackoff(backoff, func() (bool, error) {
+		var err error
+		result, err = c.EC2.DescribeNetworkInterfaces(ctx, input)
+		if err != nil {
+			// Check if this is a rate limit error
+			if strings.Contains(err.Error(), "RequestLimitExceeded") ||
+				strings.Contains(err.Error(), "Throttling") {
+				log.Info("AWS API rate limit exceeded, retrying", "error", err.Error())
+				lastErr = err
+				return false, nil // Return nil error to continue retrying
+			}
+
+			// For other errors, fail immediately
+			lastErr = err
+			return false, err
+		}
+		return true, nil
+	})
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to describe network interfaces after retries: %v", lastErr)
+	}
+
+	// Build a map of device index to ENI ID
+	eniMap := make(map[int]string)
+	for _, eni := range result.NetworkInterfaces {
+		if eni.Attachment != nil && eni.Attachment.DeviceIndex != nil {
+			deviceIndex := int(*eni.Attachment.DeviceIndex)
+			eniID := *eni.NetworkInterfaceId
+			eniMap[deviceIndex] = eniID
+			log.V(1).Info("Found attached ENI", "eniID", eniID, "deviceIndex", deviceIndex)
+		}
+	}
+
+	log.Info("Retrieved all ENIs attached to instance", "count", len(eniMap), "eniMap", eniMap)
+	return eniMap, nil
+}
+
 // GetSubnetIDByName looks up a subnet ID by its Name tag
 func (c *EC2Client) GetSubnetIDByName(ctx context.Context, subnetName string) (string, error) {
 	log := c.Logger.WithValues("subnetName", subnetName)

pkg/aws/ec2_client.go

@@ -104,3 +104,8 @@ func (c *EC2ClientFacade) GetSecurityGroupIDByName(ctx context.Context, security
 func (c *EC2ClientFacade) DescribeInstance(ctx context.Context, instanceID string) (*EC2Instance, error) {
 	return c.instanceDescriber.DescribeInstance(ctx, instanceID)
 }
+
+// GetInstanceENIs delegates to InstanceDescriber
+func (c *EC2ClientFacade) GetInstanceENIs(ctx context.Context, instanceID string) (map[int]string, error) {
+	return c.instanceDescriber.GetInstanceENIs(ctx, instanceID)
+}

pkg/aws/instance_describer.go

@@ -6,7 +6,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
+	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/util/wait"
 )
@@ -82,3 +84,70 @@ func (d *EC2InstanceDescriber) DescribeInstance(ctx context.Context, instanceID
 	log.V(1).Info("Found EC2 instance", "instanceID", ec2Instance.InstanceID, "state", ec2Instance.State)
 	return ec2Instance, nil
 }
+
+// GetInstanceENIs gets all ENIs attached to an instance
+func (d *EC2InstanceDescriber) GetInstanceENIs(ctx context.Context, instanceID string) (map[int]string, error) {
+	log := d.logger.WithValues("instanceID", instanceID)
+	log.V(1).Info("Getting all ENIs attached to instance")
+
+	// Use DescribeNetworkInterfaces with a filter to get all ENIs attached to this instance
+	input := &ec2.DescribeNetworkInterfacesInput{
+		Filters: []types.Filter{
+			{
+				Name:   aws.String("attachment.instance-id"),
+				Values: []string{instanceID},
+			},
+			{
+				Name:   aws.String("attachment.status"),
+				Values: []string{"attached"},
+			},
+		},
+	}
+
+	// Use exponential backoff for API rate limiting
+	backoff := wait.Backoff{
+		Steps:    5,
+		Duration: 1 * time.Second,
+		Factor:   2.0,
+		Jitter:   0.1,
+	}
+
+	var result *ec2.DescribeNetworkInterfacesOutput
+	var lastErr error
+	err := wait.ExponentialBackoff(backoff, func() (bool, error) {
+		var err error
+		result, err = d.ec2Client.DescribeNetworkInterfaces(ctx, input)
+		if err != nil {
+			// Check if this is a rate limit error
+			if strings.Contains(err.Error(), "RequestLimitExceeded") ||
+				strings.Contains(err.Error(), "Throttling") {
+				log.Info("AWS API rate limit exceeded, retrying", "error", err.Error())
+				lastErr = err
+				return false, nil // Return nil error to continue retrying
+			}
+
+			// For other errors, fail immediately
+			lastErr = err
+			return false, err
+		}
+		return true, nil
+	})
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to describe network interfaces after retries: %v", lastErr)
+	}
+
+	// Build a map of device index to ENI ID
+	eniMap := make(map[int]string)
+	for _, eni := range result.NetworkInterfaces {
+		if eni.Attachment != nil && eni.Attachment.DeviceIndex != nil {
+			deviceIndex := int(*eni.Attachment.DeviceIndex)
+			eniID := *eni.NetworkInterfaceId
+			eniMap[deviceIndex] = eniID
+			log.V(1).Info("Found attached ENI", "eniID", eniID, "deviceIndex", deviceIndex)
+		}
+	}
+
+	log.Info("Retrieved all ENIs attached to instance", "count", len(eniMap), "eniMap", eniMap)
+	return eniMap, nil
+}

pkg/aws/interface.go

@@ -50,6 +50,10 @@ type InstanceDescriber interface {
 	// DescribeInstance describes an EC2 instance
 	// Returns nil, nil if the instance doesn't exist
 	DescribeInstance(ctx context.Context, instanceID string) (*EC2Instance, error)
+
+	// GetInstanceENIs gets all ENIs attached to an instance
+	// Returns a map of device index to ENI ID for all attached ENIs
+	GetInstanceENIs(ctx context.Context, instanceID string) (map[int]string, error)
 }
 
 // EC2Interface defines the combined interface for all EC2 operations using AWS SDK v2

pkg/aws/mock_ec2.go

@@ -345,6 +345,36 @@ func (m *MockEC2Client) DescribeInstance(ctx context.Context, instanceID string)
 	return result, nil
 }
 
+// GetInstanceENIs gets all ENIs attached to an instance in the mock AWS environment
+func (m *MockEC2Client) GetInstanceENIs(ctx context.Context, instanceID string) (map[int]string, error) {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	// Simulate describe delay
+	if m.DescribeWaitTime > 0 {
+		time.Sleep(m.DescribeWaitTime)
+	}
+
+	// Check if we should simulate a failure
+	if m.FailureScenarios["GetInstanceENIs"] {
+		return nil, fmt.Errorf("simulated GetInstanceENIs failure")
+	}
+
+	// Build a map of device index to ENI ID for this instance
+	eniMap := make(map[int]string)
+
+	// Check all ENIs to see which ones are attached to this instance
+	for eniID, eni := range m.ENIs {
+		if eni.Attachment != nil &&
+			eni.Attachment.InstanceID == instanceID &&
+			eni.Attachment.Status == "attached" {
+			eniMap[int(eni.Attachment.DeviceIndex)] = eniID
+		}
+	}
+
+	return eniMap, nil
+}
+
 // GetSubnetIDByName looks up a subnet ID by its Name tag in the mock AWS environment
 func (m *MockEC2Client) GetSubnetIDByName(ctx context.Context, subnetName string) (string, error) {
 	m.mutex.RLock()

pkg/controller/nodeeni_controller.go

@@ -1412,12 +1412,23 @@ func (r *NodeENIReconciler) buildAttachmentMaps(nodeENI *networkingv1alpha1.Node
 	usedDeviceIndices map[int]bool,
 	subnetToDeviceIndex map[string]int,
 ) {
+	log := r.Log.WithValues("nodeeni", nodeENI.Name, "node", nodeName)
+
 	existingSubnets = make(map[string]bool)
 	usedDeviceIndices = make(map[int]bool)
 	subnetToDeviceIndex = make(map[string]int)
 
+	log.Info("Building attachment maps", "totalAttachments", len(nodeENI.Status.Attachments))
+
 	// First pass: build the subnet to device index mapping from existing attachments
 	for _, attachment := range nodeENI.Status.Attachments {
+		log.V(1).Info("Processing attachment",
+			"nodeID", attachment.NodeID,
+			"eniID", attachment.ENIID,
+			"subnetID", attachment.SubnetID,
+			"deviceIndex", attachment.DeviceIndex,
+			"isCurrentNode", attachment.NodeID == nodeName)
+
 		// We want to build a global mapping across all nodes
 		if attachment.SubnetID != "" && attachment.DeviceIndex > 0 {
 			// If we haven't seen this subnet before, or if this device index is lower
@@ -1430,14 +1441,21 @@ func (r *NodeENIReconciler) buildAttachmentMaps(nodeENI *networkingv1alpha1.Node
 		// For this specific node, track which subnets already have ENIs
 		if attachment.NodeID == nodeName && attachment.SubnetID != "" {
 			existingSubnets[attachment.SubnetID] = true
+			log.Info("Found existing ENI for node in subnet", "subnetID", attachment.SubnetID, "eniID", attachment.ENIID)
 		}
 
 		// For this specific node, track which device indices are already in use
 		if attachment.NodeID == nodeName && attachment.DeviceIndex > 0 {
 			usedDeviceIndices[attachment.DeviceIndex] = true
+			log.Info("Found used device index for node", "deviceIndex", attachment.DeviceIndex, "eniID", attachment.ENIID)
 		}
 	}
 
+	log.Info("Built attachment maps",
+		"existingSubnets", existingSubnets,
+		"usedDeviceIndices", usedDeviceIndices,
+		"subnetToDeviceIndex", subnetToDeviceIndex)
+
 	return existingSubnets, usedDeviceIndices, subnetToDeviceIndex
 }
 
@@ -1454,22 +1472,32 @@ func (r *NodeENIReconciler) createMissingENIs(
 ) {
 	log := r.Log.WithValues("nodeeni", nodeENI.Name, "node", node.Name)
 
+	log.Info("Processing subnets for ENI creation",
+		"totalSubnets", len(subnetIDs),
+		"subnetIDs", subnetIDs,
+		"existingSubnets", existingSubnets,
+		"usedDeviceIndices", usedDeviceIndices)
+
 	for i, subnetID := range subnetIDs {
 		// Skip if we already have an ENI in this subnet
 		if existingSubnets[subnetID] {
-			log.Info("Node already has an ENI in this subnet", "subnetID", subnetID)
+			log.Info("Node already has an ENI in this subnet, skipping", "subnetID", subnetID)
 			continue
 		}
 
+		log.Info("Creating ENI for subnet", "subnetID", subnetID, "subnetIndex", i)
+
 		// Determine the device index to use for this subnet
 		deviceIndex := r.determineDeviceIndex(nodeENI, subnetID, i, subnetToDeviceIndex, usedDeviceIndices, log)
 
 		// Create and attach a new ENI for this subnet
 		if err := r.createAndAttachENIForSubnet(ctx, nodeENI, node, instanceID, subnetID, deviceIndex); err != nil {
-			log.Error(err, "Failed to create and attach ENI for subnet", "subnetID", subnetID)
+			log.Error(err, "Failed to create and attach ENI for subnet", "subnetID", subnetID, "deviceIndex", deviceIndex)
 			// Continue with other subnets even if one fails
 			continue
 		}
+
+		log.Info("Successfully created and attached ENI for subnet", "subnetID", subnetID, "deviceIndex", deviceIndex)
 	}
 }
 
@@ -1602,6 +1630,14 @@ func (r *NodeENIReconciler) createAndAttachENIForSubnet(ctx context.Context, nod
 		LastUpdated:  metav1.Now(),
 	})
 
+	// Update the NodeENI status immediately to prevent race conditions
+	// This ensures that subsequent reconciliations see the attachment
+	if err := r.updateNodeENIStatusWithRetry(ctx, nodeENI); err != nil {
+		log.Error(err, "Failed to update NodeENI status after ENI attachment", "eniID", eniID)
+		// Don't return error here as the ENI is already attached successfully
+		// The status will be updated in the next reconciliation
+	}
+
 	r.Recorder.Eventf(nodeENI, corev1.EventTypeNormal, "ENIAttached",
 		"Successfully attached ENI %s to node %s in subnet %s", eniID, node.Name, subnetID)