Ability to specify subnets for NLB (#1667)

* ability to specify subnets for NLB * update resolver function * fix doc typo
kubernetes-sigs · Nov 18, 2020 · 72cef4c · 72cef4c
1 parent 1ae0d1b
commit 72cef4c
Show file tree

Hide file tree

Showing 7 changed files with 121 additions and 11 deletions.
diff --git a/docs/guide/service/annotations.md b/docs/guide/service/annotations.md
@@ -13,7 +13,7 @@
 |--------------------------------------------------------------------------------|------------|---------------------------|------------------------|
 | service.beta.kubernetes.io/aws-load-balancer-type                              | string     |                           |                        |
 | service.beta.kubernetes.io/aws-load-balancer-internal                          | boolean    | false                     |                        |
-| [service.beta.kubernetes.io/aws-load-balancer-proxy-protocol](#proxy-protocol-v2)                 | string     |                           | Set to `"*"` to enable |
+| [service.beta.kubernetes.io/aws-load-balancer-proxy-protocol](#proxy-protocol-v2)                 | string     |        | Set to `"*"` to enable |
 | service.beta.kubernetes.io/aws-load-balancer-access-log-enabled                | boolean    | false                     |                        |
 | service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-name         | string     |                           |                        |
 | service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-prefix       | string     |                           |                        |
@@ -31,7 +31,30 @@
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-port                  | string     | traffic-port              |                        |
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-path                  | string     | "/" for HTTP(S) protocols |                        |
 | service.beta.kubernetes.io/aws-load-balancer-eip-allocations                   | stringList |                           |                        |
-| [service.beta.kubernetes.io/aws-load-balancer-target-group-attributes](#target-group-attributes)  | stringMap  |                           |                        |
+| [service.beta.kubernetes.io/aws-load-balancer-target-group-attributes](#target-group-attributes)  | stringMap  |        |                        |
+| [service.beta.kubernetes.io/aws-load-balancer-subnets](#subnets)              | stringList  |                           |                        |
+
+
+## Traffic Routing
+Traffic Routing can be controlled with following annotations:
+
+- <a name="subnets">`service.beta.kubernetes.io/aws-load-balancer-subnets`</a> specifies the [Availability Zone](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html)
+the NLB will route traffic to. See [Network Load Balancers](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/network-load-balancers.html#availability-zones) for more details.
+
+    !!!tip
+        Subnets are auto-discovered if this annotation is not specified, see [Subnet Discovery](../controller/subnet_discovery.md) for further details.
+
+    !!!note ""
+        You must specify at least one subnet in any of the AZs, both subnetID or subnetName(Name tag on subnets) can be used.
+
+    !!!warning "limitations"
+        - Each subnets must be from a different Availability Zone
+        - AWS has restrictions on disabling existing subnets for NLB. As a result, you might not be able to edit this annotation once the NLB gets provisioned.
+
+    !!!example
+        ```
+        service.beta.kubernetes.io/aws-load-balancer-subnets: subnet-xxxx, mySubnet
+        ```
 
 ## Resource attributes
 NLB target group attributes can be controlled via the following annotations:

diff --git a/pkg/annotations/constants.go b/pkg/annotations/constants.go
@@ -65,4 +65,5 @@ const (
 	SvcLBSuffixHCPath                        = "aws-load-balancer-healthcheck-path"
 	SvcLBSuffixEIPAllocations                = "aws-load-balancer-eip-allocations"
 	SvcLBSuffixTargetGroupAttributes         = "aws-load-balancer-target-group-attributes"
+	SvcLBSuffixSubnets                       = "aws-load-balancer-subnets"
 )
diff --git a/pkg/networking/subnet_resolver.go b/pkg/networking/subnet_resolver.go
@@ -204,7 +204,7 @@ func (r *defaultSubnetsResolver) ResolveViaNameOrIDSlice(ctx context.Context, su
 		resolvedSubnets = append(resolvedSubnets, subnets...)
 	}
 	if len(resolvedSubnets) != len(subnetNameOrIDs) {
-		return nil, errors.Errorf("couldn't found all subnets, nameOrIDs: %v, found: %v", subnetNameOrIDs, len(resolvedSubnets))
+		return nil, errors.Errorf("couldn't find all subnets, nameOrIDs: %v, found: %v", subnetNameOrIDs, len(resolvedSubnets))
 	}
 	if len(resolvedSubnets) == 0 {
 		return nil, errors.New("unable to resolve at least one subnet")

diff --git a/pkg/networking/subnet_resolver_test.go b/pkg/networking/subnet_resolver_test.go
@@ -678,7 +678,7 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) {
 					WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal),
 				},
 			},
-			wantErr: errors.New("couldn't found all subnets, nameOrIDs: [my-name-1 my-name-2 my-name-3], found: 2"),
+			wantErr: errors.New("couldn't find all subnets, nameOrIDs: [my-name-1 my-name-2 my-name-3], found: 2"),
 		},
 		{
 			name: "empty subnet name or IDs",

diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go
@@ -11,6 +11,7 @@ import (
 	"regexp"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
 	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
 	"strconv"
 )
 
@@ -42,7 +43,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSpec(ctx context.Context, schem
 	if err != nil {
 		return elbv2model.LoadBalancerSpec{}, err
 	}
-	subnetMappings, err := t.buildSubnetMappings(ctx, t.ec2Subnets)
+	subnetMappings, err := t.buildLoadBalancerSubnetMappings(ctx, t.ec2Subnets)
 	if err != nil {
 		return elbv2model.LoadBalancerSpec{}, err
 	}
@@ -83,7 +84,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerTags(ctx context.Context) (map[
 	return t.buildAdditionalResourceTags(ctx)
 }
 
-func (t *defaultModelBuildTask) buildSubnetMappings(_ context.Context, ec2Subnets []*ec2.Subnet) ([]elbv2model.SubnetMapping, error) {
+func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(_ context.Context, ec2Subnets []*ec2.Subnet) ([]elbv2model.SubnetMapping, error) {
 	var eipAllocation []string
 	eipConfigured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixEIPAllocations, &eipAllocation, t.service.Annotations)
 	if eipConfigured && len(eipAllocation) != len(ec2Subnets) {
@@ -102,6 +103,20 @@ func (t *defaultModelBuildTask) buildSubnetMappings(_ context.Context, ec2Subnet
 	return subnetMappings, nil
 }
 
+func (t *defaultModelBuildTask) resolveLoadBalancerSubnets(ctx context.Context, scheme elbv2model.LoadBalancerScheme) ([]*ec2.Subnet, error) {
+	var rawSubnetNameOrIDs []string
+	if exists := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSubnets, &rawSubnetNameOrIDs, t.service.Annotations); exists {
+		return t.subnetsResolver.ResolveViaNameOrIDSlice(ctx, rawSubnetNameOrIDs,
+			networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
+			networking.WithSubnetsResolveLBScheme(scheme),
+		)
+	}
+	return t.subnetsResolver.ResolveViaDiscovery(ctx,
+		networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
+		networking.WithSubnetsResolveLBScheme(scheme),
+	)
+}
+
 func (t *defaultModelBuildTask) buildLoadBalancerAttributes(_ context.Context) ([]elbv2model.LoadBalancerAttribute, error) {
 	var attrs []elbv2model.LoadBalancerAttribute
 	accessLogEnabled := t.defaultAccessLogS3Enabled

diff --git a/pkg/service/model_build_load_balancer_test.go b/pkg/service/model_build_load_balancer_test.go
@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	mock_networking "sigs.k8s.io/aws-load-balancer-controller/mocks/networking"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 	"testing"
@@ -223,7 +224,7 @@ func Test_defaultModelBuilderTask_buildSubnetMappings(t *testing.T) {
 
 			annotationParser := annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io")
 			builder := &defaultModelBuildTask{service: tt.svc, annotationParser: annotationParser}
-			got, err := builder.buildSubnetMappings(context.Background(), tt.subnets)
+			got, err := builder.buildLoadBalancerSubnetMappings(context.Background(), tt.subnets)
 			if tt.wantErr != nil {
 				assert.EqualError(t, err, tt.wantErr.Error())
 			} else {
@@ -232,3 +233,76 @@ func Test_defaultModelBuilderTask_buildSubnetMappings(t *testing.T) {
 		})
 	}
 }
+
+func Test_defaultModelBuilderTask_resolveLoadBalancerSubnets(t *testing.T) {
+	type resolveSubnetResults struct {
+		subnets []*ec2.Subnet
+		err     error
+	}
+	tests := []struct {
+		name                     string
+		svc                      *corev1.Service
+		scheme                   elbv2.LoadBalancerScheme
+		resolveViaDiscovery      []resolveSubnetResults
+		resolveViaNameOrIDSlilce []resolveSubnetResults
+	}{
+		{
+			name:   "subnet auto-discovery",
+			svc:    &corev1.Service{},
+			scheme: elbv2.LoadBalancerSchemeInternal,
+			resolveViaDiscovery: []resolveSubnetResults{
+				{
+					subnets: []*ec2.Subnet{
+						{
+							SubnetId:  aws.String("subnet-1"),
+							CidrBlock: aws.String("192.168.0.0/19"),
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "subnet annotation",
+			svc: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"service.beta.kubernetes.io/aws-load-balancer-subnets": "subnet-abc, Subnet Name XYZ",
+					},
+				},
+			},
+			scheme: elbv2.LoadBalancerSchemeInternal,
+			resolveViaNameOrIDSlilce: []resolveSubnetResults{
+				{
+					subnets: []*ec2.Subnet{
+						{
+							SubnetId:  aws.String("subnet-abc"),
+							CidrBlock: aws.String("192.168.0.0/19"),
+						},
+						{
+							SubnetId:  aws.String("subnet-xyz"),
+							CidrBlock: aws.String("192.168.0.0/19"),
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			subnetsResolver := mock_networking.NewMockSubnetsResolver(ctrl)
+			for _, call := range tt.resolveViaDiscovery {
+				subnetsResolver.EXPECT().ResolveViaDiscovery(gomock.Any(), gomock.Any()).Return(call.subnets, call.err)
+			}
+			for _, call := range tt.resolveViaNameOrIDSlilce {
+				subnetsResolver.EXPECT().ResolveViaNameOrIDSlice(gomock.Any(), gomock.Any(), gomock.Any()).Return(call.subnets, call.err)
+			}
+			annotationParser := annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io")
+			builder := &defaultModelBuildTask{service: tt.svc, annotationParser: annotationParser, subnetsResolver: subnetsResolver}
+
+			builder.resolveLoadBalancerSubnets(context.Background(), tt.scheme)
+		})
+	}
+}
diff --git a/pkg/service/model_builder.go b/pkg/service/model_builder.go
@@ -103,10 +103,7 @@ func (t *defaultModelBuildTask) buildModel(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	t.ec2Subnets, err = t.subnetsResolver.ResolveViaDiscovery(ctx,
-		networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
-		networking.WithSubnetsResolveLBScheme(scheme),
-	)
+	t.ec2Subnets, err = t.resolveLoadBalancerSubnets(ctx, scheme)
 	if err != nil {
 		return err
 	}