Repository: kubernetes-sigs/aws-alb-ingress-controller
Pull request #1411 (2020-09-11 21:09:23)

add describeSubnetsAsList API
1. add a describeSubnetsAsList to handle subnets pagination.
2. make vpc subnet resolver to use it.

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/aws-alb-ingress-controller/pull/1411#" title="Author self-approved">M00nF1sh</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign 
You can assign the PR to them by writing `/assign ` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Faws-alb-ingress-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/aws-alb-ingress-controller/blob/v2_ga/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/mocks/aws/services/mock_ec2.go b/mocks/aws/services/mock_ec2.go
index 8ec46d31c..1f5ae3517 100644
--- a/mocks/aws/services/mock_ec2.go
+++ b/mocks/aws/services/mock_ec2.go
@@ -13676,6 +13676,21 @@ func (mr *MockEC2MockRecorder) DescribeSubnets(arg0 interface{}) *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeSubnets", reflect.TypeOf((*MockEC2)(nil).DescribeSubnets), arg0)
 }
 
+// DescribeSubnetsAsList mocks base method
+func (m *MockEC2) DescribeSubnetsAsList(arg0 context.Context, arg1 *ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "DescribeSubnetsAsList", arg0, arg1)
+	ret0, _ := ret[0].([]*ec2.Subnet)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// DescribeSubnetsAsList indicates an expected call of DescribeSubnetsAsList
+func (mr *MockEC2MockRecorder) DescribeSubnetsAsList(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeSubnetsAsList", reflect.TypeOf((*MockEC2)(nil).DescribeSubnetsAsList), arg0, arg1)
+}
+
 // DescribeSubnetsPages mocks base method
 func (m *MockEC2) DescribeSubnetsPages(arg0 *ec2.DescribeSubnetsInput, arg1 func(*ec2.DescribeSubnetsOutput, bool) bool) error {
 	m.ctrl.T.Helper()
diff --git a/pkg/aws/services/ec2.go b/pkg/aws/services/ec2.go
index 3a6eb3562..43c79a2c2 100644
--- a/pkg/aws/services/ec2.go
+++ b/pkg/aws/services/ec2.go
@@ -18,6 +18,9 @@ type EC2 interface {
 
 	// wrapper to DescribeSecurityGroupsPagesWithContext API, which aggregates paged results into list.
 	DescribeSecurityGroupsAsList(ctx context.Context, input *ec2.DescribeSecurityGroupsInput) ([]*ec2.SecurityGroup, error)
+
+	// wrapper to DescribeSubnetsPagesWithContext API, which aggregates paged results into list.
+	DescribeSubnetsAsList(ctx context.Context, input *ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error)
 }
 
 // NewEC2 constructs new EC2 implementation.
@@ -65,3 +68,14 @@ func (c *defaultEC2) DescribeSecurityGroupsAsList(ctx context.Context, input *ec
 	}
 	return result, nil
 }
+
+func (c *defaultEC2) DescribeSubnetsAsList(ctx context.Context, input *ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error) {
+	var result []*ec2.Subnet
+	if err := c.DescribeSubnetsPagesWithContext(ctx, input, func(output *ec2.DescribeSubnetsOutput, _ bool) bool {
+		result = append(result, output.Subnets...)
+		return true
+	}); err != nil {
+		return nil, err
+	}
+	return result, nil
+}
diff --git a/pkg/networking/subnet_resolver.go b/pkg/networking/subnet_resolver.go
index abd09bc8c..6c8a5ab14 100644
--- a/pkg/networking/subnet_resolver.go
+++ b/pkg/networking/subnet_resolver.go
@@ -62,16 +62,16 @@ func (r *subnetsResolver) DiscoverSubnets(ctx context.Context, scheme elbv2.Load
 			Values: aws.StringSlice([]string{r.vpcID}),
 		},
 	}}
-	resp, err := r.ec2Client.DescribeSubnetsWithContext(ctx, req)
+	subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req)
 	if err != nil {
 		return nil, err
 	}
