Skip to content
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.

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

Ability to specify subnets for NLB #1667

Merged
merged 3 commits into from
Nov 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 25 additions & 2 deletions docs/guide/service/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | | |
Expand All @@ -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:
Expand Down
1 change: 1 addition & 0 deletions pkg/annotations/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
2 changes: 1 addition & 1 deletion pkg/networking/subnet_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion pkg/networking/subnet_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 17 additions & 2 deletions pkg/service/model_build_load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down
76 changes: 75 additions & 1 deletion pkg/service/model_build_load_balancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
})
}
}
5 changes: 1 addition & 4 deletions pkg/service/model_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down