Imrpoved TG reconciling

Author: bigkraig

pkg/alb/lb/loadbalancer.go

@@ -332,9 +332,10 @@ func (l *LoadBalancer) Reconcile(rOpts *ReconcileOptions) []error {
 		Eventf:            rOpts.Eventf,
 		VpcID:             l.lb.current.VpcId,
 		ManagedSGInstance: l.options.current.managedInstanceSG,
+		IgnoreDeletes:     true,
 	}
 
-	tgs, deletedTG, err := l.targetgroups.Reconcile(tgsOpts)
+	tgs, err := l.targetgroups.Reconcile(tgsOpts)
 	if err != nil {
 		errors = append(errors, err)
 		return errors
@@ -352,33 +353,26 @@ func (l *LoadBalancer) Reconcile(rOpts *ReconcileOptions) []error {
 		l.listeners = ltnrs
 	}
 
-	// REFACTOR!
-	// This chunk of code has some questionable logic and we should probably move
-	// the TG clean up out of here and into tg. I also dont think that lb.listeners < 1 is a valid check
-	//
-	// Return now if listeners are already deleted, signifies has already been destructed and
-	// TG clean-up, based on rules below does not need to occur.
-	for _, t := range deletedTG {
-		if err := albelbv2.ELBV2svc.RemoveTargetGroup(t.CurrentARN()); err != nil {
-			errors = append(errors, err)
-			continue
-		}
-		index, _ := l.targetgroups.FindById(t.ID)
-		l.targetgroups = append(l.targetgroups[:index], l.targetgroups[index+1:]...)
+	tgsOpts.IgnoreDeletes = false
+	tgs, err = l.targetgroups.Reconcile(tgsOpts)
+	if err != nil {
+		errors = append(errors, err)
+		return errors
 	}
+	l.targetgroups = tgs
 
-	for _, listener := range l.listeners {
-		unusedTGs := listener.GetRules().FindUnusedTGs(l.targetgroups)
-		for _, t := range unusedTGs {
-			if err := albelbv2.ELBV2svc.RemoveTargetGroup(t.CurrentARN()); err != nil {
-				errors = append(errors, err)
-				continue
-			}
-			index, _ := l.targetgroups.FindById(t.ID)
-			l.targetgroups = append(l.targetgroups[:index], l.targetgroups[index+1:]...)
-		}
-	}
-	// END REFACTOR
+	// Decide: Is this still needed?
+	// for _, listener := range l.listeners {
+	// 	unusedTGs := listener.GetRules().FindUnusedTGs(l.targetgroups)
+	// 	for _, t := range unusedTGs {
+	// 		if err := albelbv2.ELBV2svc.RemoveTargetGroup(t.CurrentARN()); err != nil {
+	// 			errors = append(errors, err)
+	// 			continue
+	// 		}
+	// 		index, _ := l.targetgroups.FindById(t.ID)
+	// 		l.targetgroups = append(l.targetgroups[:index], l.targetgroups[index+1:]...)
+	// 	}
+	// }
 
 	return errors
 }

pkg/alb/tg/targetgroup.go

@@ -144,9 +144,13 @@ func (t *TargetGroup) Reconcile(rOpts *ReconcileOptions) error {
 	// No DesiredState means target group may not be needed.
 	// However, target groups aren't deleted until after rules are created
 	// Ensuring we know what target groups are truly no longer in use.
-	case t.tg.desired == nil:
-		t.deleted = true
-		return nil
+	case t.tg.desired == nil && !rOpts.IgnoreDeletes:
+		t.logger.Infof("Start TargetGroup deletion.")
+		if err := t.delete(rOpts); err != nil {
+			return err
+		}
+		rOpts.Eventf(api.EventTypeNormal, "DELETE", "%v target group deleted", t.ID)
+		t.logger.Infof("Completed TargetGroup deletion.")
 
 		// No CurrentState means target group doesn't exist in AWS and should be created.
 	case t.tg.current == nil:
@@ -302,6 +306,16 @@ func (t *TargetGroup) modify(mods tgChange, rOpts *ReconcileOptions) error {
 	return nil
 }
 
+// delete a TargetGroup.
+func (t *TargetGroup) delete(rOpts *ReconcileOptions) error {
+	if err := albelbv2.ELBV2svc.RemoveTargetGroup(t.CurrentARN()); err != nil {
+		rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error deleting %v target group: %s", t.ID, err.Error())
+		return err
+	}
+	t.deleted = true
+	return nil
+}
+
 func (t *TargetGroup) needsModification() tgChange {
 	var changes tgChange
 

pkg/alb/tg/targetgroups.go

@@ -48,20 +48,17 @@ func (t TargetGroups) FindCurrentByARN(id string) (int, *TargetGroup) {
 // Reconcile kicks off the state synchronization for every target group inside this TargetGroups
 // instance. It returns the new TargetGroups its created and a list of TargetGroups it believes
 // should be cleaned up.
-func (t TargetGroups) Reconcile(rOpts *ReconcileOptions) (TargetGroups, TargetGroups, error) {
+func (t TargetGroups) Reconcile(rOpts *ReconcileOptions) (TargetGroups, error) {
 	var output TargetGroups
-	var deleted TargetGroups
+
 	for _, tg := range t {
 		if err := tg.Reconcile(rOpts); err != nil {
-			return nil, nil, err
-		}
-		if tg.deleted {
-			deleted = append(deleted, tg)
+			return nil, err
 		}
 		output = append(output, tg)
 	}
 
-	return output, deleted, nil
+	return output, nil
 }
 
 // StripDesiredState removes the Tags.Desired, DesiredTargetGroup, and Targets.Desired from all TargetGroups

pkg/alb/tg/types.go

@@ -48,6 +48,7 @@ type ReconcileOptions struct {
 	Eventf            func(string, string, string, ...interface{})
 	VpcID             *string
 	ManagedSGInstance *string
+	IgnoreDeletes     bool
 }
 
 type tgChange uint