Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #436 (2021-09-28 23:06:49)

Do not attach nonexistent target groups
When controller attempts to attach a batch of target groups that
contains nonexistent target group (could be deleted manually by mistake)
to the autoscaling group the attempt fails to pass validation which
prevents attachment of the other valid target groups from the batch.

Also:
* Improves error log messages
* Adds processChunked helper function
* Adds describeTargetGroups
* Fixes `detachARNs`/`validARNs` slices' capacity

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>

jonathanbeber: :+1: 

Patch:
diff --git a/.gitignore b/.gitignore
index 067f81d..f7dafd1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,3 +34,5 @@ Dockerfile.upstream
 /.GOPATH
 /bin
 profile.cov
+
+.idea/
diff --git a/aws/adapter.go b/aws/adapter.go
index 3427b69..9c3d432 100644
--- a/aws/adapter.go
+++ b/aws/adapter.go
@@ -543,7 +543,7 @@ func (a *Adapter) UpdateTargetGroupsAndAutoScalingGroups(stacks []*Stack) {
 	for _, asg := range a.TargetedAutoScalingGroups {
 		// This call is idempotent and safe to execute every time
 		if err := updateTargetGroupsForAutoScalingGroup(a.autoscaling, a.elbv2, targetGroupARNs, asg.name, ownerTags); err != nil {
-			log.Errorf("UpdateTargetGroupsAndAutoScalingGroups() failed to attach target groups to ASG '%s': %v", asg.name, err)
+			log.Errorf("Failed to update target groups for autoscaling group %q: %v", asg.name, err)
 		}
 	}
 
@@ -552,7 +552,7 @@ func (a *Adapter) UpdateTargetGroupsAndAutoScalingGroups(stacks []*Stack) {
 	for _, asg := range nonTargetedASGs {
 		// This call is idempotent and safe to execute every time
 		if err := updateTargetGroupsForAutoScalingGroup(a.autoscaling, a.elbv2, nil, asg.name, ownerTags); err != nil {
-			log.Errorf("UpdateTargetGroupsAndAutoScalingGroups() failed to attach target groups to ASG '%s': %v", asg.name, err)
+			log.Errorf("Failed to update target groups for non-targeted autoscaling group %q: %v", asg.name, err)
 		}
 	}
 
@@ -560,13 +560,13 @@ func (a *Adapter) UpdateTargetGroupsAndAutoScalingGroups(stacks []*Stack) {
 	if len(runningSingleInstances) != 0 {
 		// This call is idempotent too
 		if err := registerTargetsOnTargetGroups(a.elbv2, targetGroupARNs, runningSingleInstances); err != nil {
-			log.Errorf("UpdateTargetGroupsAndAutoScalingGroups() failed to register instances %q in target groups: %v", runningSingleInstances, err)
+			log.Errorf("Failed to register instances %q in target groups: %v", runningSingleInstances, err)
 		}
 	}
 	if len(a.obsoleteInstances) != 0 {
 		// Deregister instances from target groups and clean up list of obsolete instances
 		if err := deregisterTargetsOnTargetGroups(a.elbv2, targetGroupARNs, a.obsoleteInstances); err != nil {
-			log.Errorf("UpdateTargetGroupsAndAutoScalingGroups() failed to deregister instances %q in target groups: %v", a.obsoleteInstances, err)
+			log.Errorf("Failed to deregister instances %q in target groups: %v", a.obsoleteInstances, err)
 		} else {
 			a.obsoleteInstances = make([]string, 0)
 		}
@@ -721,10 +721,9 @@ func (a *Adapter) GetStack(stackID string) (*Stack, error) {
 func (a *Adapter) DeleteStack(stack *Stack) error {
 	for _, asg := range a.TargetedAutoScalingGroups {
 		if err := detachTargetGroupsFromAutoScalingGroup(a.autoscaling, stack.TargetGroupARNs, asg.name); err != nil {
-			return fmt.Errorf("DeleteStack failed to detach: %v", err)
+			return fmt.Errorf("failed to detach target groups from autoscaling group %q: %w", asg.name, err)
 		}
 	}
-
 	return deleteStack(a.cloudformation, stack.Name)
 }
 
@@ -899,7 +898,7 @@ func (a *Adapter) parseFilters(clusterId string) []*ec2.Filter {
 		for i, term := range terms {
 			parts := strings.Split(term, "=")
 			if len(parts) != 2 {
-				log.Errorf("failed parsing %s, falling back to default", a.customFilter)
+				log.Errorf("Failed parsing %s, falling back to default", a.customFilter)
 				return generateDefaultFilters(clusterId)
 			}
 			filters[i] = &ec2.Filter{
@@ -936,7 +935,7 @@ func (a *Adapter) parseAutoscaleFilterTags(clusterId string) map[string][]string
 		for _, term := range terms {
 			parts := strings.Split(term, "=")
 			if len(parts) != 2 {
-				log.Errorf("failed parsing %s, falling back to default", a.customFilter)
+				log.Errorf("Failed parsing %s, falling back to default", a.customFilter)
 				return generateDefaultAutoscaleFilterTags(clusterId)
 			}
 			if parts[0] == "tag-key" {
diff --git a/aws/asg.go b/aws/asg.go
index 983162e..4c248a7 100644
--- a/aws/asg.go
+++ b/aws/asg.go
@@ -9,6 +9,8 @@ import (
 	"github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/aws/aws-sdk-go/service/elbv2/elbv2iface"
+
+	log "github.com/sirupsen/logrus"
 )
 
 type autoScalingGroupDetails struct {
@@ -165,36 +167,27 @@ func updateTargetGroupsForAutoScalingGroup(svc autoscalingiface.AutoScalingAPI,
 		return err
 	}
 
-	// get all target groups to ensure we are only working with target
-	// groups that still exists.
-	tgParams := &elbv2.DescribeTargetGroupsInput{}
-	allTGs := make(map[string]struct{}, len(resp.LoadBalancerTargetGroups))
-	err = elbv2svc.DescribeTargetGroupsPagesWithContext(context.TODO(), tgParams, func(resp *elbv2.DescribeTargetGroupsOutput, lastPage bool) bool {
-		for _, tg := range resp.TargetGroups {
-			allTGs[aws.StringValue(tg.TargetGroupArn)] = struct{}{}
-		}
-		return true
-	})
+	allTGs, err := describeTargetGroups(elbv2svc)
 	if err != nil {
 		return err
 	}
 
 	if len(resp.LoadBalancerTargetGroups) > 0 {
 		// find non-existing target groups which should be detached
-		detachARNs := make([]string, 0, len(targetGroupARNs))
-		validARNs := make([]string, 0, len(targetGroupARNs))
+		detachARNs := make([]string, 0, len(resp.LoadBalancerTargetGroups))
+		validARNs := make([]string, 0, len(resp.LoadBalancerTargetGroups))
 		for _, tg := range resp.LoadBalancerTargetGroups {
 			tgARN := aws.StringValue(tg.LoadBalancerTargetGroupARN)
 
 			// check that TG exists at all, otherwise detach it
 			if _, ok := allTGs[tgARN]; !ok {
 				detachARNs = append(detachARNs, tgARN)
-				continue
+			} else {
+				validARNs = append(validARNs, tgARN)
 			}
-			validARNs = append(validARNs, tgARN)
 		}
 
-		descs, err := describeTagsChunked(elbv2svc, validARNs)
+		descs, err := describeTags(elbv2svc, validARNs)
 		if err != nil {
 			return err
 		}
@@ -213,67 +206,58 @@ func updateTargetGroupsForAutoScalingGroup(svc autoscalingiface.AutoScalingAPI,
 		}
 
 		if len(detachARNs) > 0 {
-			err = detachTargetGroupsFromAutoScalingGroup(svc, detachARNs, autoScalingGroupName)
+			err := detachTargetGroupsFromAutoScalingGroup(svc, detachARNs, autoScalingGroupName)
 			if err != nil {
 				return err
 			}
 		}
 	}
 
-	// limit target group updates to 10 at a time since this is the limit
-	// in AWS API.
-	chunkSize := 10
-
-	for i := 0; i < len(targetGroupARNs); i += chunkSize {
-		end := i + chunkSize
-
-		if end > len(targetGroupARNs) {
-			end = len(targetGroupARNs)
+	attachARNs := make([]string, 0, len(targetGroupARNs))
+	for _, tgARN := range targetGroupARNs {
+		if _, ok := allTGs[tgARN]; ok {
+			attachARNs = append(attachARNs, tgARN)
+		} else {
+			// TODO: it is better to validate stack's target groups earlier to identify owning stack
+			log.Errorf("Target group %q does not exist, will not attach", tgARN)
 		}
+	}
 
-		groups := targetGroupARNs[i:end]
-		if len(groups) > 0 {
-			attachParams := &autoscaling.AttachLoadBalancerTargetGroupsInput{
-				AutoScalingGroupName: aws.String(autoScalingGroupName),
-				TargetGroupARNs:      aws.StringSlice(groups),
-			}
-			_, err = svc.AttachLoadBalancerTargetGroups(attachParams)
-			if err != nil {
-				return err
-			}
+	if len(attachARNs) > 0 {
+		err := attachTargetGroupsToAutoScalingGroup(svc, attachARNs, autoScalingGroupName)
+		if err != nil {
+			return err
 		}
 	}
 
 	return nil
 }
 
-func describeTagsChunked(svc elbv2iface.ELBV2API, arns []string) ([]*elbv2.TagDescription, error) {
+func describeTags(svc elbv2iface.ELBV2API, arns []string) ([]*elbv2.TagDescription, error) {
 	descs := make([]*elbv2.TagDescription, 0, len(arns))
-	chunkSize := 20
-
-	for i := 0; i < len(arns); i += chunkSize {
-		end := i + chunkSize
-
-		if end > len(arns) {
-			end = len(arns)
-		}
-
-		arnsChunked := arns[i:end]
-		if len(arnsChunked) > 0 {
-			tgParams := &elbv2.DescribeTagsInput{
-				ResourceArns: aws.StringSlice(arnsChunked),
-			}
-
-			tgResp, err := svc.DescribeTags(tgParams)
-			if err != nil {
-				return nil, err
-			}
-
+	// You can specify up to 20 resources in a single call,
+	// see https://docs.aws.amazon.com/sdk-for-go/api/service/elbv2/#DescribeTagsInput
+	err := processChunked(arns, 20, func(chunk []string) error {
+		tgResp, err := svc.DescribeTags(&elbv2.DescribeTagsInput{
+			ResourceArns: aws.StringSlice(chunk),
+		})
+		if err == nil {
 			descs = append(descs, tgResp.TagDescriptions...)
 		}
-	}
+		return err
+	})
+	return descs, err
+}
 
-	return descs, nil
+func describeTargetGroups(elbv2svc elbv2iface.ELBV2API) (map[string]struct{}, error) {
+	targetGroups := make(map[string]struct{})
+	err := elbv2svc.DescribeTargetGroupsPagesWithContext(context.TODO(), &elbv2.DescribeTargetGroupsInput{}, func(resp *elbv2.DescribeTargetGroupsOutput, lastPage bool) bool {
+		for _, tg := range resp.TargetGroups {
+			targetGroups[aws.StringValue(tg.TargetGroupArn)] = struct{}{}
+		}
+		return true
+	})
+	return targetGroups, err
 }
 
 // tgHasTags returns true if the specified resource has the expected tags.
@@ -322,31 +306,47 @@ func hasTagsASG(tags []*autoscaling.TagDescription, expectedTags map[string]stri
 	return true
 }
 
+func attachTargetGroupsToAutoScalingGroup(svc autoscalingiface.AutoScalingAPI, targetGroupARNs []string, autoScalingGroupName string) error {
+	// You can specify up to 10 target groups,
+	// see https://docs.aws.amazon.com/sdk-for-go/api/service/autoscaling/#AttachLoadBalancerTargetGroupsInput
+	return processChunked(targetGroupARNs, 10, func(chunk []string) error {
+		_, err := svc.AttachLoadBalancerTargetGroups(&autoscaling.AttachLoadBalancerTargetGroupsInput{
+			AutoScalingGroupName: aws.String(autoScalingGroupName),
+			TargetGroupARNs:      aws.StringSlice(chunk),
+		})
+		return err
+	})
+}
+
 func detachTargetGroupsFromAutoScalingGroup(svc autoscalingiface.AutoScalingAPI, targetGroupARNs []string, autoScalingGroupName string) error {
-	// limit target group updates to 10 at a time since this is the limit
-	// in AWS API.
-	chunkSize := 10
+	// You can specify up to 10 target groups,
+	// see https://docs.aws.amazon.com/sdk-for-go/api/service/autoscaling/#DetachLoadBalancerTargetGroupsInput
+	return processChunked(targetGroupARNs, 10, func(chunk []string) error {
+		_, err := svc.DetachLoadBalancerTargetGroups(&autoscaling.DetachLoadBalancerTargetGroupsInput{
+			AutoScalingGroupName: aws.String(autoScalingGroupName),
+			TargetGroupARNs:      aws.StringSlice(chunk),
+		})
+		return err
+	})
+}
 
-	for i := 0; i < len(targetGroupARNs); i += chunkSize {
+// Processes slice in chunks
+// Stops and returns on the first error encountered
+func processChunked(slice []string, chunkSize int, process func(chunk []string) error) error {
+	for i := 0; i < len(slice); i += chunkSize {
 		end := i + chunkSize
 
-		if end > len(targetGroupARNs) {
-			end = len(targetGroupARNs)
+		if end > len(slice) {
+			end = len(slice)
 		}
 
-		targetGroupARNsChunked := targetGroupARNs[i:end]
-		if len(targetGroupARNsChunked) > 0 {
-			params := &autoscaling.DetachLoadBalancerTargetGroupsInput{
-				AutoScalingGroupName: aws.String(autoScalingGroupName),
-				TargetGroupARNs:      aws.StringSlice(targetGroupARNsChunked),
-			}
-			_, err := svc.DetachLoadBalancerTargetGroups(params)
-			if err != nil {
+		chunk := slice[i:end]
+		if len(chunk) > 0 {
+			if err := process(chunk); err != nil {
 				return err
 			}
 		}
 	}
-
 	return nil
 }
 
diff --git a/worker.go b/worker.go
index 6b46a97..3c127be 100644
--- a/worker.go
+++ b/worker.go
@@ -260,7 +260,7 @@ func doWork(
 ) error {
 	defer func() error {
 		if r := recover(); r != nil {
-			log.Errorln("shit has hit the fan:", errors.Wrap(r.(error), "panic caused by"))
+			log.Errorln("Shit has hit the fan:", errors.Wrap(r.(error), "panic caused by"))
 			debug.PrintStack()
 			return r.(error)
 		}
@@ -500,7 +500,7 @@ func createStack(awsAdapter *aws.Adapter, lb *loadBalancer) {
 		certificates = append(certificates, cert)
 	}
 
-	log.Infof("creating stack for certificates %q / ingress %q", certificates, lb.ingresses)
+	log.Infof("Creating stack for certificates %q / ingress %q", certificates, lb.ingresses)
 
 	stackId, err := awsAdapter.CreateStack(certificates, lb.scheme, lb.securityGroup, lb.Owner(), lb.sslPolicy, lb.ipAddressType, lb.wafWebACLID, lb.cwAlarms, lb.loadBalancerType, lb.http2)
 	if err != nil {
@@ -510,24 +510,24 @@ func createStack(awsAdapter *aws.Adapter, lb *loadBalancer) {
 				return
 			}
 		}
-		log.Errorf("createStack(%q) failed: %v", certificates, err)
+		log.Errorf("CreateStack(%q) failed: %v", certificates, err)
 	} else {
-		log.Infof("stack %q for certificates %q created", stackId, certificates)
+		log.Infof("Stack %q for certificates %q created", stackId, certificates)
 	}
 }
 
 func updateStack(awsAdapter *aws.Adapter, lb *loadBalancer) {
 	certificates := lb.CertificateARNs()
 
-	log.Infof("updating %q stack for %d certificates / %d ingresses", lb.scheme, len(certificates), len(lb.ingresses))
+	log.Infof("Updating %q stack for %d certificates / %d ingresses", lb.scheme, len(certificates), len(lb.ingresses))
 
 	stackId, err := awsAdapter.UpdateStack(lb.stack.Name, certificates, lb.scheme, lb.securityGroup, lb.Owner(), lb.sslPolicy, lb.ipAddressType, lb.wafWebACLID, lb.cwAlarms, lb.loadBalancerType, lb.http2)
 	if isNoUpdatesToBePerformedError(err) {
-		log.Debugf("stack(%q) is already up to date", certificates)
+		log.Debugf("Stack(%q) is already up to date", certificates)
 	} else if err != nil {
-		log.Errorf("updateStack(%q) failed: %v", certificates, err)
+		log.Errorf("UpdateStack(%q) failed: %v", certificates, err)
 	} else {
-		log.Infof("stack %q for certificate %q updated", stackId, certificates)
+		log.Infof("Stack %q for certificate %q updated", stackId, certificates)
 	}
 }
 
@@ -568,7 +568,7 @@ func updateIngress(kubeAdapter *kubernetes.Adapter, lb *loadBalancer) {
 					log.Errorf("Failed to update ingress: %v", err)
 				}
 			} else {
-				log.Infof("updated ingress %v with DNS name %q", ing, dnsName)
+				log.Infof("Updated ingress %v with DNS name %q", ing, dnsName)
 			}
 		}
 	}
@@ -577,9 +577,9 @@ func updateIngress(kubeAdapter *kubernetes.Adapter, lb *loadBalancer) {
 func deleteStack(awsAdapter *aws.Adapter, lb *loadBalancer) {
 	stackName := lb.stack.Name
 	if err := awsAdapter.DeleteStack(lb.stack); err != nil {
-		log.Errorf("deleteStack failed to delete stack %q: %v", stackName, err)
+		log.Errorf("Failed to delete stack %q: %v", stackName, err)
 	} else {
-		log.Infof("deleted orphaned stack %q", stackName)
+		log.Infof("Deleted orphaned stack %q", stackName)
 	}
 }
 
@@ -620,7 +620,7 @@ func getCloudWatchAlarmsFromConfigMap(configMap *kubernetes.ConfigMap) aws.Cloud
 
 		list, err := aws.NewCloudWatchAlarmListFromYAML(data)
 		if err != nil {
-			log.Warnf("ignoring cloudwatch alarm configuration from config map key %q due to error: %v", key, err)
+			log.Warnf("Ignoring cloudwatch alarm configuration from config map key %q due to error: %v", key, err)
 			continue
 		}
 


Test patch:
diff --git a/aws/asg_test.go b/aws/asg_test.go
index 7a79683..7565a58 100644
--- a/aws/asg_test.go
+++ b/aws/asg_test.go
@@ -8,6 +8,8 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/elbv2"
+
+	"github.com/stretchr/testify/assert"
 )
 
 type asgtags map[string]string
@@ -183,22 +185,26 @@ func TestGetAutoScalingGroupsByName(t *testing.T) {
 
 func TestAttach(t *testing.T) {
 	for _, test := range []struct {
-		name          string
-		responses     autoscalingMockOutputs
-		elbv2Response elbv2MockOutputs
-		ownerTags     map[string]string
-		wantError     bool
+		name               string
+		targetGroups       []string
+		autoscalingOutputs autoscalingMockOutputs
+		autoscalingInputs  autoscalingMockInputs
+		elbv2Response      elbv2MockOutputs
+		ownerTags          map[string]string
+		wantError          bool
 	}{
 		{
-			name: "describe-lb-target-groups-failed",
-			responses: autoscalingMockOutputs{
+			name:         "describe-lb-target-groups-failed",
+			targetGroups: []string{"foo"},
+			autoscalingOutputs: autoscalingMockOutputs{
 				describeLoadBalancerTargetGroups: R(nil, errDummy),
 			},
 			wantError: true,
 		},
 		{
-			name: "describe-all-target-groups-failed",
-			responses: autoscalingMockOutputs{
+			name:         "describe-all-target-groups-failed",
+			targetGroups: []string{"foo"},
+			autoscalingOutputs: autoscalingMockOutputs{
 				attachLoadBalancerTargetGroups: R(nil, nil),
 				describeLoadBalancerTargetGroups: R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
 					LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
@@ -215,8 +221,9 @@ func TestAttach(t *testing.T) {
 			wantError: true,
 		},
 		{
-			name: "describe-tags-failed",
-			responses: autoscalingMockOutputs{
+			name:         "describe-tags-failed",
+			targetGroups: []string{"foo"},
+			autoscalingOutputs: autoscalingMockOutputs{
 				attachLoadBalancerTargetGroups: R(nil, nil),
 				describeLoadBalancerTargetGroups: R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
 					LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
@@ -240,8 +247,9 @@ func TestAttach(t *testing.T) {
 			wantError: true,
 		},
 		{
-			name: "success-attach",
-			responses: autoscalingMockOutputs{
+			name:         "success-attach",
+			targetGroups: []string{"foo"},
+			autoscalingOutputs: autoscalingMockOutputs{
 				attachLoadBalancerTargetGroups: R(nil, nil),
 				describeLoadBalancerTargetGroups: R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
 					LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
@@ -276,8 +284,9 @@ func TestAttach(t *testing.T) {
 			wantError: false,
 		},
 		{
-			name: "failed-attach",
-			responses: autoscalingMockOutputs{
+			name:         "failed-attach",
+			targetGroups: []string{"foo"},
+			autoscalingOutputs: autoscalingMockOutputs{
 				attachLoadBalancerTargetGroups: R(nil, errDummy),
 				describeLoadBalancerTargetGroups: R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
 					LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
@@ -313,8 +322,9 @@ func TestAttach(t *testing.T) {
 			wantError: true,
 		},
 		{
-			name: "detach-obsolete",
-			responses: autoscalingMockOutputs{
+			name:         "detach-obsolete",
+			targetGroups: []string{"foo"},
+			autoscalingOutputs: autoscalingMockOutputs{
 				attachLoadBalancerTargetGroups: R(nil, nil),
 				describeLoadBalancerTargetGroups: R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
 					LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
@@ -383,8 +393,9 @@ func TestAttach(t *testing.T) {
 			wantError: false,
 		},
 		{
-			name: "failed-detach",
-			responses: autoscalingMockOutputs{
+			name:         "failed-detach",
+			targetGroups: []string{"foo"},
+			autoscalingOutputs: autoscalingMockOutputs{
 				attachLoadBalancerTargetGroups: R(nil, nil),
 				describeLoadBalancerTargetGroups: R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
 					LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
@@ -435,11 +446,59 @@ func TestAttach(t *testing.T) {
 			ownerTags: map[string]string{"owner": "true"},
 			wantError: true,
 		},
+		{
+			name:         "attach ignores nonexistent target groups",
+			targetGroups: []string{"foo", "void", "bar", "blank"},
+			autoscalingOutputs: autoscalingMockOutputs{
+				attachLoadBalancerTargetGroups: R(nil, nil),
+				describeLoadBalancerTargetGroups: R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
+					LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
+						{
+							LoadBalancerTargetGroupARN: aws.String("foo"),
+						},
+						{
+							LoadBalancerTargetGroupARN: aws.String("bar"),
+						},
+					},
+				}, nil)},
+			elbv2Response: elbv2MockOutputs{
+				describeTargetGroups: R(&elbv2.DescribeTargetGroupsOutput{
+					TargetGroups: []*elbv2.TargetGroup{
+						{
+							TargetGroupArn: aws.String("foo"),
+						},
+						{
+							TargetGroupArn: aws.String("bar"),
+						},
+					},
+				}, nil),
+				describeTags: R(&elbv2.DescribeTagsOutput{
+					TagDescriptions: []*elbv2.TagDescription{
+						{
+							ResourceArn: aws.String("foo"),
+							Tags:        []*elbv2.Tag{{Key: aws.String("owner"), Value: aws.String("true")}},
+						},
+						{
+							ResourceArn: aws.String("bar"),
+							Tags:        []*elbv2.Tag{{Key: aws.String("owner"), Value: aws.String("true")}},
+						},
+					},
+				}, nil),
+			},
+			autoscalingInputs: autoscalingMockInputs{
+				attachLoadBalancerTargetGroups: func(t *testing.T, input *autoscaling.AttachLoadBalancerTargetGroupsInput) {
+					assert.Equal(t, aws.String("asg-name"), input.AutoScalingGroupName)
+					assert.Equal(t, aws.StringSlice([]string{"foo", "bar"}), input.TargetGroupARNs)
+				},
+			},
+			ownerTags: map[string]string{"owner": "true"},
+			wantError: false,
+		},
 	} {
-		t.Run(fmt.Sprintf("%v", test.name), func(t *testing.T) {
-			mockSvc := &mockAutoScalingClient{outputs: test.responses}
+		t.Run(test.name, func(t *testing.T) {
+			mockSvc := &mockAutoScalingClient{outputs: test.autoscalingOutputs, inputs: test.autoscalingInputs, t: t}
 			mockElbv2Svc := &mockElbv2Client{outputs: test.elbv2Response}
-			err := updateTargetGroupsForAutoScalingGroup(mockSvc, mockElbv2Svc, []string{"foo"}, "bar", test.ownerTags)
+			err := updateTargetGroupsForAutoScalingGroup(mockSvc, mockElbv2Svc, test.targetGroups, "asg-name", test.ownerTags)
 			if test.wantError {
 				if err == nil {
 					t.Error("wanted an error but call seemed to have succeeded")
@@ -451,7 +510,6 @@ func TestAttach(t *testing.T) {
 			}
 		})
 	}
-
 }
 
 func TestDetach(t *testing.T) {
@@ -523,6 +581,16 @@ func TestTestFilterTags(t *testing.T) {
 			},
 			true,
 		},
+		{
+			"test value mismatch",
+			"mycluster",
+			"tag:kubernetes.io/cluster/mycluster=owned tag:custom.com/ingress=owned",
+			map[string]string{
+				"kubernetes.io/cluster/mycluster": "owned",
+				"custom.com/ingress":              "whatever",
+			},
+			false,
+		},
 	} {
 		t.Run(fmt.Sprintf("%v", test.name), func(t *testing.T) {
 			a := &Adapter{customFilter: test.customFilter}
@@ -535,3 +603,74 @@ func TestTestFilterTags(t *testing.T) {
 		})
 	}
 }
+
+type testChunkProcessor struct {
+	failOn int
+	chunks [][]string
+}
+
+func (p *testChunkProcessor) process(chunk []string) error {
+	if p.failOn-1 == len(p.chunks) {
+		return fmt.Errorf("failing on %d chunk", p.failOn)
+	}
+	p.chunks = append(p.chunks, chunk)
+	return nil
+}
+
+func TestProcessChunked(t *testing.T) {
+	const chunkSize = 5
+
+	for _, ti := range []struct {
+		name   string
+		input  []string
+		failOn int
+		expect [][]string
+		err    bool
+	}{
+		{
+			name:   "empty",
+			input:  nil,
+			expect: nil,
+		},
+		{
+			name:   "less than chunk size",
+			input:  []string{"1", "2", "3", "4"},
+			expect: [][]string{[]string{"1", "2", "3", "4"}},
+		},
+		{
+			name:   "equal to chunk size",
+			input:  []string{"1", "2", "3", "4", "5"},
+			expect: [][]string{[]string{"1", "2", "3", "4", "5"}},
+		},
+		{
+			name:   "greater than chunk size",
+			input:  []string{"1", "2", "3", "4", "5", "6"},
+			expect: [][]string{[]string{"1", "2", "3", "4", "5"}, []string{"6"}},
+		},
+		{
+			name:   "fail on first chunk",
+			input:  []string{"1", "2", "3", "4", "5", "6"},
+			failOn: 1,
+			expect: nil,
+			err:    true,
+		},
+		{
+			name:   "fail on second chunk",
+			input:  []string{"1", "2", "3", "4", "5", "6"},
+			failOn: 2,
+			expect: [][]string{[]string{"1", "2", "3", "4", "5"}},
+			err:    true,
+		},
+	} {
+		t.Run(ti.name, func(t *testing.T) {
+			cp := testChunkProcessor{failOn: ti.failOn}
+			err := processChunked(ti.input, chunkSize, cp.process)
+			if ti.err {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, ti.expect, cp.chunks)
+		})
+	}
+}
diff --git a/aws/asgmock_test.go b/aws/asgmock_test.go
index de16cf3..592ec5f 100644
--- a/aws/asgmock_test.go
+++ b/aws/asgmock_test.go
@@ -1,11 +1,17 @@
 package aws
 
 import (
+	"testing"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface"
 )
 
+type autoscalingMockInputs struct {
+	attachLoadBalancerTargetGroups func(*testing.T, *autoscaling.AttachLoadBalancerTargetGroupsInput)
+}
+
 type autoscalingMockOutputs struct {
 	describeAutoScalingGroups        *apiResponse
 	attachLoadBalancerTargetGroups   *apiResponse
@@ -16,6 +22,8 @@ type autoscalingMockOutputs struct {
 type mockAutoScalingClient struct {
 	autoscalingiface.AutoScalingAPI
 	outputs autoscalingMockOutputs
+	inputs  autoscalingMockInputs
+	t       *testing.T
 }
 
 func (m *mockAutoScalingClient) DescribeAutoScalingGroups(*autoscaling.DescribeAutoScalingGroupsInput) (*autoscaling.DescribeAutoScalingGroupsOutput, error) {
@@ -39,7 +47,10 @@ func (m *mockAutoScalingClient) DescribeLoadBalancerTargetGroups(*autoscaling.De
 	return nil, m.outputs.describeLoadBalancerTargetGroups.err
 }
 
-func (m *mockAutoScalingClient) AttachLoadBalancerTargetGroups(*autoscaling.AttachLoadBalancerTargetGroupsInput) (*autoscaling.AttachLoadBalancerTargetGroupsOutput, error) {
+func (m *mockAutoScalingClient) AttachLoadBalancerTargetGroups(input *autoscaling.AttachLoadBalancerTargetGroupsInput) (*autoscaling.AttachLoadBalancerTargetGroupsOutput, error) {
+	if m.inputs.attachLoadBalancerTargetGroups != nil {
+		m.inputs.attachLoadBalancerTargetGroups(m.t, input)
+	}
 	return nil, m.outputs.attachLoadBalancerTargetGroups.err
 }