fix: paginate DescribeNetworkInterfaces with deep filters

main.go

@@ -107,6 +107,7 @@ func main() {
 	var healthCheckTimeout int
 	var enableWindowsPrefixDelegation bool
 	var region string
+	var vpcID string
 
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080",
 		"The address the metric endpoint binds to.")
@@ -141,6 +142,7 @@ func main() {
 	flag.BoolVar(&enableWindowsPrefixDelegation, "enable-windows-prefix-delegation", false,
 		"Enable the feature flag for Windows prefix delegation")
 	flag.StringVar(&region, "aws-region", "", "The aws region of the k8s cluster")
+	flag.StringVar(&vpcID, "vpc-id", "", "The vpc-id where EKS cluster is deployed")
 
 	flag.Parse()
 
@@ -183,6 +185,11 @@ func main() {
 		os.Exit(1)
 	}
 
+	if vpcID == "" {
+		setupLog.Error(fmt.Errorf("vpc-id is a required parameter"), "unable to start the controller")
+		os.Exit(1)
+	}
+
 	// Profiler disabled by default, to enable set the enableProfiling argument
 	if enableProfiling {
 		// To use the profiler - https://golang.org/pkg/net/http/pprof/
@@ -336,6 +343,7 @@ func main() {
 		EC2Wrapper:  ec2Wrapper,
 		ClusterName: clusterName,
 		Log:         ctrl.Log.WithName("eni cleaner"),
+		VPCID:       vpcID,
 	}).SetupWithManager(ctx, mgr, healthzHandler); err != nil {
 		setupLog.Error(err, "unable to start eni cleaner")
 		os.Exit(1)

pkg/aws/ec2/api/eni_cleanup.go

@@ -34,6 +34,7 @@ type ENICleaner struct {
 	EC2Wrapper  EC2Wrapper
 	ClusterName string
 	Log         logr.Logger
+	VPCID       string
 
 	availableENIs     map[string]struct{}
 	shutdown          bool
@@ -116,61 +117,57 @@ func (e *ENICleaner) cleanUpAvailableENIs() {
 				Values: aws.StringSlice([]string{config.NetworkInterfaceOwnerTagValue,
 					config.NetworkInterfaceOwnerVPCCNITagValue}),
 			},
+			{
+				Name:   aws.String("vpc-id"),
+				Values: []*string{aws.String(e.VPCID)},
+			},
 		},
 	}
 
 	availableENIs := make(map[string]struct{})
 
-	for {
-		describeNetworkInterfaceOp, err := e.EC2Wrapper.DescribeNetworkInterfaces(describeNetworkInterfaceIp)
-		if err != nil {
-			e.Log.Error(err, "failed to describe network interfaces, will retry")
-			return
-		}
-
-		for _, networkInterface := range describeNetworkInterfaceOp.NetworkInterfaces {
-			if _, exists := e.availableENIs[*networkInterface.NetworkInterfaceId]; exists {
-				// Increment promethues metrics for number of leaked ENIs cleaned up
-				if tagIdx := slices.IndexFunc(networkInterface.TagSet, func(tag *ec2.Tag) bool {
-					return *tag.Key == config.NetworkInterfaceOwnerTagKey
-				}); tagIdx != -1 {
-					switch *networkInterface.TagSet[tagIdx].Value {
-					case config.NetworkInterfaceOwnerTagValue:
-						vpcrcLeakedENICleanupCnt.Inc()
-					case config.NetworkInterfaceOwnerVPCCNITagValue:
-						vpcCniLeakedENICleanupCnt.Inc()
-					default:
-						// We will not hit this case as we only filter for above two tag values, adding it for any future use cases
-						e.Log.Info("found available ENI not created by VPC-CNI/VPC-RC")
-					}
-				}
+	networkInterfaces, err := e.EC2Wrapper.DescribeNetworkInterfacesPages(describeNetworkInterfaceIp)
+	if err != nil {
+		e.Log.Error(err, "failed to describe network interfaces, will retry")
+		return
+	}
 
-				// The ENI in available state has been sitting for at least the eni clean up interval and it should
-				// be removed
-				_, err := e.EC2Wrapper.DeleteNetworkInterface(&ec2.DeleteNetworkInterfaceInput{
-					NetworkInterfaceId: networkInterface.NetworkInterfaceId,
-				})
-				if err != nil {
-					// Log and continue, if the ENI is still present it will be cleaned up in next 2 cycles
-					e.Log.Error(err, "failed to delete the dangling network interface",
-						"id", *networkInterface.NetworkInterfaceId)
-					continue
+	for _, networkInterface := range networkInterfaces {
+		if _, exists := e.availableENIs[*networkInterface.NetworkInterfaceId]; exists {
+			// Increment promethues metrics for number of leaked ENIs cleaned up
+			if tagIdx := slices.IndexFunc(networkInterface.TagSet, func(tag *ec2.Tag) bool {
+				return *tag.Key == config.NetworkInterfaceOwnerTagKey
+			}); tagIdx != -1 {
+				switch *networkInterface.TagSet[tagIdx].Value {
+				case config.NetworkInterfaceOwnerTagValue:
+					vpcrcLeakedENICleanupCnt.Inc()
+				case config.NetworkInterfaceOwnerVPCCNITagValue:
+					vpcCniLeakedENICleanupCnt.Inc()
+				default:
+					// We will not hit this case as we only filter for above two tag values, adding it for any future use cases
+					e.Log.Info("found available ENI not created by VPC-CNI/VPC-RC")
 				}
-				e.Log.Info("deleted dangling ENI successfully",
-					"eni id", networkInterface.NetworkInterfaceId)
-			} else {
-				// Seeing the ENI for the first time, add it to the new list of available network interfaces
-				availableENIs[*networkInterface.NetworkInterfaceId] = struct{}{}
-				e.Log.V(1).Info("adding eni to to the map of available ENIs, will be removed if present in "+
-					"next run too", "id", *networkInterface.NetworkInterfaceId)
 			}
-		}
 
-		if describeNetworkInterfaceOp.NextToken == nil {
-			break
+			// The ENI in available state has been sitting for at least the eni clean up interval and it should
+			// be removed
+			_, err := e.EC2Wrapper.DeleteNetworkInterface(&ec2.DeleteNetworkInterfaceInput{
+				NetworkInterfaceId: networkInterface.NetworkInterfaceId,
+			})
+			if err != nil {
+				// Log and continue, if the ENI is still present it will be cleaned up in next 2 cycles
+				e.Log.Error(err, "failed to delete the dangling network interface",
+					"id", *networkInterface.NetworkInterfaceId)
+				continue
+			}
+			e.Log.Info("deleted dangling ENI successfully",
+				"eni id", networkInterface.NetworkInterfaceId)
+		} else {
+			// Seeing the ENI for the first time, add it to the new list of available network interfaces
+			availableENIs[*networkInterface.NetworkInterfaceId] = struct{}{}
+			e.Log.V(1).Info("adding eni to to the map of available ENIs, will be removed if present in "+
+				"next run too", "id", *networkInterface.NetworkInterfaceId)
 		}
-
-		describeNetworkInterfaceIp.NextToken = describeNetworkInterfaceOp.NextToken
 	}
 
 	// Set the available ENIs to the list of ENIs seen in the current cycle

pkg/aws/ec2/api/eni_cleanup_test.go

@@ -37,6 +37,8 @@ var (
 	mockNetworkInterfaceId2 = "eni-000000000000001"
 	mockNetworkInterfaceId3 = "eni-000000000000002"
 
+	mockVPCID = "vpc-0000000000000000"
+
 	mockDescribeNetworkInterfaceIp = &ec2.DescribeNetworkInterfacesInput{
 		Filters: []*ec2.Filter{
 			{
@@ -52,19 +54,19 @@ var (
 				Values: aws.StringSlice([]string{config.NetworkInterfaceOwnerTagValue,
 					config.NetworkInterfaceOwnerVPCCNITagValue}),
 			},
+			{
+				Name:   aws.String("vpc-id"),
+				Values: []*string{aws.String(mockVPCID)},
+			},
 		},
 	}
-	mockDescribeInterfaceOpWith1And2 = &ec2.DescribeNetworkInterfacesOutput{
-		NetworkInterfaces: []*ec2.NetworkInterface{
-			{NetworkInterfaceId: &mockNetworkInterfaceId1},
-			{NetworkInterfaceId: &mockNetworkInterfaceId2},
-		},
+	mockDescribeInterfaceOpWith1And2 = []*ec2.NetworkInterface{
+		{NetworkInterfaceId: &mockNetworkInterfaceId1},
+		{NetworkInterfaceId: &mockNetworkInterfaceId2},
 	}
-	mockDescribeInterfaceOpWith1And3 = &ec2.DescribeNetworkInterfacesOutput{
-		NetworkInterfaces: []*ec2.NetworkInterface{
-			{NetworkInterfaceId: &mockNetworkInterfaceId1},
-			{NetworkInterfaceId: &mockNetworkInterfaceId3},
-		},
+	mockDescribeInterfaceOpWith1And3 = []*ec2.NetworkInterface{
+		{NetworkInterfaceId: &mockNetworkInterfaceId1},
+		{NetworkInterfaceId: &mockNetworkInterfaceId3},
 	}
 )
 
@@ -74,6 +76,7 @@ func getMockENICleaner(ctrl *gomock.Controller) (*ENICleaner, *mock_api.MockEC2W
 		EC2Wrapper:        mockEC2Wrapper,
 		availableENIs:     map[string]struct{}{},
 		Log:               zap.New(zap.UseDevMode(true)),
+		VPCID:             mockVPCID,
 		clusterNameTagKey: mockClusterNameTagKey,
 		ctx:               context.Background(),
 	}, mockEC2Wrapper
@@ -85,10 +88,10 @@ func TestENICleaner_cleanUpAvailableENIs(t *testing.T) {
 
 	gomock.InOrder(
 		// Return network interface 1 and 2 in first cycle
-		mockWrapper.EXPECT().DescribeNetworkInterfaces(mockDescribeNetworkInterfaceIp).
+		mockWrapper.EXPECT().DescribeNetworkInterfacesPages(mockDescribeNetworkInterfaceIp).
 			Return(mockDescribeInterfaceOpWith1And2, nil),
 		// Return network interface 1 and 3 in the second cycle
-		mockWrapper.EXPECT().DescribeNetworkInterfaces(mockDescribeNetworkInterfaceIp).
+		mockWrapper.EXPECT().DescribeNetworkInterfacesPages(mockDescribeNetworkInterfaceIp).
 			Return(mockDescribeInterfaceOpWith1And3, nil),
 		// Expect to delete the network interface 1
 		mockWrapper.EXPECT().DeleteNetworkInterface(

pkg/aws/ec2/api/helper.go

@@ -79,7 +79,7 @@ type EC2APIHelper interface {
 		ipResourceCount *config.IPResourceCount, interfaceType *string) (*ec2.NetworkInterface, error)
 	DeleteNetworkInterface(interfaceId *string) error
 	GetSubnet(subnetId *string) (*ec2.Subnet, error)
-	GetBranchNetworkInterface(trunkID *string) ([]*ec2.NetworkInterface, error)
+	GetBranchNetworkInterface(trunkID *string, subnetID *string) ([]*ec2.NetworkInterface, error)
 	GetInstanceNetworkInterface(instanceId *string) ([]*ec2.InstanceNetworkInterface, error)
 	DescribeNetworkInterfaces(nwInterfaceIds []*string) ([]*ec2.NetworkInterface, error)
 	DescribeTrunkInterfaceAssociation(trunkInterfaceId *string) ([]*ec2.TrunkInterfaceAssociation, error)
@@ -562,43 +562,20 @@ func (h *ec2APIHelper) UnassignIPv4Resources(eniID string, resourceType config.R
 	return err
 }
 
-func (h *ec2APIHelper) GetBranchNetworkInterface(trunkID *string) ([]*ec2.NetworkInterface, error) {
-	filters := []*ec2.Filter{{
-		Name:   aws.String("tag:" + config.TrunkENIIDTag),
-		Values: []*string{trunkID},
-	}}
-
-	describeNetworkInterfacesInput := &ec2.DescribeNetworkInterfacesInput{Filters: filters}
-	var nwInterfaces []*ec2.NetworkInterface
-	for {
-		describeNetworkInterfaceOutput, err := h.ec2Wrapper.DescribeNetworkInterfaces(describeNetworkInterfacesInput)
-		if err != nil {
-			return nil, err
-		}
-
-		if describeNetworkInterfaceOutput == nil || describeNetworkInterfaceOutput.NetworkInterfaces == nil ||
-			len(describeNetworkInterfaceOutput.NetworkInterfaces) == 0 {
-			// No more interface associated with the trunk, return the result
-			break
-		}
-
-		// One or more interface associated with the trunk, return the result
-		for _, nwInterface := range describeNetworkInterfaceOutput.NetworkInterfaces {
-			// Only attach the required details to avoid consuming extra memory
-			nwInterfaces = append(nwInterfaces, &ec2.NetworkInterface{
-				NetworkInterfaceId: nwInterface.NetworkInterfaceId,
-				TagSet:             nwInterface.TagSet,
-			})
-		}
-
-		if describeNetworkInterfaceOutput.NextToken == nil {
-			break
-		}
-
-		describeNetworkInterfacesInput.NextToken = describeNetworkInterfaceOutput.NextToken
+func (h *ec2APIHelper) GetBranchNetworkInterface(trunkID *string, subnetID *string) ([]*ec2.NetworkInterface, error) {
+	filters := []*ec2.Filter{
+		{
+			Name:   aws.String("tag:" + config.TrunkENIIDTag),
+			Values: []*string{trunkID},
+		},
+		{
+			Name:   aws.String("subnet-id"),
+			Values: []*string{subnetID},
+		},
 	}
 
-	return nwInterfaces, nil
+	describeNetworkInterfacesInput := &ec2.DescribeNetworkInterfacesInput{Filters: filters}
+	return h.ec2Wrapper.DescribeNetworkInterfacesPages(describeNetworkInterfacesInput)
 }
 
 // DetachAndDeleteNetworkInterface detaches the network interface first and then deletes it

pkg/aws/ec2/api/helper_test.go

@@ -179,27 +179,20 @@ var (
 
 	tokenID = "token"
 
-	describeTrunkInterfaceInput1 = &ec2.DescribeNetworkInterfacesInput{
-		Filters: []*ec2.Filter{{
-			Name:   aws.String("tag:" + config.TrunkENIIDTag),
-			Values: []*string{&trunkInterfaceId},
-		}},
-	}
-	describeTrunkInterfaceInput2 = &ec2.DescribeNetworkInterfacesInput{
-		Filters: []*ec2.Filter{{
-			Name:   aws.String("tag:" + config.TrunkENIIDTag),
-			Values: []*string{&trunkInterfaceId},
-		}},
-		NextToken: &tokenID,
+	describeTrunkInterfaceInput = &ec2.DescribeNetworkInterfacesInput{
+		Filters: []*ec2.Filter{
+			{
+				Name:   aws.String("tag:" + config.TrunkENIIDTag),
+				Values: []*string{&trunkInterfaceId},
+			},
+			{
+				Name:   aws.String("subnet-id"),
+				Values: aws.StringSlice([]string{subnetId}),
+			},
+		},
 	}
 
-	describeTrunkInterfaceOutput1 = &ec2.DescribeNetworkInterfacesOutput{
-		NetworkInterfaces: []*ec2.NetworkInterface{&networkInterface1},
-		NextToken:         &tokenID,
-	}
-	describeTrunkInterfaceOutput2 = &ec2.DescribeNetworkInterfacesOutput{
-		NetworkInterfaces: []*ec2.NetworkInterface{&networkInterface2},
-	}
+	describeTrunkInterfaceOutput = []*ec2.NetworkInterface{&networkInterface1, &networkInterface2}
 
 	describeTrunkInterfaceAssociationsInput = &ec2.DescribeTrunkInterfaceAssociationsInput{
 		Filters: []*ec2.Filter{{
@@ -1178,16 +1171,15 @@ func TestEC2APIHelper_AssignIPv4ResourcesAndWaitTillReady_TypeIPv4Prefix_Describ
 }
 
 // TestEc2APIHelper_GetBranchNetworkInterface_PaginatedResults returns the branch interface when paginated results is returned
-func TestEc2APIHelper_GetBranchNetworkInterface_PaginatedResults(t *testing.T) {
+func TestEc2APIHelper_GetBranchNetworkInterface(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
 	ec2ApiHelper, mockWrapper := getMockWrapper(ctrl)
 
-	mockWrapper.EXPECT().DescribeNetworkInterfaces(describeTrunkInterfaceInput1).Return(describeTrunkInterfaceOutput1, nil)
-	mockWrapper.EXPECT().DescribeNetworkInterfaces(describeTrunkInterfaceInput2).Return(describeTrunkInterfaceOutput2, nil)
+	mockWrapper.EXPECT().DescribeNetworkInterfacesPages(describeTrunkInterfaceInput).Return(describeTrunkInterfaceOutput, nil)
 
-	branchInterfaces, err := ec2ApiHelper.GetBranchNetworkInterface(&trunkInterfaceId)
+	branchInterfaces, err := ec2ApiHelper.GetBranchNetworkInterface(&trunkInterfaceId, &subnetId)
 	assert.NoError(t, err)
 	assert.ElementsMatch(t, []*ec2.NetworkInterface{&networkInterface1, &networkInterface2}, branchInterfaces)
 }