Skip to content

Commit c55c548

Browse files
committed
feat: add --default-subnets option
1 parent 1824e8e commit c55c548

13 files changed

+451
-14
lines changed

controllers/ingress/group_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func NewGroupReconciler(cloud services.Cloud, k8sClient client.Client, eventReco
6363
cloud.EC2(), cloud.ELBV2(), cloud.ACM(),
6464
annotationParser, subnetsResolver,
6565
authConfigBuilder, enhancedBackendBuilder, trackingProvider, elbv2TaggingManager, controllerConfig.FeatureGates,
66-
cloud.VpcID(), controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
66+
cloud.VpcID(), controllerConfig.ClusterName, controllerConfig.DefaultSubnets, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
6767
controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, backendSGProvider, sgResolver,
6868
controllerConfig.EnableBackendSecurityGroup, controllerConfig.EnableManageBackendSecurityGroupRules, controllerConfig.DisableRestrictedSGRules, controllerConfig.IngressConfig.AllowedCertificateAuthorityARNs, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), logger, metricsCollector)
6969
stackMarshaller := deploy.NewDefaultStackMarshaller()

controllers/service/service_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func NewServiceReconciler(cloud services.Cloud, k8sClient client.Client, eventRe
4949
trackingProvider := tracking.NewDefaultProvider(serviceTagPrefix, controllerConfig.ClusterName)
5050
serviceUtils := service.NewServiceUtils(annotationParser, shared_constants.ServiceFinalizer, controllerConfig.ServiceConfig.LoadBalancerClass, controllerConfig.FeatureGates)
5151
modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcInfoProvider, cloud.VpcID(), trackingProvider,
52-
elbv2TaggingManager, cloud.EC2(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
52+
elbv2TaggingManager, cloud.EC2(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultSubnets, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
5353
controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), serviceUtils,
5454
backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.EnableManageBackendSecurityGroupRules, controllerConfig.DisableRestrictedSGRules, logger, metricsCollector)
5555
stackMarshaller := deploy.NewDefaultStackMarshaller()

docs/deploy/configurations.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
7777
| backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG |
7878
| cluster-name | string | | Kubernetes cluster name |
7979
| default-ssl-policy | string | ELBSecurityPolicy-2016-08 | Default SSL Policy that will be applied to all Ingresses or Services that do not have the SSL Policy annotation |
80+
| default-subnets | stringList | [] | Default subnets to be selected when not explicitly specified through annotations or other methods |
8081
| default-tags | stringMap | | AWS Tags that will be applied to all AWS resources managed by this controller. Specified Tags takes highest priority |
8182
| default-target-type | string | instance | Default target type for Ingresses and Services - ip, instance |
8283
| default-load-balancer-scheme | string | internal | Default scheme for ELBs - internal, internet-facing |

docs/deploy/subnet_discovery.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ A subnet is classified as public if its route table contains a route to an Inter
6161
The controller selects one subnet per availability zone. When multiple subnets exist per Availability Zone, the following priority order applies:
6262

6363
1. Subnets with cluster tag for the current cluster (`kubernetes.io/cluster/<clusterName>`) are prioritized
64-
2. Subnets with lower lexicographical order of subnet ID are prioritized
64+
2. Subnets with the `--default-subnets` flag (prioritized in the order specified)
65+
3. Subnets with lower lexicographical order of subnet ID are prioritized
6566

6667
## Minimum Subnet Requirements
6768

pkg/config/controller_config.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
const (
1717
flagLogLevel = "log-level"
1818
flagK8sClusterName = "cluster-name"
19+
flagDefaultSubnets = "default-subnets"
1920
flagDefaultTags = "default-tags"
2021
flagDefaultTargetType = "default-target-type"
2122
flagDefaultLoadBalancerScheme = "default-load-balancer-scheme"
@@ -78,6 +79,9 @@ type ControllerConfig struct {
7879
// Configurations for the Service controller
7980
ServiceConfig ServiceConfig
8081

82+
// Default subnets that will be used for all AWS resources managed by the networking controller.
83+
DefaultSubnets []string
84+
8185
// Default AWS Tags that will be applied to all AWS resources managed by this controller.
8286
DefaultTags map[string]string
8387

@@ -137,6 +141,8 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
137141
fs.StringVar(&cfg.LogLevel, flagLogLevel, defaultLogLevel,
138142
"Set the controller log level - info(default), debug")
139143
fs.StringVar(&cfg.ClusterName, flagK8sClusterName, "", "Kubernetes cluster name")
144+
fs.StringSliceVar(&cfg.DefaultSubnets, flagDefaultSubnets, nil,
145+
"Default subnets that will be used for all AWS resources managed by the networking controller")
140146
fs.StringToStringVar(&cfg.DefaultTags, flagDefaultTags, nil,
141147
"Default AWS Tags that will be applied to all AWS resources managed by this controller")
142148
fs.StringVar(&cfg.DefaultTargetType, flagDefaultTargetType, string(elbv2.TargetTypeInstance),
@@ -186,7 +192,9 @@ func (cfg *ControllerConfig) Validate() error {
186192
if len(cfg.ClusterName) == 0 {
187193
return errors.New("kubernetes cluster name must be specified")
188194
}
189-
195+
if err := cfg.validateDefaultSubnets(); err != nil {
196+
return err
197+
}
190198
if err := cfg.validateDefaultTagsCollisionWithTrackingTags(); err != nil {
191199
return err
192200
}
@@ -211,6 +219,27 @@ func (cfg *ControllerConfig) Validate() error {
211219
return nil
212220
}
213221

222+
func (cfg *ControllerConfig) validateDefaultSubnets() error {
223+
if len(cfg.DefaultSubnets) == 0 {
224+
return nil
225+
}
226+
for _, subnetID := range cfg.DefaultSubnets {
227+
if !strings.HasPrefix(subnetID, "subnet-") {
228+
return errors.Errorf("invalid value %v for default subnet id", subnetID)
229+
}
230+
}
231+
232+
//validate duplicate subnet ids
233+
seen := make(map[string]bool)
234+
for _, str := range cfg.DefaultSubnets {
235+
if seen[str] {
236+
return errors.Errorf("duplicate subnet id %v is specified in the --default-subnets flag", str)
237+
}
238+
seen[str] = true
239+
}
240+
return nil
241+
}
242+
214243
func (cfg *ControllerConfig) validateDefaultTagsCollisionWithTrackingTags() error {
215244
for tagKey := range cfg.DefaultTags {
216245
if trackingTagKeys.Has(tagKey) {

pkg/config/controller_config_test.go

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
package config
22

33
import (
4+
"testing"
5+
46
"github.com/pkg/errors"
57
"github.com/stretchr/testify/assert"
68
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
7-
"testing"
89
)
910

1011
func TestControllerConfig_validateDefaultTagsCollisionWithTrackingTags(t *testing.T) {
@@ -219,3 +220,49 @@ func TestControllerConfig_validateManageBackendSecurityGroupRulesConfiguration(t
219220
})
220221
}
221222
}
223+
224+
func TestControllerConfig_validateDefaultSubnets(t *testing.T) {
225+
type fields struct {
226+
DefaultSubnets []string
227+
}
228+
tests := []struct {
229+
name string
230+
fields fields
231+
wantErr error
232+
}{
233+
{
234+
name: "default subnets is empty",
235+
fields: fields{
236+
DefaultSubnets: nil,
237+
},
238+
wantErr: nil,
239+
},
240+
{
241+
name: "default subnets is not empty",
242+
fields: fields{
243+
DefaultSubnets: []string{"subnet-1", "subnet-2"},
244+
},
245+
wantErr: nil,
246+
},
247+
{
248+
name: "default subnets is not empty and duplicate subnets are specified",
249+
fields: fields{
250+
DefaultSubnets: []string{"subnet-1", "subnet-2", "subnet-1"},
251+
},
252+
wantErr: errors.New("duplicate subnet id subnet-1 is specified in the --default-subnets flag"),
253+
},
254+
}
255+
for _, tt := range tests {
256+
t.Run(tt.name, func(t *testing.T) {
257+
cfg := &ControllerConfig{
258+
DefaultSubnets: tt.fields.DefaultSubnets,
259+
}
260+
err := cfg.validateDefaultSubnets()
261+
if tt.wantErr != nil {
262+
assert.EqualError(t, err, tt.wantErr.Error())
263+
} else {
264+
assert.NoError(t, err)
265+
}
266+
})
267+
}
268+
}

pkg/ingress/model_build_load_balancer.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont
260260
chosenSubnets, err := t.subnetsResolver.ResolveViaDiscovery(ctx,
261261
networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication),
262262
networking.WithSubnetsResolveLBScheme(scheme),
263+
networking.WithDefaultSubnets(t.defaultSubnets),
263264
)
264265
if err != nil {
265266
return nil, errors.Wrap(err, "couldn't auto-discover subnets")

pkg/ingress/model_builder.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ package ingress
33
import (
44
"context"
55
"reflect"
6-
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
76
"strconv"
87

8+
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
9+
910
elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
1011

1112
awssdk "github.com/aws/aws-sdk-go-v2/aws"
@@ -46,7 +47,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
4647
annotationParser annotations.Parser, subnetsResolver networkingpkg.SubnetsResolver,
4748
authConfigBuilder AuthConfigBuilder, enhancedBackendBuilder EnhancedBackendBuilder,
4849
trackingProvider tracking.Provider, elbv2TaggingManager elbv2deploy.TaggingManager, featureGates config.FeatureGates,
49-
vpcID string, clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string, defaultLoadBalancerScheme string,
50+
vpcID string, clusterName string, defaultSubnets []string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string, defaultLoadBalancerScheme string,
5051
backendSGProvider networkingpkg.BackendSGProvider, sgResolver networkingpkg.SecurityGroupResolver,
5152
enableBackendSG bool, defaultEnableManageBackendSGRules bool, disableRestrictedSGRules bool, allowedCAARNs []string, enableIPTargetType bool, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector) *defaultModelBuilder {
5253
certDiscovery := NewACMCertDiscovery(acmClient, allowedCAARNs, logger)
@@ -69,6 +70,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
6970
trackingProvider: trackingProvider,
7071
elbv2TaggingManager: elbv2TaggingManager,
7172
featureGates: featureGates,
73+
defaultSubnets: defaultSubnets,
7274
defaultTags: defaultTags,
7375
externalManagedTags: sets.NewString(externalManagedTags...),
7476
defaultSSLPolicy: defaultSSLPolicy,
@@ -106,6 +108,7 @@ type defaultModelBuilder struct {
106108
trackingProvider tracking.Provider
107109
elbv2TaggingManager elbv2deploy.TaggingManager
108110
featureGates config.FeatureGates
111+
defaultSubnets []string
109112
defaultTags map[string]string
110113
externalManagedTags sets.String
111114
defaultSSLPolicy string
@@ -154,6 +157,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group, metrics
154157
stack: stack,
155158
frontendNlbTargetGroupDesiredState: frontendNlbTargetGroupDesiredState,
156159

160+
defaultSubnets: b.defaultSubnets,
157161
defaultTags: b.defaultTags,
158162
externalManagedTags: b.externalManagedTags,
159163
defaultIPAddressType: elbv2model.IPAddressTypeIPV4,
@@ -213,6 +217,7 @@ type defaultModelBuildTask struct {
213217
disableRestrictedSGRules bool
214218
enableIPTargetType bool
215219

220+
defaultSubnets []string
216221
defaultTags map[string]string
217222
externalManagedTags sets.String
218223
defaultIPAddressType elbv2model.IPAddressType

pkg/networking/subnet_resolver.go

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ type SubnetsResolveOptions struct {
5353
// The Load Balancer Scheme.
5454
// By default, it's internet-facing.
5555
LBScheme elbv2model.LoadBalancerScheme
56+
// Subnets specified with --default-subnets
57+
DefaultSubnets []string
5658
}
5759

5860
// ApplyOptions applies slice of SubnetsResolveOption.
@@ -86,6 +88,13 @@ func WithSubnetsResolveLBScheme(lbScheme elbv2model.LoadBalancerScheme) SubnetsR
8688
}
8789
}
8890

91+
// WithDefaultSubnets generates an option that configures DefaultSubnets.
92+
func WithDefaultSubnets(defaultSubnets []string) SubnetsResolveOption {
93+
return func(opts *SubnetsResolveOptions) {
94+
opts.DefaultSubnets = defaultSubnets
95+
}
96+
}
97+
8998
// SubnetsResolver is responsible for resolve EC2 Subnets for Load Balancers.
9099
type SubnetsResolver interface {
91100
// ResolveViaDiscovery resolve subnets by auto discover matching subnets.
@@ -412,7 +421,7 @@ func (r *defaultSubnetsResolver) validateSpecifiedSubnets(ctx context.Context, s
412421
// chooseAndValidateSubnetsPerAZ will choose one subnet per AZ from eligible subnets and then validate against chosen subnets.
413422
func (r *defaultSubnetsResolver) chooseAndValidateSubnetsPerAZ(ctx context.Context, subnets []ec2types.Subnet, resolveOpts SubnetsResolveOptions) ([]ec2types.Subnet, error) {
414423
categorizedSubnets := r.categorizeSubnetsByEligibility(subnets)
415-
chosenSubnets := r.chooseSubnetsPerAZ(categorizedSubnets.eligible)
424+
chosenSubnets := r.chooseSubnetsPerAZ(categorizedSubnets.eligible, resolveOpts.DefaultSubnets)
416425
if len(chosenSubnets) == 0 {
417426
return nil, fmt.Errorf("unable to resolve at least one subnet. Evaluated %d subnets: %d are tagged for other clusters, and %d have insufficient available IP addresses",
418427
len(subnets), len(categorizedSubnets.ineligibleClusterTag), len(categorizedSubnets.insufficientIPs))
@@ -452,7 +461,15 @@ func (r *defaultSubnetsResolver) categorizeSubnetsByEligibility(subnets []ec2typ
452461

453462
// chooseSubnetsPerAZ will choose one subnet per AZ.
454463
// * subnets with current cluster tag will be prioritized.
455-
func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) []ec2types.Subnet {
464+
func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet, defaultSubnets []string) []ec2types.Subnet {
465+
466+
prioritySubnetMap := make(map[string]int)
467+
468+
if len(defaultSubnets) > 0 {
469+
for i, subnetID := range defaultSubnets {
470+
prioritySubnetMap[subnetID] = i
471+
}
472+
}
456473
subnetsByAZ := mapSDKSubnetsByAZ(subnets)
457474
chosenSubnets := make([]ec2types.Subnet, 0, len(subnetsByAZ))
458475
for az, azSubnets := range subnetsByAZ {
@@ -467,9 +484,24 @@ func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) [
467484
} else if (!subnetIHasCurrentClusterTag) && subnetJHasCurrentClusterTag {
468485
return false
469486
}
487+
488+
// When azSubnets are specified in --default-azSubnets, the azSubnets list will be sorted according to this order.
489+
// Any azSubnets not specified in --default-azSubnets will be sorted in lexicographical order and placed after the prioritized azSubnets.
490+
iVal, iExists := prioritySubnetMap[awssdk.ToString(azSubnets[i].SubnetId)]
491+
jVal, jExists := prioritySubnetMap[awssdk.ToString(azSubnets[j].SubnetId)]
492+
493+
if iExists && jExists {
494+
return iVal < jVal
495+
}
496+
if iExists {
497+
return true
498+
}
499+
if jExists {
500+
return false
501+
}
470502
return awssdk.ToString(azSubnets[i].SubnetId) < awssdk.ToString(azSubnets[j].SubnetId)
471503
})
472-
r.logger.V(1).Info("multiple subnets in the same AvailabilityZone", "AvailabilityZone", az,
504+
r.logger.V(1).Info("multiple azSubnets in the same AvailabilityZone", "AvailabilityZone", az,
473505
"chosen", azSubnets[0].SubnetId, "ignored", extractSubnetIDs(azSubnets[1:]))
474506
chosenSubnets = append(chosenSubnets, azSubnets[0])
475507
}

0 commit comments

Comments
 (0)