Skip to content

Commit 8d6e30a

Browse files
committed
feat: add --default-subnets option
1 parent 7191fb7 commit 8d6e30a

13 files changed

+449
-13
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, controllerConfig.FeatureGates.Enabled(config.EnableTCPUDPListenerType))
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"
@@ -79,6 +80,9 @@ type ControllerConfig struct {
7980
// Configurations for the Service controller
8081
ServiceConfig ServiceConfig
8182

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

@@ -141,6 +145,8 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
141145
fs.StringVar(&cfg.LogLevel, flagLogLevel, defaultLogLevel,
142146
"Set the controller log level - info(default), debug")
143147
fs.StringVar(&cfg.ClusterName, flagK8sClusterName, "", "Kubernetes cluster name")
148+
fs.StringSliceVar(&cfg.DefaultSubnets, flagDefaultSubnets, nil,
149+
"Default subnets that will be used for all AWS resources managed by the networking controller")
144150
fs.StringToStringVar(&cfg.DefaultTags, flagDefaultTags, nil,
145151
"Default AWS Tags that will be applied to all AWS resources managed by this controller")
146152
fs.StringVar(&cfg.DefaultTargetType, flagDefaultTargetType, string(elbv2.TargetTypeInstance),
@@ -192,7 +198,9 @@ func (cfg *ControllerConfig) Validate() error {
192198
if len(cfg.ClusterName) == 0 {
193199
return errors.New("kubernetes cluster name must be specified")
194200
}
195-
201+
if err := cfg.validateDefaultSubnets(); err != nil {
202+
return err
203+
}
196204
if err := cfg.validateDefaultTagsCollisionWithTrackingTags(); err != nil {
197205
return err
198206
}
@@ -217,6 +225,27 @@ func (cfg *ControllerConfig) Validate() error {
217225
return nil
218226
}
219227

228+
func (cfg *ControllerConfig) validateDefaultSubnets() error {
229+
if len(cfg.DefaultSubnets) == 0 {
230+
return nil
231+
}
232+
for _, subnetID := range cfg.DefaultSubnets {
233+
if !strings.HasPrefix(subnetID, "subnet-") {
234+
return errors.Errorf("invalid value %v for default subnet id", subnetID)
235+
}
236+
}
237+
238+
//validate duplicate subnet ids
239+
seen := make(map[string]bool)
240+
for _, str := range cfg.DefaultSubnets {
241+
if seen[str] {
242+
return errors.Errorf("duplicate subnet id %v is specified in the --default-subnets flag", str)
243+
}
244+
seen[str] = true
245+
}
246+
return nil
247+
}
248+
220249
func (cfg *ControllerConfig) validateDefaultTagsCollisionWithTrackingTags() error {
221250
for tagKey := range cfg.DefaultTags {
222251
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: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
4848
annotationParser annotations.Parser, subnetsResolver networkingpkg.SubnetsResolver,
4949
authConfigBuilder AuthConfigBuilder, enhancedBackendBuilder EnhancedBackendBuilder,
5050
trackingProvider tracking.Provider, elbv2TaggingManager elbv2deploy.TaggingManager, featureGates config.FeatureGates,
51-
vpcID string, clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string, defaultLoadBalancerScheme string,
51+
vpcID string, clusterName string, defaultSubnets []string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string, defaultLoadBalancerScheme string,
5252
backendSGProvider networkingpkg.BackendSGProvider, sgResolver networkingpkg.SecurityGroupResolver,
5353
enableBackendSG bool, defaultEnableManageBackendSGRules bool, disableRestrictedSGRules bool, allowedCAARNs []string, enableIPTargetType bool, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector) *defaultModelBuilder {
5454
certDiscovery := certs.NewACMCertDiscovery(acmClient, allowedCAARNs, logger)
@@ -71,6 +71,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
7171
trackingProvider: trackingProvider,
7272
elbv2TaggingManager: elbv2TaggingManager,
7373
featureGates: featureGates,
74+
defaultSubnets: defaultSubnets,
7475
defaultTags: defaultTags,
7576
externalManagedTags: sets.NewString(externalManagedTags...),
7677
defaultSSLPolicy: defaultSSLPolicy,
@@ -108,6 +109,7 @@ type defaultModelBuilder struct {
108109
trackingProvider tracking.Provider
109110
elbv2TaggingManager elbv2deploy.TaggingManager
110111
featureGates config.FeatureGates
112+
defaultSubnets []string
111113
defaultTags map[string]string
112114
externalManagedTags sets.String
113115
defaultSSLPolicy string
@@ -156,6 +158,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group, metrics
156158
stack: stack,
157159
frontendNlbTargetGroupDesiredState: frontendNlbTargetGroupDesiredState,
158160

161+
defaultSubnets: b.defaultSubnets,
159162
defaultTags: b.defaultTags,
160163
externalManagedTags: b.externalManagedTags,
161164
defaultIPAddressType: elbv2model.IPAddressTypeIPV4,
@@ -215,6 +218,7 @@ type defaultModelBuildTask struct {
215218
disableRestrictedSGRules bool
216219
enableIPTargetType bool
217220

221+
defaultSubnets []string
218222
defaultTags map[string]string
219223
externalManagedTags sets.String
220224
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.
@@ -415,7 +424,7 @@ func (r *defaultSubnetsResolver) validateSpecifiedSubnets(ctx context.Context, s
415424
// chooseAndValidateSubnetsPerAZ will choose one subnet per AZ from eligible subnets and then validate against chosen subnets.
416425
func (r *defaultSubnetsResolver) chooseAndValidateSubnetsPerAZ(ctx context.Context, subnets []ec2types.Subnet, resolveOpts SubnetsResolveOptions) ([]ec2types.Subnet, error) {
417426
categorizedSubnets := r.categorizeSubnetsByEligibility(subnets)
418-
chosenSubnets := r.chooseSubnetsPerAZ(categorizedSubnets.eligible)
427+
chosenSubnets := r.chooseSubnetsPerAZ(categorizedSubnets.eligible, resolveOpts.DefaultSubnets)
419428
if len(chosenSubnets) == 0 {
420429
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",
421430
len(subnets), len(categorizedSubnets.ineligibleClusterTag), len(categorizedSubnets.insufficientIPs))
@@ -455,7 +464,15 @@ func (r *defaultSubnetsResolver) categorizeSubnetsByEligibility(subnets []ec2typ
455464

456465
// chooseSubnetsPerAZ will choose one subnet per AZ.
457466
// * subnets with current cluster tag will be prioritized.
458-
func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) []ec2types.Subnet {
467+
func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet, defaultSubnets []string) []ec2types.Subnet {
468+
469+
prioritySubnetMap := make(map[string]int)
470+
471+
if len(defaultSubnets) > 0 {
472+
for i, subnetID := range defaultSubnets {
473+
prioritySubnetMap[subnetID] = i
474+
}
475+
}
459476
subnetsByAZ := mapSDKSubnetsByAZ(subnets)
460477
chosenSubnets := make([]ec2types.Subnet, 0, len(subnetsByAZ))
461478
for az, azSubnets := range subnetsByAZ {
@@ -470,9 +487,24 @@ func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) [
470487
} else if (!subnetIHasCurrentClusterTag) && subnetJHasCurrentClusterTag {
471488
return false
472489
}
490+
491+
// When azSubnets are specified in --default-azSubnets, the azSubnets list will be sorted according to this order.
492+
// Any azSubnets not specified in --default-azSubnets will be sorted in lexicographical order and placed after the prioritized azSubnets.
493+
iVal, iExists := prioritySubnetMap[awssdk.ToString(azSubnets[i].SubnetId)]
494+
jVal, jExists := prioritySubnetMap[awssdk.ToString(azSubnets[j].SubnetId)]
495+
496+
if iExists && jExists {
497+
return iVal < jVal
498+
}
499+
if iExists {
500+
return true
501+
}
502+
if jExists {
503+
return false
504+
}
473505
return awssdk.ToString(azSubnets[i].SubnetId) < awssdk.ToString(azSubnets[j].SubnetId)
474506
})
475-
r.logger.V(1).Info("multiple subnets in the same AvailabilityZone", "AvailabilityZone", az,
507+
r.logger.V(1).Info("multiple azSubnets in the same AvailabilityZone", "AvailabilityZone", az,
476508
"chosen", azSubnets[0].SubnetId, "ignored", extractSubnetIDs(azSubnets[1:]))
477509
chosenSubnets = append(chosenSubnets, azSubnets[0])
478510
}

0 commit comments

Comments
 (0)