Merge pull request #16339 from justinsb/refactor_targetgroups

target group: refactor discovery into awsup

cloudmock/aws/mockelb/tags.go

@@ -17,14 +17,20 @@ limitations under the License.
 package mockelb
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/request"
 	"github.com/aws/aws-sdk-go/service/elb"
 	"k8s.io/klog/v2"
 )
 
 func (m *MockELB) DescribeTags(request *elb.DescribeTagsInput) (*elb.DescribeTagsOutput, error) {
+	return m.DescribeTagsWithContext(context.TODO(), request)
+}
+
+func (m *MockELB) DescribeTagsWithContext(ctx aws.Context, request *elb.DescribeTagsInput, opt ...request.Option) (*elb.DescribeTagsOutput, error) {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 

cloudmock/aws/mockelbv2/targetgroups.go

@@ -27,7 +27,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-func (m *MockELBV2) DescribeTargetGroups(request *elbv2.DescribeTargetGroupsInput) (*elbv2.DescribeTargetGroupsOutput, error) {
+func (m *MockELBV2) DescribeTargetGroupsWithContext(ctx context.Context, request *elbv2.DescribeTargetGroupsInput, opts ...request.Option) (*elbv2.DescribeTargetGroupsOutput, error) {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
@@ -79,7 +79,7 @@ func (m *MockELBV2) DescribeTargetGroups(request *elbv2.DescribeTargetGroupsInpu
 }
 
 func (m *MockELBV2) DescribeTargetGroupsPagesWithContext(ctx context.Context, request *elbv2.DescribeTargetGroupsInput, callback func(p *elbv2.DescribeTargetGroupsOutput, lastPage bool) (shouldContinue bool), opt ...request.Option) error {
-	page, err := m.DescribeTargetGroups(request)
+	page, err := m.DescribeTargetGroupsWithContext(ctx, request)
 	if err != nil {
 		return err
 	}

pkg/resources/aws/aws.go

@@ -1613,17 +1613,18 @@ func DumpTargetGroup(op *resources.DumpOperation, r *resources.Resource) error {
 }
 
 func ListTargetGroups(cloud fi.Cloud, vpcID, clusterName string) ([]*resources.Resource, error) {
-	targetgroups, _, err := DescribeTargetGroups(cloud)
+	targetGroups, err := listMatchingTargetGroups(cloud)
 	if err != nil {
 		return nil, err
 	}
 
 	var resourceTrackers []*resources.Resource
-	for _, tg := range targetgroups {
+	for _, targetGroup := range targetGroups {
+		tg := targetGroup.TargetGroup
 		id := aws.StringValue(tg.TargetGroupName)
 		resourceTracker := &resources.Resource{
 			Name:    id,
-			ID:      string(*tg.TargetGroupArn),
+			ID:      targetGroup.ARN(),
 			Type:    TypeTargetGroup,
 			Deleter: DeleteTargetGroup,
 			Dumper:  DumpTargetGroup,
@@ -1635,62 +1636,27 @@ func ListTargetGroups(cloud fi.Cloud, vpcID, clusterName string) ([]*resources.R
 	return resourceTrackers, nil
 }
 
-func DescribeTargetGroups(cloud fi.Cloud) ([]*elbv2.TargetGroup, map[string][]*elbv2.Tag, error) {
+func listMatchingTargetGroups(cloud fi.Cloud) ([]*awsup.TargetGroupInfo, error) {
+	ctx := context.TODO()
+
 	c := cloud.(awsup.AWSCloud)
 	tags := c.Tags()
 
 	klog.V(2).Infof("Listing all TargetGroups")
 
-	request := &elbv2.DescribeTargetGroupsInput{}
-	// DescribeTags has a limit of 20 names, so we set the page size here to 20 also
-	request.PageSize = aws.Int64(20)
-
-	var targetgroups []*elbv2.TargetGroup
-	targetgroupTags := make(map[string][]*elbv2.Tag)
-
-	var innerError error
-	err := c.ELBV2().DescribeTargetGroupsPages(request, func(p *elbv2.DescribeTargetGroupsOutput, lastPage bool) bool {
-		if len(p.TargetGroups) == 0 {
-			return true
-		}
-
-		tagRequest := &elbv2.DescribeTagsInput{}
-
-		nameToTargetGroup := make(map[string]*elbv2.TargetGroup)
-		for _, tg := range p.TargetGroups {
-			name := aws.StringValue(tg.TargetGroupArn)
-			nameToTargetGroup[name] = tg
-
-			tagRequest.ResourceArns = append(tagRequest.ResourceArns, tg.TargetGroupArn)
-		}
-
-		tagResponse, err := c.ELBV2().DescribeTags(tagRequest)
-		if err != nil {
-			innerError = fmt.Errorf("error listing TargetGroup Tags: %v", err)
-			return false
-		}
-
-		for _, t := range tagResponse.TagDescriptions {
-			tgARN := aws.StringValue(t.ResourceArn)
-			if !matchesElbV2Tags(tags, t.Tags) {
-				continue
-			}
-			targetgroupTags[tgARN] = t.Tags
-
-			tg := nameToTargetGroup[tgARN]
-			targetgroups = append(targetgroups, tg)
-		}
-
-		return true
-	})
+	targetGroups, err := awsup.ListELBV2TargetGroups(ctx, c)
 	if err != nil {
-		return nil, nil, fmt.Errorf("error describing TargetGroups: %v", err)
+		return nil, err
 	}
-	if innerError != nil {
-		return nil, nil, fmt.Errorf("error describing TargetGroups: %v", innerError)
+
+	var matches []*awsup.TargetGroupInfo
+	for _, tg := range targetGroups {
+		if matchesElbV2Tags(tags, tg.Tags) {
+			matches = append(matches, tg)
+		}
 	}
 
-	return targetgroups, targetgroupTags, nil
+	return matches, nil
 }
 
 func DeleteElasticIP(cloud fi.Cloud, t *resources.Resource) error {

upup/pkg/fi/cloudup/awstasks/targetgroup.go

@@ -17,8 +17,10 @@ limitations under the License.
 package awstasks
 
 import (
+	"context"
 	"fmt"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"k8s.io/klog/v2"
@@ -51,7 +53,7 @@ type TargetGroup struct {
 	// ARN is the Amazon Resource Name for the Target Group
 	ARN *string
 
-	// Shared is set if this is an external LB (one we don't create or own)
+	// Shared is set if this is an external TargetGroup (one we don't create or own)
 	Shared *bool
 
 	Attributes map[string]string
@@ -67,33 +69,95 @@ func (e *TargetGroup) CompareWithID() *string {
 	return e.ARN
 }
 
-func (e *TargetGroup) Find(c *fi.CloudupContext) (*TargetGroup, error) {
-	cloud := c.T.Cloud.(awsup.AWSCloud)
+func (e *TargetGroup) findTargetGroupByName(ctx context.Context, cloud awsup.AWSCloud) (*awsup.TargetGroupInfo, error) {
+	name := fi.ValueOf(e.Name)
 
-	request := &elbv2.DescribeTargetGroupsInput{}
-	if e.ARN != nil {
-		request.TargetGroupArns = []*string{e.ARN}
-	} else if e.Name != nil {
-		request.Names = []*string{e.Name}
+	targetGroups, err := awsup.ListELBV2TargetGroups(ctx, cloud)
+	if err != nil {
+		return nil, err
 	}
 
-	response, err := cloud.ELBV2().DescribeTargetGroups(request)
-	if err != nil {
+	var latest *awsup.TargetGroupInfo
+	for _, targetGroup := range targetGroups {
+		// We accept the name tag _or_ the TargetGroupName itself, to allow matching groups that might predate tagging.
+		if aws.StringValue(targetGroup.TargetGroup.TargetGroupName) != name && targetGroup.NameTag() != name {
+			continue
+		}
+		if latest != nil {
+			return nil, fmt.Errorf("found multiple TargetGroups with name %q, expected 1", fi.ValueOf(e.Name))
+		}
+		latest = targetGroup
+	}
+
+	return latest, nil
+}
+
+func (e *TargetGroup) findTargetGroupByARN(ctx context.Context, cloud awsup.AWSCloud) (*awsup.TargetGroupInfo, error) {
+	request := &elbv2.DescribeTargetGroupsInput{}
+	request.TargetGroupArns = []*string{e.ARN}
+
+	var targetGroups []*elbv2.TargetGroup
+	if err := cloud.ELBV2().DescribeTargetGroupsPagesWithContext(ctx, request, func(page *elbv2.DescribeTargetGroupsOutput, lastPage bool) bool {
+		targetGroups = append(targetGroups, page.TargetGroups...)
+		return true
+	}); err != nil {
 		if aerr, ok := err.(awserr.Error); ok && aerr.Code() == elbv2.ErrCodeTargetGroupNotFoundException {
 			if !fi.ValueOf(e.Shared) {
 				return nil, nil
 			}
 		}
-		return nil, fmt.Errorf("error describing targetgroup %s: %v", *e.Name, err)
+		return nil, fmt.Errorf("error describing targetgroup %s: %w", *e.ARN, err)
+	}
+	if len(targetGroups) > 1 {
+		return nil, fmt.Errorf("found %d TargetGroups with ID %q, expected 1", len(targetGroups), fi.ValueOf(e.Name))
+	} else if len(targetGroups) == 0 {
+		return nil, nil
+	}
+	tg := targetGroups[0]
+
+	tagResponse, err := cloud.ELBV2().DescribeTagsWithContext(ctx, &elbv2.DescribeTagsInput{
+		ResourceArns: []*string{tg.TargetGroupArn},
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	info := &awsup.TargetGroupInfo{
+		TargetGroup: tg,
 	}
 
-	if len(response.TargetGroups) > 1 {
-		return nil, fmt.Errorf("found %d TargetGroups with ID %q, expected 1", len(response.TargetGroups), fi.ValueOf(e.Name))
-	} else if len(response.TargetGroups) == 0 {
+	for _, t := range tagResponse.TagDescriptions {
+		info.Tags = append(info.Tags, t.Tags...)
+	}
+
+	return info, nil
+}
+
+func (e *TargetGroup) Find(c *fi.CloudupContext) (*TargetGroup, error) {
+	ctx := c.Context()
+	cloud := c.T.Cloud.(awsup.AWSCloud)
+
+	var targetGroupInfo *awsup.TargetGroupInfo
+
+	if e.ARN == nil {
+		tgi, err := e.findTargetGroupByName(ctx, cloud)
+		if err != nil {
+			return nil, err
+		}
+		targetGroupInfo = tgi
+	} else {
+		tgi, err := e.findTargetGroupByARN(ctx, cloud)
+		if err != nil {
+			return nil, err
+		}
+		targetGroupInfo = tgi
+	}
+
+	if targetGroupInfo == nil {
 		return nil, nil
 	}
 
-	tg := response.TargetGroups[0]
+	tg := targetGroupInfo.TargetGroup
 
 	actual := &TargetGroup{
 		Name:               tg.TargetGroupName,
@@ -110,17 +174,11 @@ func (e *TargetGroup) Find(c *fi.CloudupContext) (*TargetGroup, error) {
 
 	e.ARN = tg.TargetGroupArn
 
-	tagsResp, err := cloud.ELBV2().DescribeTags(&elbv2.DescribeTagsInput{
-		ResourceArns: []*string{tg.TargetGroupArn},
-	})
-	if err != nil {
-		return nil, err
-	}
 	tags := make(map[string]string)
-	for _, tagDesc := range tagsResp.TagDescriptions {
-		for _, tag := range tagDesc.Tags {
-			tags[fi.ValueOf(tag.Key)] = fi.ValueOf(tag.Value)
-		}
+	for _, tag := range targetGroupInfo.Tags {
+		k := fi.ValueOf(tag.Key)
+		v := fi.ValueOf(tag.Value)
+		tags[k] = v
 	}
 	actual.Tags = tags
 
@@ -144,6 +202,10 @@ func (e *TargetGroup) Find(c *fi.CloudupContext) (*TargetGroup, error) {
 	actual.Lifecycle = e.Lifecycle
 	actual.Shared = e.Shared
 
+	if e.Name != nil {
+		actual.Name = e.Name
+	}
+
 	return actual, nil
 }
 

upup/pkg/fi/cloudup/awsup/elbv2_loadbalancers.go

@@ -29,11 +29,12 @@ import (
 type LoadBalancerInfo struct {
 	LoadBalancer *elbv2.LoadBalancer
 	Tags         []*elbv2.Tag
+	arn          string
 }
 
 // ARN returns the ARN of the load balancer.
 func (i *LoadBalancerInfo) ARN() string {
-	return aws.StringValue(i.LoadBalancer.LoadBalancerArn)
+	return i.arn
 }
 
 // NameTag returns the value of the tag with the key "Name".
@@ -72,7 +73,7 @@ func ListELBV2LoadBalancers(ctx context.Context, cloud AWSCloud) ([]*LoadBalance
 
 		for _, elb := range p.LoadBalancers {
 			arn := aws.StringValue(elb.LoadBalancerArn)
-			byARN[arn] = &LoadBalancerInfo{LoadBalancer: elb}
+			byARN[arn] = &LoadBalancerInfo{LoadBalancer: elb, arn: arn}
 
 			// TODO: Any way to filter by cluster here?