-	chosenSubnets := []string{}
-	subnetsByAZ := map[string][]string{}
-	for _, subnet := range resp.Subnets {
+	subnetsByAZ := make(map[string][]string)
+	for _, subnet := range subnets {
 		subnetAZ := aws.StringValue(subnet.AvailabilityZone)
 		subnetsByAZ[subnetAZ] = append(subnetsByAZ[subnetAZ], aws.StringValue(subnet.SubnetId))
 	}
+	chosenSubnets := make([]string, 0, len(subnetsByAZ))
 	for az, subnets := range subnetsByAZ {
 		if len(subnets) == 1 {
 			chosenSubnets = append(chosenSubnets, subnets[0])


Test patch:
diff --git a/pkg/networking/subnet_resolver_test.go b/pkg/networking/subnet_resolver_test.go
index e2ced1715..5fc5d6905 100644
--- a/pkg/networking/subnet_resolver_test.go
+++ b/pkg/networking/subnet_resolver_test.go
@@ -17,7 +17,7 @@ import (
 func Test_subnetResolver_DiscoverSubnets(t *testing.T) {
 	type fields struct {
 		input  *ec2.DescribeSubnetsInput
-		output *ec2.DescribeSubnetsOutput
+		output []*ec2.Subnet
 		err    error
 	}
 	tests := []struct {
@@ -47,18 +47,16 @@ func Test_subnetResolver_DiscoverSubnets(t *testing.T) {
 						Values: aws.StringSlice([]string{"vpc-1"}),
 					},
 				}},
-				output: &ec2.DescribeSubnetsOutput{
-					Subnets: []*ec2.Subnet{
-						{
-							SubnetId:         aws.String("subnet-1"),
-							AvailabilityZone: aws.String("az-1"),
-							VpcId:            aws.String("vpc-1"),
-						},
-						{
-							SubnetId:         aws.String("subnet-2"),
-							AvailabilityZone: aws.String("az-2"),
-							VpcId:            aws.String("vpc-1"),
-						},
+				output: []*ec2.Subnet{
+					{
+						SubnetId:         aws.String("subnet-1"),
+						AvailabilityZone: aws.String("az-1"),
+						VpcId:            aws.String("vpc-1"),
+					},
+					{
+						SubnetId:         aws.String("subnet-2"),
+						AvailabilityZone: aws.String("az-2"),
+						VpcId:            aws.String("vpc-1"),
 					},
 				},
 			},
@@ -84,18 +82,16 @@ func Test_subnetResolver_DiscoverSubnets(t *testing.T) {
 						Values: aws.StringSlice([]string{"vpc-xx"}),
 					},
 				}},
-				output: &ec2.DescribeSubnetsOutput{
-					Subnets: []*ec2.Subnet{
-						{
-							SubnetId:         aws.String("subnet-ab1"),
-							AvailabilityZone: aws.String("az-133"),
-							VpcId:            aws.String("vpc-xx"),
-						},
-						{
-							SubnetId:         aws.String("subnet-bc1"),
-							AvailabilityZone: aws.String("az-22"),
-							VpcId:            aws.String("vpc-xx"),
-						},
+				output: []*ec2.Subnet{
+					{
+						SubnetId:         aws.String("subnet-ab1"),
+						AvailabilityZone: aws.String("az-133"),
+						VpcId:            aws.String("vpc-xx"),
+					},
+					{
+						SubnetId:         aws.String("subnet-bc1"),
+						AvailabilityZone: aws.String("az-22"),
+						VpcId:            aws.String("vpc-xx"),
 					},
 				},
 			},
@@ -121,9 +117,7 @@ func Test_subnetResolver_DiscoverSubnets(t *testing.T) {
 						Values: aws.StringSlice([]string{"vpc-xx"}),
 					},
 				}},
-				output: &ec2.DescribeSubnetsOutput{
-					Subnets: []*ec2.Subnet{},
-				},
+				output: nil,
 			},
 			vpcID:       "vpc-xx",
 			clusterName: "kube-cl",
@@ -171,28 +165,26 @@ func Test_subnetResolver_DiscoverSubnets(t *testing.T) {
 						Values: aws.StringSlice([]string{"vpc-1"}),
 					},
 				}},
-				output: &ec2.DescribeSubnetsOutput{
-					Subnets: []*ec2.Subnet{
-						{
-							SubnetId:         aws.String("fab"),
-							AvailabilityZone: aws.String("az-1"),
-							VpcId:            aws.String("vpc-1"),
-						},
-						{
-							SubnetId:         aws.String("cd"),
-							AvailabilityZone: aws.String("az-2"),
-							VpcId:            aws.String("vpc-1"),
-						},
-						{
-							SubnetId:         aws.String("ef"),
-							AvailabilityZone: aws.String("az-1"),
-							VpcId:            aws.String("vpc-1"),
-						},
-						{
-							SubnetId:         aws.String("gh"),
-							AvailabilityZone: aws.String("az-2"),
-							VpcId:            aws.String("vpc-1"),
-						},
+				output: []*ec2.Subnet{
+					{
+						SubnetId:         aws.String("fab"),
+						AvailabilityZone: aws.String("az-1"),
+						VpcId:            aws.String("vpc-1"),
+					},
+					{
+						SubnetId:         aws.String("cd"),
+						AvailabilityZone: aws.String("az-2"),
+						VpcId:            aws.String("vpc-1"),
+					},
+					{
+						SubnetId:         aws.String("ef"),
+						AvailabilityZone: aws.String("az-1"),
+						VpcId:            aws.String("vpc-1"),
+					},
+					{
+						SubnetId:         aws.String("gh"),
+						AvailabilityZone: aws.String("az-2"),
+						VpcId:            aws.String("vpc-1"),
 					},
 				},
 			},
@@ -207,7 +199,7 @@ func Test_subnetResolver_DiscoverSubnets(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 			ec2Client := mock_services.NewMockEC2(ctrl)
-			ec2Client.EXPECT().DescribeSubnetsWithContext(gomock.Any(), tt.apiParams.input).Return(tt.apiParams.output, tt.apiParams.err)
+			ec2Client.EXPECT().DescribeSubnetsAsList(gomock.Any(), tt.apiParams.input).Return(tt.apiParams.output, tt.apiParams.err)
 			resolver := NewSubnetsResolver(ec2Client, tt.vpcID, tt.clusterName, &log.NullLogger{})
 
 			got, err := resolver.DiscoverSubnets(context.Background(), tt.scheme)