Call DisassociateTrunkInterface before deleting branch ENI (#372)

* Call DisassociateTrunkInterface before deleting branch ENI

pkg/aws/ec2/api/helper.go

@@ -93,6 +93,7 @@ type EC2APIHelper interface {
 	GetInstanceDetails(instanceId *string) (*ec2.Instance, error)
 	AssignIPv4ResourcesAndWaitTillReady(eniID string, resourceType config.ResourceType, count int) ([]string, error)
 	UnassignIPv4Resources(eniID string, resourceType config.ResourceType, resources []string) error
+	DisassociateTrunkInterface(associationID *string) error
 }
 
 // CreateNetworkInterface creates a new network interface
@@ -617,3 +618,10 @@ func (h *ec2APIHelper) DetachAndDeleteNetworkInterface(attachmentID *string, nwI
 	}
 	return nil
 }
+
+func (h *ec2APIHelper) DisassociateTrunkInterface(associationID *string) error {
+	input := &ec2.DisassociateTrunkInterfaceInput{
+		AssociationId: associationID,
+	}
+	return h.ec2Wrapper.DisassociateTrunkInterface(input)
+}

pkg/aws/ec2/api/wrapper.go

@@ -58,6 +58,7 @@ type EC2Wrapper interface {
 	DescribeTrunkInterfaceAssociations(input *ec2.DescribeTrunkInterfaceAssociationsInput) (*ec2.DescribeTrunkInterfaceAssociationsOutput, error)
 	ModifyNetworkInterfaceAttribute(input *ec2.ModifyNetworkInterfaceAttributeInput) (*ec2.ModifyNetworkInterfaceAttributeOutput, error)
 	CreateNetworkInterfacePermission(input *ec2.CreateNetworkInterfacePermissionInput) (*ec2.CreateNetworkInterfacePermissionOutput, error)
+	DisassociateTrunkInterface(input *ec2.DisassociateTrunkInterfaceInput) error
 }
 
 var (
@@ -253,7 +254,7 @@ var (
 	ec2AssociateTrunkInterfaceAPIErrCnt = prometheus.NewCounter(
 		prometheus.CounterOpts{
 			Name: "ec2_associate_trunk_interface_api_err_count",
-			Help: "The number of errors encountered while disassociating Trunk with Branch ENI",
+			Help: "The number of errors encountered while associating Trunk with Branch ENI",
 		},
 	)
 
@@ -307,6 +308,20 @@ var (
 		},
 	)
 
+	ec2DisassociateTrunkInterfaceCallCnt = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "ec2_disassociate_trunk_interface_api_req_count",
+			Help: "The number of calls made to EC2 to remove association between a branch and trunk network interface",
+		},
+	)
+
+	ec2DisassociateTrunkInterfaceErrCnt = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "ec2_disassociate_trunk_interface_api_err_count",
+			Help: "The number of errors encountered while removing association between a branch and trunk network interface",
+		},
+	)
+
 	prometheusRegistered = false
 )
 
@@ -347,6 +362,8 @@ func prometheusRegister() {
 			ec2APICallLatencies,
 			vpcCniLeakedENICleanupCnt,
 			vpcrcLeakedENICleanupCnt,
+			ec2DisassociateTrunkInterfaceCallCnt,
+			ec2DisassociateTrunkInterfaceErrCnt,
 		)
 
 		prometheusRegistered = true
@@ -375,7 +392,7 @@ func NewEC2Wrapper(roleARN, clusterName, region string, log logr.Logger) (EC2Wra
 
 	// Role ARN is passed, assume the role ARN to make EC2 API Calls
 	if roleARN != "" {
-		// Create the instance service client with low QPS, it will be only used fro associate branch to trunk calls
+		// Create the instance service client with low QPS, it will be only used for associate branch to trunk calls
 		log.Info("Creating INSTANCE service client with configured QPS", "QPS", config.InstanceServiceClientQPS, "Burst", config.InstanceServiceClientBurst)
 		instanceServiceClient, err := ec2Wrapper.getInstanceServiceClient(config.InstanceServiceClientQPS,
 			config.InstanceServiceClientBurst, instanceSession)
@@ -788,3 +805,19 @@ func (e *ec2Wrapper) CreateNetworkInterfacePermission(input *ec2.CreateNetworkIn
 
 	return output, err
 }
+
+func (e *ec2Wrapper) DisassociateTrunkInterface(input *ec2.DisassociateTrunkInterfaceInput) error {
+	start := time.Now()
+	// Using the instance role
+	_, err := e.instanceServiceClient.DisassociateTrunkInterface(input)
+	ec2APICallLatencies.WithLabelValues("disassociate_branch_from_trunk").Observe(timeSinceMs(start))
+
+	ec2APICallCnt.Inc()
+	ec2DisassociateTrunkInterfaceCallCnt.Inc()
+
+	if err != nil {
+		ec2APIErrCnt.Inc()
+		ec2DisassociateTrunkInterfaceErrCnt.Inc()
+	}
+	return err
+}

pkg/provider/branch/trunk/trunk.go

@@ -138,6 +138,8 @@ type ENIDetails struct {
 	deletionTimeStamp time.Time
 	// deleteRetryCount is the
 	deleteRetryCount int
+	// ID of association between branch and trunk ENI
+	AssociationID string `json:"associationID"`
 }
 
 type IntrospectResponse struct {
@@ -398,12 +400,14 @@ func (t *trunkENI) CreateAndAssociateBranchENIs(pod *v1.Pod, securityGroups []st
 		newENIs = append(newENIs, newENI)
 
 		// Associate Branch to trunk
-		_, err = t.ec2ApiHelper.AssociateBranchToTrunk(&t.trunkENIId, nwInterface.NetworkInterfaceId, vlanID)
+		var associationOutput *awsEC2.AssociateTrunkInterfaceOutput
+		associationOutput, err = t.ec2ApiHelper.AssociateBranchToTrunk(&t.trunkENIId, nwInterface.NetworkInterfaceId, vlanID)
 		if err != nil {
 			err = fmt.Errorf("associating branch to trunk, %w", err)
 			trunkENIOperationsErrCount.WithLabelValues("associate_branch").Inc()
 			break
 		}
+		newENI.AssociationID = *associationOutput.InterfaceAssociation.AssociationId
 	}
 
 	if err != nil {
@@ -499,7 +503,19 @@ func (t *trunkENI) DeleteCooledDownENIs() {
 
 // deleteENIs deletes the provided ENIs and frees up the Vlan assigned to then
 func (t *trunkENI) deleteENI(eniDetail *ENIDetails) (err error) {
-	// Delete Branch network interface first
+	// Disassociate branch ENI from trunk if association ID exists and delete branch network interface
+	if eniDetail.AssociationID != "" {
+		err = t.ec2ApiHelper.DisassociateTrunkInterface(&eniDetail.AssociationID)
+		if err != nil {
+			trunkENIOperationsErrCount.WithLabelValues("disassociate_trunk_error").Inc()
+			if !strings.Contains(err.Error(), ec2Errors.NotFoundAssociationID) {
+				t.log.Error(err, "failed to disassciate branch ENI from trunk, will try to delete the branch ENI")
+				// Not returning error here, fallback to force branch ENI deletion
+			} else {
+				t.log.Info("AssociationID not found when disassociating branch from trunk ENI, it is already disassociated so delete the branch ENI")
+			}
+		}
+	}
 	err = t.ec2ApiHelper.DeleteNetworkInterface(&eniDetail.ID)
 	if err != nil {
 		branchENIOperationsFailureCount.WithLabelValues("delete_branch_error").Inc()