Skip to content

rewrite TargetGroup filter using ELBV2 tag operation #1366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

rewrite TargetGroup filter using ELBV2 tag operation #1366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
89b5442
rewrite TargetGroup filter using ELBV2 DescribeTargetGroups API
Fedomn Aug 20, 2020
78594e6
fix dangling TargetGroups by fetching TargetGroups before reconcile a…
Fedomn Aug 28, 2020
0a80fea
Revert "fix dangling TargetGroups by fetching TargetGroups before rec…
Fedomn Nov 14, 2020
4ba8b59
Revert "rewrite TargetGroup filter using ELBV2 DescribeTargetGroups API"
Fedomn Nov 14, 2020
269388c
use ELBv2 tag operation to filter TargetGroups, same solution #1387
Fedomn Nov 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion internal/alb/tg/targetgroup_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,12 @@ func (controller *defaultGroupController) GC(ctx context.Context, tgGroup Target
for _, tg := range tgGroup.TGByBackend {
usedServiceTGARNs.Insert(tg.Arn)
}
arns, err := controller.cloud.GetResourcesByFilters(tagFilters, aws.ResourceTypeEnumELBTargetGroup)

targetGroups, err := controller.cloud.GetTargetGroupsByTagFilters(ctx, tagFilters)
arns := make([]string, 0, len(targetGroups))
for _, tg := range targetGroups {
arns = append(arns, aws.StringValue(tg.TargetGroupArn))
}
if err != nil {
return fmt.Errorf("failed to get targetGroups due to %v", err)
}
Expand Down
104 changes: 58 additions & 46 deletions internal/alb/tg/targetgroup_group_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ type TGReconcileCall struct {
Err error
}

type GetResourcesByFiltersCall struct {
type GetTargetGroupsByTagFiltersCall struct {
TagFilters map[string][]string
ResourceType string
Arns []string
TargetGroups []*elbv2.TargetGroup
Err error
}

Expand Down Expand Up @@ -536,11 +535,11 @@ func TestDefaultGroupController_Reconcile(t *testing.T) {

func TestDefaultGroupController_GC(t *testing.T) {
for _, tc := range []struct {
Name string
TGGroup TargetGroupGroup
GetResourcesByFiltersCall *GetResourcesByFiltersCall
DeleteTargetGroupByArnCalls []DeleteTargetGroupByArnCall
ExpectedError error
Name string
TGGroup TargetGroupGroup
GetTargetGroupsByTagFiltersCall *GetTargetGroupsByTagFiltersCall
DeleteTargetGroupByArnCalls []DeleteTargetGroupByArnCall
ExpectedError error
}{
{
Name: "GC succeeds",
Expand All @@ -553,10 +552,13 @@ func TestDefaultGroupController_GC(t *testing.T) {
},
selector: map[string]string{"key1": "value1", "key2": "value2"},
},
GetResourcesByFiltersCall: &GetResourcesByFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
ResourceType: aws.ResourceTypeEnumELBTargetGroup,
Arns: []string{"arn1", "arn2", "arn3"},
GetTargetGroupsByTagFiltersCall: &GetTargetGroupsByTagFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
TargetGroups: []*elbv2.TargetGroup{
{TargetGroupArn: aws.String("arn1")},
{TargetGroupArn: aws.String("arn2")},
{TargetGroupArn: aws.String("arn3")},
},
},
DeleteTargetGroupByArnCalls: []DeleteTargetGroupByArnCall{
{
Expand All @@ -579,10 +581,13 @@ func TestDefaultGroupController_GC(t *testing.T) {
externalTGARNs: []string{"arn3"},
selector: map[string]string{"key1": "value1", "key2": "value2"},
},
GetResourcesByFiltersCall: &GetResourcesByFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
ResourceType: aws.ResourceTypeEnumELBTargetGroup,
Arns: []string{"arn1", "arn2", "arn3"},
GetTargetGroupsByTagFiltersCall: &GetTargetGroupsByTagFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
TargetGroups: []*elbv2.TargetGroup{
{TargetGroupArn: aws.String("arn1")},
{TargetGroupArn: aws.String("arn2")},
{TargetGroupArn: aws.String("arn3")},
},
},
DeleteTargetGroupByArnCalls: []DeleteTargetGroupByArnCall{
{
Expand All @@ -601,10 +606,9 @@ func TestDefaultGroupController_GC(t *testing.T) {
},
selector: map[string]string{"key1": "value1", "key2": "value2"},
},
GetResourcesByFiltersCall: &GetResourcesByFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
ResourceType: aws.ResourceTypeEnumELBTargetGroup,
Err: errors.New("GetResourcesByFiltersCall"),
GetTargetGroupsByTagFiltersCall: &GetTargetGroupsByTagFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
Err: errors.New("GetResourcesByFiltersCall"),
},
ExpectedError: errors.New("failed to get targetGroups due to GetResourcesByFiltersCall"),
},
Expand All @@ -619,10 +623,13 @@ func TestDefaultGroupController_GC(t *testing.T) {
},
selector: map[string]string{"key1": "value1", "key2": "value2"},
},
GetResourcesByFiltersCall: &GetResourcesByFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
ResourceType: aws.ResourceTypeEnumELBTargetGroup,
Arns: []string{"arn1", "arn2", "arn3"},
GetTargetGroupsByTagFiltersCall: &GetTargetGroupsByTagFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
TargetGroups: []*elbv2.TargetGroup{
{TargetGroupArn: aws.String("arn1")},
{TargetGroupArn: aws.String("arn2")},
{TargetGroupArn: aws.String("arn3")},
},
},
DeleteTargetGroupByArnCalls: []DeleteTargetGroupByArnCall{
{
Expand All @@ -635,8 +642,8 @@ func TestDefaultGroupController_GC(t *testing.T) {
} {
ctx := context.Background()
cloud := &mocks.CloudAPI{}
if tc.GetResourcesByFiltersCall != nil {
cloud.On("GetResourcesByFilters", tc.GetResourcesByFiltersCall.TagFilters, tc.GetResourcesByFiltersCall.ResourceType).Return(tc.GetResourcesByFiltersCall.Arns, tc.GetResourcesByFiltersCall.Err)
if tc.GetTargetGroupsByTagFiltersCall != nil {
cloud.On("GetTargetGroupsByTagFilters", ctx, tc.GetTargetGroupsByTagFiltersCall.TagFilters).Return(tc.GetTargetGroupsByTagFiltersCall.TargetGroups, tc.GetTargetGroupsByTagFiltersCall.Err)
}
for _, call := range tc.DeleteTargetGroupByArnCalls {
cloud.On("DeleteTargetGroupByArn", ctx, call.Arn).Return(call.Err)
Expand All @@ -663,12 +670,12 @@ func TestDefaultGroupController_GC(t *testing.T) {

func TestDefaultGroupController_Delete(t *testing.T) {
for _, tc := range []struct {
Name string
IngressKey types.NamespacedName
TagTGGroupCall *TagTGGroupCall
GetResourcesByFiltersCall *GetResourcesByFiltersCall
DeleteTargetGroupByArnCalls []DeleteTargetGroupByArnCall
ExpectedError error
Name string
IngressKey types.NamespacedName
TagTGGroupCall *TagTGGroupCall
GetTargetGroupsByTagFiltersCall *GetTargetGroupsByTagFiltersCall
DeleteTargetGroupByArnCalls []DeleteTargetGroupByArnCall
ExpectedError error
}{
{
Name: "DELETE succeeds",
Expand All @@ -681,10 +688,13 @@ func TestDefaultGroupController_Delete(t *testing.T) {
IngressName: "ingress",
Tags: map[string]string{"key1": "value1", "key2": "value2"},
},
GetResourcesByFiltersCall: &GetResourcesByFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
ResourceType: aws.ResourceTypeEnumELBTargetGroup,
Arns: []string{"arn1", "arn2", "arn3"},
GetTargetGroupsByTagFiltersCall: &GetTargetGroupsByTagFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
TargetGroups: []*elbv2.TargetGroup{
{TargetGroupArn: aws.String("arn1")},
{TargetGroupArn: aws.String("arn2")},
{TargetGroupArn: aws.String("arn3")},
},
},
DeleteTargetGroupByArnCalls: []DeleteTargetGroupByArnCall{
{
Expand All @@ -709,10 +719,9 @@ func TestDefaultGroupController_Delete(t *testing.T) {
IngressName: "ingress",
Tags: map[string]string{"key1": "value1", "key2": "value2"},
},
GetResourcesByFiltersCall: &GetResourcesByFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
ResourceType: aws.ResourceTypeEnumELBTargetGroup,
Err: errors.New("GetResourcesByFiltersCall"),
GetTargetGroupsByTagFiltersCall: &GetTargetGroupsByTagFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
Err: errors.New("GetResourcesByFiltersCall"),
},
ExpectedError: errors.New("failed to get targetGroups due to GetResourcesByFiltersCall"),
},
Expand All @@ -727,10 +736,13 @@ func TestDefaultGroupController_Delete(t *testing.T) {
IngressName: "ingress",
Tags: map[string]string{"key1": "value1", "key2": "value2"},
},
GetResourcesByFiltersCall: &GetResourcesByFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
ResourceType: aws.ResourceTypeEnumELBTargetGroup,
Arns: []string{"arn1", "arn2", "arn3"},
GetTargetGroupsByTagFiltersCall: &GetTargetGroupsByTagFiltersCall{
TagFilters: map[string][]string{"key1": {"value1"}, "key2": {"value2"}},
TargetGroups: []*elbv2.TargetGroup{
{TargetGroupArn: aws.String("arn1")},
{TargetGroupArn: aws.String("arn2")},
{TargetGroupArn: aws.String("arn3")},
},
},
DeleteTargetGroupByArnCalls: []DeleteTargetGroupByArnCall{
{
Expand All @@ -743,8 +755,8 @@ func TestDefaultGroupController_Delete(t *testing.T) {
} {
ctx := context.Background()
cloud := &mocks.CloudAPI{}
if tc.GetResourcesByFiltersCall != nil {
cloud.On("GetResourcesByFilters", tc.GetResourcesByFiltersCall.TagFilters, tc.GetResourcesByFiltersCall.ResourceType).Return(tc.GetResourcesByFiltersCall.Arns, tc.GetResourcesByFiltersCall.Err)
if tc.GetTargetGroupsByTagFiltersCall != nil {
cloud.On("GetTargetGroupsByTagFilters", ctx, tc.GetTargetGroupsByTagFiltersCall.TagFilters).Return(tc.GetTargetGroupsByTagFiltersCall.TargetGroups, tc.GetTargetGroupsByTagFiltersCall.Err)
}
for _, call := range tc.DeleteTargetGroupByArnCalls {
cloud.On("DeleteTargetGroupByArn", ctx, call.Arn).Return(call.Err)
Expand Down
64 changes: 64 additions & 0 deletions internal/aws/elbv2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/service/elbv2"
"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/utils"
util "github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util/types"
)

type ELBV2API interface {
Expand Down Expand Up @@ -36,6 +38,9 @@ type ELBV2API interface {
// GetTargetGroupByName retrieve TargetGroup instance by name
GetTargetGroupByName(context.Context, string) (*elbv2.TargetGroup, error)

// GetTargetGroupsByTagFilters retrieve TargetGroups instance by tags
GetTargetGroupsByTagFilters(ctx context.Context, tagFilters map[string][]string) ([]*elbv2.TargetGroup, error)

// DeleteTargetGroupByArn deletes TargetGroup instance by arn
DeleteTargetGroupByArn(context.Context, string) error

Expand Down Expand Up @@ -291,6 +296,35 @@ func (c *Cloud) GetTargetGroupByName(ctx context.Context, name string) (*elbv2.T
return targetGroups[0], nil
}

// GetTargetGroupsByTagFilters retrieve TargetGroups instance by tags
func (c *Cloud) GetTargetGroupsByTagFilters(ctx context.Context, tagFilters map[string][]string) ([]*elbv2.TargetGroup, error) {
req := &elbv2.DescribeTargetGroupsInput{}
tgs, err := c.describeTargetGroupsHelper(req)
if err != nil {
return nil, err
}

tgARNs := make([]string, 0, len(tgs))
tgByARN := make(map[string]*elbv2.TargetGroup, len(tgs))
for _, tg := range tgs {
tgARN := aws.StringValue(tg.TargetGroupArn)
tgARNs = append(tgARNs, tgARN)
tgByARN[tgARN] = tg
}
tagsByARN, err := c.describeResourceTags(ctx, tgARNs)
if err != nil {
return nil, err
}

var matchedTGs []*elbv2.TargetGroup
for _, arn := range tgARNs {
if util.Matches(tagsByARN[arn], tagFilters) {
matchedTGs = append(matchedTGs, tgByARN[arn])
}
}
return matchedTGs, nil
}

// DeleteTargetGroupByArn deletes TargetGroup instance by arn
func (c *Cloud) DeleteTargetGroupByArn(ctx context.Context, arn string) error {
_, err := c.elbv2.DeleteTargetGroupWithContext(ctx, &elbv2.DeleteTargetGroupInput{
Expand Down Expand Up @@ -322,3 +356,33 @@ func (c *Cloud) describeTargetGroupsHelper(input *elbv2.DescribeTargetGroupsInpu
})
return result, err
}

const (
// ELBV2 API supports up to 20 resource per DescribeTags API call.
defaultDescribeTagsChunkSize = 20
)

// describeResourceTags describes tags for elbv2 resources.
// returns tags indexed by resource ARN.
func (c *Cloud) describeResourceTags(ctx context.Context, arns []string) (map[string]map[string]string, error) {
tagsByARN := make(map[string]map[string]string, len(arns))
arnsChunks := utils.SplitStringSlice(arns, defaultDescribeTagsChunkSize)
for _, arnsChunk := range arnsChunks {
req := &elbv2.DescribeTagsInput{
ResourceArns: aws.StringSlice(arnsChunk),
}

resp, err := c.elbv2.DescribeTagsWithContext(ctx, req)
if err != nil {
return nil, err
}
for _, tagDescription := range resp.TagDescriptions {
tags := make(map[string]string, len(tagDescription.Tags))
for _, sdkTag := range tagDescription.Tags {
tags[aws.StringValue(sdkTag.Key)] = aws.StringValue(sdkTag.Value)
}
tagsByARN[aws.StringValue(tagDescription.ResourceArn)] = tags
}
}
return tagsByARN, nil
}
Loading