Fix for leaking target groups on service delete (#525)

* Fix for leaking target groups on service delete due to listener delete race condition

Author: erikfuller

docs/contributing/developer.md

@@ -93,7 +93,7 @@ make e2e-clean
 
 ## Local Development
 
-A minimal sanity check on changes can be done with make presubmit. This command will also run on PR.
+A minimal test of changes can be done with ```make presubmit```. This command will also run on PR.
 
 ```
 make presubmit

pkg/aws/services/vpclattice.go

@@ -94,6 +94,7 @@ func IsInvalidError(err error) bool {
 
 type Lattice interface {
 	vpclatticeiface.VPCLatticeAPI
+	ListListenersAsList(ctx context.Context, input *vpclattice.ListListenersInput) ([]*vpclattice.ListenerSummary, error)
 	GetRulesAsList(ctx context.Context, input *vpclattice.ListRulesInput) ([]*vpclattice.GetRuleOutput, error)
 	ListRulesAsList(ctx context.Context, input *vpclattice.ListRulesInput) ([]*vpclattice.RuleSummary, error)
 	ListServiceNetworksAsList(ctx context.Context, input *vpclattice.ListServiceNetworksInput) ([]*vpclattice.ServiceNetworkSummary, error)
@@ -128,6 +129,23 @@ func NewDefaultLattice(sess *session.Session, region string) *defaultLattice {
 	return &defaultLattice{VPCLatticeAPI: latticeSess, cache: cache}
 }
 
+func (d *defaultLattice) ListListenersAsList(ctx context.Context, input *vpclattice.ListListenersInput) ([]*vpclattice.ListenerSummary, error) {
+	var result []*vpclattice.ListenerSummary
+
+	err := d.ListListenersPagesWithContext(ctx, input, func(page *vpclattice.ListListenersOutput, lastPage bool) bool {
+		for _, l := range page.Items {
+			result = append(result, l)
+		}
+		return true
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return result, nil
+}
+
 func (d *defaultLattice) GetRulesAsList(ctx context.Context, input *vpclattice.ListRulesInput) ([]*vpclattice.GetRuleOutput, error) {
 	var result []*vpclattice.GetRuleOutput
 

pkg/aws/services/vpclattice_mocks.go

@@ -302,6 +302,25 @@ func (m *defaultServiceManager) deleteAllAssociations(ctx context.Context, svc *
 	return nil
 }
 
+func (m *defaultServiceManager) deleteAllListeners(ctx context.Context, svc *SvcSummary) error {
+	listeners, err := m.cloud.Lattice().ListListenersAsList(ctx, &vpclattice.ListListenersInput{
+		ServiceIdentifier: svc.Id,
+	})
+	if err != nil {
+		return err
+	}
+	for _, listener := range listeners {
+		_, err = m.cloud.Lattice().DeleteListenerWithContext(ctx, &vpclattice.DeleteListenerInput{
+			ServiceIdentifier:  svc.Id,
+			ListenerIdentifier: listener.Id,
+		})
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (m *defaultServiceManager) deleteAssociation(ctx context.Context, assocArn *string) error {
 	delReq := &DelSnSvcAssocReq{ServiceNetworkServiceAssociationIdentifier: assocArn}
 	_, err := m.cloud.Lattice().DeleteServiceNetworkServiceAssociationWithContext(ctx, delReq)
@@ -371,6 +390,12 @@ func (m *defaultServiceManager) Delete(ctx context.Context, svc *Service) error
 		return err
 	}
 
+	// deleting listeners explicitly helps ensure target groups are free to delete
+	err = m.deleteAllListeners(ctx, svcSum)
+	if err != nil {
+		return err
+	}
+
 	err = m.deleteService(ctx, svcSum)
 	if err != nil {
 		return err

pkg/deploy/lattice/service_manager.go

@@ -311,6 +311,23 @@ func TestServiceManagerInteg(t *testing.T) {
 			DeleteServiceWithContext(gomock.Any(), gomock.Any()).Return(nil, nil).
 			Times(1)
 
+		// assert we delete listeners
+		mockLattice.EXPECT().
+			ListListenersAsList(gomock.Any(), gomock.Any()).
+			Return([]*vpclattice.ListenerSummary{
+				{
+					Id: aws.String("L1"),
+				},
+				{
+					Id: aws.String("L2"),
+				},
+			}, nil)
+
+		mockLattice.EXPECT().
+			DeleteListenerWithContext(gomock.Any(), gomock.Any()).
+			Return(nil, nil).
+			Times(2)
+
 		err := m.Delete(ctx, svc)
 		assert.Nil(t, err)
 	})