Skip to content

Commit a1d4946

Browse files
oliviassssTimothy-Dougherty
authored andcommitted
Discovery subnets by available ip addresses (kubernetes-sigs#2146)
* discovery subnet by the count of available ip address * update unit test for subnet_resolver * filter subnets before iterating subnetsByAZ * decouple buildLoadBalancerSubnets from buildLoadBalancerSubnetMappings
1 parent 87d43e2 commit a1d4946

File tree

4 files changed

+143
-1
lines changed

4 files changed

+143
-1
lines changed

pkg/ingress/model_build_load_balancer.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
const (
2727
resourceIDLoadBalancer = "LoadBalancer"
28+
minimalAvailableIPAddressCount = int64(8)
2829
)
2930

3031
func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, listenPortConfigByPort map[int64]listenPortConfig) (*elbv2model.LoadBalancer, error) {
@@ -218,6 +219,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont
218219
chosenSubnets, err := t.subnetsResolver.ResolveViaDiscovery(ctx,
219220
networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication),
220221
networking.WithSubnetsResolveLBScheme(scheme),
222+
networking.WithSubnetsResolveAvailableIPAddressCount(minimalAvailableIPAddressCount),
221223
)
222224
if err != nil {
223225
return nil, errors.Wrap(err, "couldn't auto-discover subnets")

pkg/networking/subnet_resolver.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ type SubnetsResolveOptions struct {
4242
// The Load Balancer Scheme.
4343
// By default, it's internet-facing.
4444
LBScheme elbv2model.LoadBalancerScheme
45+
// count of available ip addresses
46+
AvailableIPAddressCount int64
4547
}
4648

4749
// ApplyOptions applies slice of SubnetsResolveOption.
@@ -75,6 +77,13 @@ func WithSubnetsResolveLBScheme(lbScheme elbv2model.LoadBalancerScheme) SubnetsR
7577
}
7678
}
7779

80+
// WithSubnetsResolveAvailableIPAddressCount generates a option that configures AvailableIPAddressCount.
81+
func WithSubnetsResolveAvailableIPAddressCount(AvailableIPAddressCount int64) SubnetsResolveOption {
82+
return func(opts *SubnetsResolveOptions) {
83+
opts.AvailableIPAddressCount = AvailableIPAddressCount
84+
}
85+
}
86+
7887
// SubnetsResolver is responsible for resolve EC2 Subnets for Load Balancers.
7988
type SubnetsResolver interface {
8089
// ResolveViaDiscovery resolve subnets by auto discover matching subnets.
@@ -143,7 +152,8 @@ func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts .
143152
subnets = append(subnets, subnet)
144153
}
145154
}
146-
subnetsByAZ := mapSDKSubnetsByAZ(subnets)
155+
filteredSubnets := r.filterSubnetsByAvailableIPAddress(subnets, resolveOpts.AvailableIPAddressCount)
156+
subnetsByAZ := mapSDKSubnetsByAZ(filteredSubnets)
147157
chosenSubnets := make([]*ec2sdk.Subnet, 0, len(subnetsByAZ))
148158
for az, subnets := range subnetsByAZ {
149159
if len(subnets) == 1 {
@@ -370,3 +380,14 @@ func sortSubnetsByID(subnets []*ec2sdk.Subnet) {
370380
return awssdk.StringValue(subnets[i].SubnetId) < awssdk.StringValue(subnets[j].SubnetId)
371381
})
372382
}
383+
384+
func (r *defaultSubnetsResolver) filterSubnetsByAvailableIPAddress(subnets []*ec2sdk.Subnet, availableIPAddressCount int64) []*ec2sdk.Subnet {
385+
filteredSubnets := make([]*ec2sdk.Subnet, 0, len(subnets))
386+
for _, subnet := range subnets {
387+
if awssdk.Int64Value(subnet.AvailableIpAddressCount) >= availableIPAddressCount {
388+
filteredSubnets = append(filteredSubnets, subnet)
389+
}
390+
}
391+
r.logger.V(4).Info("Subnets filtered by Available IP Address: ", filteredSubnets)
392+
return filteredSubnets
393+
}

