Revert "fix: paginate DescribeNetworkInterfaces with deep filters (aw…

…s#375)"

This reverts commit b5699de.

main.go

@@ -107,7 +107,6 @@ 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.")
@@ -142,7 +141,6 @@ 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()
 
@@ -185,11 +183,6 @@ 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/
@@ -343,7 +336,6 @@ 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,7 +34,6 @@ type ENICleaner struct {
 	EC2Wrapper  EC2Wrapper
 	ClusterName string
 	Log         logr.Logger
-	VPCID       string
 
 	availableENIs     map[string]struct{}
 	shutdown          bool
@@ -43,22 +42,16 @@ type ENICleaner struct {
 }
 
 var (
-	vpccniAvailableENICnt = prometheus.NewGauge(
-		prometheus.GaugeOpts{
-			Name: "vpc_cni_created_available_eni_count",
-			Help: "The number of available ENIs created by VPC-CNI that controller will try to delete in each cleanup cycle",
+	vpcCniLeakedENICleanupCnt = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "vpc_cni_created_leaked_eni_cleanup_count",
+			Help: "The number of leaked ENIs created by VPC-CNI that is cleaned up by the controller",
 		},
 	)
-	vpcrcAvailableENICnt = prometheus.NewGauge(
-		prometheus.GaugeOpts{
-			Name: "vpc_rc_created_available_eni_count",
-			Help: "The number of available ENIs created by VPC-RC that controller will try to delete in each cleanup cycle",
-		},
-	)
-	leakedENICnt = prometheus.NewGauge(
-		prometheus.GaugeOpts{
-			Name: "leaked_eni_count",
-			Help: "The number of available ENIs that failed to be deleted by the controller in each cleanup cycle",
+	vpcrcLeakedENICleanupCnt = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "vpc_rc_created_leaked_eni_cleanup_count",
+			Help: "The number of leaked ENIs created by VPC-RC that is cleaned up by the controller",
 		},
 	)
 )
@@ -108,9 +101,6 @@ func (e *ENICleaner) Start(ctx context.Context) error {
 // interval between cycle 1 and 2 and hence can be safely deleted. And we can also conclude that Interface 1 was
 // created but not attached at the the time when 1st cycle ran and hence it should not be deleted.
 func (e *ENICleaner) cleanUpAvailableENIs() {
-	vpcrcAvailableCount := 0
-	vpccniAvailableCount := 0
-	leakedENICount := 0
 	describeNetworkInterfaceIp := &ec2.DescribeNetworkInterfacesInput{
 		Filters: []*ec2.Filter{
 			{
@@ -126,65 +116,63 @@ 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{})
 
-	networkInterfaces, err := e.EC2Wrapper.DescribeNetworkInterfacesPages(describeNetworkInterfaceIp)
-	if err != nil {
-		e.Log.Error(err, "failed to describe network interfaces, cleanup will be retried in next cycle")
-		return
-	}
+	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 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:
-					vpcrcAvailableCount += 1
-				case config.NetworkInterfaceOwnerVPCCNITagValue:
-					vpccniAvailableCount += 1
-				default:
-					// We should not hit this case as we only filter for relevant tag values, log error and continue if unexpected ENIs found
-					e.Log.Error(fmt.Errorf("found available ENI not created by VPC-CNI/VPC-RC"), "eniID", *networkInterface.NetworkInterfaceId)
+		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")
+					}
+				}
+
+				// 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)
 			}
+		}
 
-			// 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 {
-				leakedENICount += 1
-				// 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)
+		if describeNetworkInterfaceOp.NextToken == nil {
+			break
 		}
+
+		describeNetworkInterfaceIp.NextToken = describeNetworkInterfaceOp.NextToken
 	}
 
-	// Update leaked ENI metrics
-	vpcrcAvailableENICnt.Set(float64(vpcrcAvailableCount))
-	vpccniAvailableENICnt.Set(float64(vpccniAvailableCount))
-	leakedENICnt.Set(float64(leakedENICount))
 	// Set the available ENIs to the list of ENIs seen in the current cycle
 	e.availableENIs = availableENIs
 }