pkg/networking/subnet_resolver_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
3535
type args struct {
3636
opts []SubnetsResolveOption
3737
}
38+
const (
39+
minimalAvailableIPAddressCount = int64(8)
40+
)
3841
tests := []struct {
3942
name string
4043
fields fields
@@ -886,6 +889,109 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
886889
},
887890
},
888891
},
892+
{
893+
name: "subnets with insufficient available ip addresses get ignored",
894+
fields: fields{
895+
vpcID: "vpc-1",
896+
clusterName: "kube-cluster",
897+
describeSubnetsAsListCalls: []describeSubnetsAsListCall{
898+
{
899+
input: &ec2sdk.DescribeSubnetsInput{
900+
Filters: []*ec2sdk.Filter{
901+
{
902+
Name: awssdk.String("tag:kubernetes.io/role/elb"),
903+
Values: awssdk.StringSlice([]string{"", "1"}),
904+
},
905+
{
906+
Name: awssdk.String("vpc-id"),
907+
Values: awssdk.StringSlice([]string{"vpc-1"}),
908+
},
909+
},
910+
},
911+
output: []*ec2sdk.Subnet{
912+
{
913+
SubnetId: awssdk.String("subnet-1"),
914+
AvailabilityZone: awssdk.String("us-west-2a"),
915+
AvailabilityZoneId: awssdk.String("usw2-az1"),
916+
VpcId: awssdk.String("vpc-1"),
917+
AvailableIpAddressCount: awssdk.Int64(0),
918+
},
919+
{
920+
SubnetId: awssdk.String("subnet-3"),
921+
AvailabilityZone: awssdk.String("us-west-2a"),
922+
AvailabilityZoneId: awssdk.String("usw2-az1"),
923+
VpcId: awssdk.String("vpc-1"),
924+
AvailableIpAddressCount: awssdk.Int64(8),
925+
},
926+
{
927+
SubnetId: awssdk.String("subnet-4"),
928+
AvailabilityZone: awssdk.String("us-west-2b"),
929+
AvailabilityZoneId: awssdk.String("usw2-az2"),
930+
VpcId: awssdk.String("vpc-1"),
931+
AvailableIpAddressCount: awssdk.Int64(25),
932+
},
933+
{
934+
SubnetId: awssdk.String("subnet-2"),
935+
AvailabilityZone: awssdk.String("us-west-2a"),
936+
AvailabilityZoneId: awssdk.String("usw2-az1"),
937+
VpcId: awssdk.String("vpc-1"),
938+
AvailableIpAddressCount: awssdk.Int64(2),
939+
},
940+
{
941+
SubnetId: awssdk.String("subnet-5"),
942+
AvailabilityZone: awssdk.String("us-west-2b"),
943+
AvailabilityZoneId: awssdk.String("usw2-az2"),
944+
VpcId: awssdk.String("vpc-1"),
945+
AvailableIpAddressCount: awssdk.Int64(10),
946+
},
947+
},
948+
},
949+
},
950+
fetchAZInfosCalls: []fetchAZInfosCall{
951+
{
952+
availabilityZoneIDs: []string{"usw2-az1"},
953+
azInfoByAZID: map[string]ec2sdk.AvailabilityZone{
954+
"usw2-az1": {
955+
ZoneId: awssdk.String("usw2-az1"),
956+
ZoneType: awssdk.String("availability-zone"),
957+
},
958+
},
959+
},
960+
{
961+
availabilityZoneIDs: []string{"usw2-az2"},
962+
azInfoByAZID: map[string]ec2sdk.AvailabilityZone{
963+
"usw2-az2": {
964+
ZoneId: awssdk.String("usw2-az2"),
965+
ZoneType: awssdk.String("availability-zone"),
966+
},
967+
},
968+
},
969+
},
970+
},
971+
args: args{
972+
opts: []SubnetsResolveOption{
973+
WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
974+
WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing),
975+
WithSubnetsResolveAvailableIPAddressCount(minimalAvailableIPAddressCount),
976+
},
977+
},
978+
want: []*ec2sdk.Subnet{
979+
{
980+
SubnetId: awssdk.String("subnet-3"),
981+
AvailabilityZone: awssdk.String("us-west-2a"),
982+
AvailabilityZoneId: awssdk.String("usw2-az1"),
983+
VpcId: awssdk.String("vpc-1"),
984+
AvailableIpAddressCount: awssdk.Int64(8),
985+
},
986+
{
987+
SubnetId: awssdk.String("subnet-4"),
988+
AvailabilityZone: awssdk.String("us-west-2b"),
989+
AvailabilityZoneId: awssdk.String("usw2-az2"),
990+
VpcId: awssdk.String("vpc-1"),
991+
AvailableIpAddressCount: awssdk.Int64(25),
992+
},
993+
},
994+
},
889995
}
890996

891997
for _, tt := range tests {

pkg/service/model_build_load_balancer.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const (
2727
lbAttrsAccessLogsS3Prefix = "access_logs.s3.prefix"
2828
lbAttrsLoadBalancingCrossZoneEnabled = "load_balancing.cross_zone.enabled"
2929
resourceIDLoadBalancer = "LoadBalancer"
30+
minimalAvailableIPAddressCount = int64(8)
3031
)
3132

3233
func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error {
@@ -267,6 +268,18 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnets(ctx context.Context, sc
267268
)
268269
}
269270

271+
// for internet-facing Load Balancers, the subnets mush have at least 8 available IP addresses;
272+
// for internal Load Balancers, this is only required if private ip address is not assigned
273+
var privateIpv4Addresses []string
274+
ipv4Configured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixPrivateIpv4Addresses, &privateIpv4Addresses, t.service.Annotations)
275+
if (scheme == elbv2model.LoadBalancerSchemeInternetFacing) ||
276+
((scheme == elbv2model.LoadBalancerSchemeInternal) && !ipv4Configured) {
277+
return t.subnetsResolver.ResolveViaDiscovery(ctx,
278+
networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
279+
networking.WithSubnetsResolveLBScheme(scheme),
280+
networking.WithSubnetsResolveAvailableIPAddressCount(minimalAvailableIPAddressCount),
281+
)
282+
}
270283
return t.subnetsResolver.ResolveViaDiscovery(ctx,
271284
networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
272285
networking.WithSubnetsResolveLBScheme(scheme),

0 commit comments

Comments
 (0)