pkg/aws/ec2/api/eni_cleanup_test.go

@@ -37,8 +37,6 @@ var (
 	mockNetworkInterfaceId2 = "eni-000000000000001"
 	mockNetworkInterfaceId3 = "eni-000000000000002"
 
-	mockVPCID = "vpc-0000000000000000"
-
 	mockDescribeNetworkInterfaceIp = &ec2.DescribeNetworkInterfacesInput{
 		Filters: []*ec2.Filter{
 			{
@@ -54,19 +52,19 @@ var (
 				Values: aws.StringSlice([]string{config.NetworkInterfaceOwnerTagValue,
 					config.NetworkInterfaceOwnerVPCCNITagValue}),
 			},
-			{
-				Name:   aws.String("vpc-id"),
-				Values: []*string{aws.String(mockVPCID)},
-			},
 		},
 	}
-	mockDescribeInterfaceOpWith1And2 = []*ec2.NetworkInterface{
-		{NetworkInterfaceId: &mockNetworkInterfaceId1},
-		{NetworkInterfaceId: &mockNetworkInterfaceId2},
+	mockDescribeInterfaceOpWith1And2 = &ec2.DescribeNetworkInterfacesOutput{
+		NetworkInterfaces: []*ec2.NetworkInterface{
+			{NetworkInterfaceId: &mockNetworkInterfaceId1},
+			{NetworkInterfaceId: &mockNetworkInterfaceId2},
+		},
 	}
-	mockDescribeInterfaceOpWith1And3 = []*ec2.NetworkInterface{
-		{NetworkInterfaceId: &mockNetworkInterfaceId1},
-		{NetworkInterfaceId: &mockNetworkInterfaceId3},
+	mockDescribeInterfaceOpWith1And3 = &ec2.DescribeNetworkInterfacesOutput{
+		NetworkInterfaces: []*ec2.NetworkInterface{
+			{NetworkInterfaceId: &mockNetworkInterfaceId1},
+			{NetworkInterfaceId: &mockNetworkInterfaceId3},
+		},
 	}
 )
 
@@ -76,7 +74,6 @@ 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
@@ -88,10 +85,10 @@ func TestENICleaner_cleanUpAvailableENIs(t *testing.T) {
 
 	gomock.InOrder(
 		// Return network interface 1 and 2 in first cycle
-		mockWrapper.EXPECT().DescribeNetworkInterfacesPages(mockDescribeNetworkInterfaceIp).
+		mockWrapper.EXPECT().DescribeNetworkInterfaces(mockDescribeNetworkInterfaceIp).
 			Return(mockDescribeInterfaceOpWith1And2, nil),
 		// Return network interface 1 and 3 in the second cycle
-		mockWrapper.EXPECT().DescribeNetworkInterfacesPages(mockDescribeNetworkInterfaceIp).
+		mockWrapper.EXPECT().DescribeNetworkInterfaces(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, subnetID *string) ([]*ec2.NetworkInterface, error)
+	GetBranchNetworkInterface(trunkID *string) ([]*ec2.NetworkInterface, error)
 	GetInstanceNetworkInterface(instanceId *string) ([]*ec2.InstanceNetworkInterface, error)
 	DescribeNetworkInterfaces(nwInterfaceIds []*string) ([]*ec2.NetworkInterface, error)
 	DescribeTrunkInterfaceAssociation(trunkInterfaceId *string) ([]*ec2.TrunkInterfaceAssociation, error)
@@ -562,20 +562,43 @@ func (h *ec2APIHelper) UnassignIPv4Resources(eniID string, resourceType config.R
 	return err
 }
 
-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},
-		},
-	}
+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}
-	return h.ec2Wrapper.DescribeNetworkInterfacesPages(describeNetworkInterfacesInput)
+	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
+	}
+
+	return nwInterfaces, nil
 }
 
 // DetachAndDeleteNetworkInterface detaches the network interface first and then deletes it