Skip to content

Commit f25c48f

Browse files
oliviassssoliviassss
authored
call subnet auto discovery when lb scheme changes (#2387)
* call subnet auto discovery when lb scheme changes * add unit tests
1 parent 65a371a commit f25c48f

File tree

4 files changed

+98
-17
lines changed

4 files changed

+98
-17
lines changed

‎pkg/ingress/model_build_load_balancer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont
220220
return nil, err
221221
}
222222

223-
if len(sdkLBs) == 0 {
223+
if len(sdkLBs) == 0 || (string(scheme) != awssdk.StringValue(sdkLBs[0].LoadBalancer.Scheme)) {
224224
chosenSubnets, err := t.subnetsResolver.ResolveViaDiscovery(ctx,
225225
networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication),
226226
networking.WithSubnetsResolveLBScheme(scheme),

‎pkg/ingress/model_builder_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2453,6 +2453,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
24532453
SubnetId: awssdk.String("subnet-f"),
24542454
},
24552455
},
2456+
Scheme: awssdk.String("internal"),
24562457
},
24572458
Tags: map[string]string{
24582459
"elbv2.k8s.aws/cluster": "cluster-name",
@@ -2470,6 +2471,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
24702471
SubnetId: awssdk.String("subnet-f"),
24712472
},
24722473
},
2474+
Scheme: awssdk.String("internal"),
24732475
},
24742476
Tags: map[string]string{
24752477
"keyA": "valueA2",
@@ -2487,6 +2489,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
24872489
SubnetId: awssdk.String("subnet-f"),
24882490
},
24892491
},
2492+
Scheme: awssdk.String("internal"),
24902493
},
24912494
Tags: map[string]string{
24922495
"keyA": "valueA3",

‎pkg/service/model_build_load_balancer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnets(ctx context.Context, sc
259259
if err != nil {
260260
return nil, err
261261
}
262-
if existingLB != nil {
262+
if existingLB != nil && string(scheme) == aws.StringValue(existingLB.LoadBalancer.Scheme) {
263263
availabilityZones := existingLB.LoadBalancer.AvailabilityZones
264264
subnetIDs := make([]string, 0, len(availabilityZones))
265265
for _, availabilityZone := range availabilityZones {

‎pkg/service/model_build_load_balancer_test.go

Lines changed: 93 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,8 @@ func Test_defaultModelBuilderTask_buildLoadBalancerSubnets(t *testing.T) {
556556
listLoadBalancersCalls []listLoadBalancerCall
557557
resolveViaDiscoveryCalls []resolveSubnetResults
558558
resolveViaNameOrIDSlilceCalls []resolveSubnetResults
559-
want map[string]string
559+
want []*ec2.Subnet
560+
wantErr error
560561
}{
561562
{
562563
name: "subnet auto-discovery",
@@ -579,9 +580,15 @@ func Test_defaultModelBuilderTask_buildLoadBalancerSubnets(t *testing.T) {
579580
},
580581
},
581582
},
582-
want: map[string]string{
583-
"elbv2.k8s.aws/cluster": "cluster-name",
584-
"service.k8s.aws/stack": "namespace/serviceName",
583+
want: []*ec2.Subnet{
584+
{
585+
SubnetId: aws.String("subnet-a"),
586+
CidrBlock: aws.String("192.168.0.0/19"),
587+
},
588+
{
589+
SubnetId: aws.String("subnet-b"),
590+
CidrBlock: aws.String("192.168.32.0/19"),
591+
},
585592
},
586593
},
587594
{
@@ -610,13 +617,19 @@ func Test_defaultModelBuilderTask_buildLoadBalancerSubnets(t *testing.T) {
610617
},
611618
},
612619
},
613-
want: map[string]string{
614-
"elbv2.k8s.aws/cluster": "cluster-name",
615-
"service.k8s.aws/stack": "namespace/serviceName",
620+
want: []*ec2.Subnet{
621+
{
622+
SubnetId: aws.String("subnet-abc"),
623+
CidrBlock: aws.String("192.168.0.0/19"),
624+
},
625+
{
626+
SubnetId: aws.String("subnet-xyz"),
627+
CidrBlock: aws.String("192.168.0.0/19"),
628+
},
616629
},
617630
},
618631
{
619-
name: "subnet resolve via Name or ID",
632+
name: "subnet resolve via Name or ID, with existing LB and scheme wouldn't change",
620633
svc: &corev1.Service{},
621634
scheme: elbv2.LoadBalancerSchemeInternal,
622635
provider: tracking.NewDefaultProvider("service.k8s.aws", "cluster-name"),
@@ -635,6 +648,7 @@ func Test_defaultModelBuilderTask_buildLoadBalancerSubnets(t *testing.T) {
635648
SubnetId: aws.String("subnet-d"),
636649
},
637650
},
651+
Scheme: aws.String("internal"),
638652
},
639653
Tags: map[string]string{
640654
"elbv2.k8s.aws/cluster": "cluster-name",
@@ -658,9 +672,70 @@ func Test_defaultModelBuilderTask_buildLoadBalancerSubnets(t *testing.T) {
658672
},
659673
},
660674
},
661-
want: map[string]string{
662-
"elbv2.k8s.aws/cluster": "cluster-name",
663-
"service.k8s.aws/stack": "namespace/serviceName",
675+
want: []*ec2.Subnet{
676+
{
677+
SubnetId: aws.String("subnet-c"),
678+
CidrBlock: aws.String("192.168.0.0/19"),
679+
},
680+
{
681+
SubnetId: aws.String("subnet-d"),
682+
CidrBlock: aws.String("192.168.0.0/19"),
683+
},
684+
},
685+
},
686+
{
687+
name: "subnet auto discovery, with existing LB and scheme would change",
688+
svc: &corev1.Service{},
689+
scheme: elbv2.LoadBalancerSchemeInternal,
690+
provider: tracking.NewDefaultProvider("service.k8s.aws", "cluster-name"),
691+
args: args{stack: core.NewDefaultStack(core.StackID{Namespace: "namespace", Name: "serviceName"})},
692+
listLoadBalancersCalls: []listLoadBalancerCall{
693+
{
694+
sdkLBs: []elbv2deploy.LoadBalancerWithTags{
695+
{
696+
LoadBalancer: &elbv2sdk.LoadBalancer{
697+
LoadBalancerArn: aws.String("lb-1"),
698+
AvailabilityZones: []*elbv2sdk.AvailabilityZone{
699+
{
700+
SubnetId: aws.String("subnet-c"),
701+
},
702+
{
703+
SubnetId: aws.String("subnet-d"),
704+
},
705+
},
706+
Scheme: aws.String("internet-facing"),
707+
},
708+
Tags: map[string]string{
709+
"elbv2.k8s.aws/cluster": "cluster-name",
710+
"service.k8s.aws/stack": "namespace/serviceName",
711+
},
712+
},
713+
},
714+
},
715+
},
716+
resolveViaDiscoveryCalls: []resolveSubnetResults{
717+
{
718+
subnets: []*ec2.Subnet{
719+
{
720+
SubnetId: aws.String("subnet-a"),
721+
CidrBlock: aws.String("192.168.0.0/19"),
722+
},
723+
{
724+
SubnetId: aws.String("subnet-b"),
725+
CidrBlock: aws.String("192.168.0.0/19"),
726+
},
727+
},
728+
},
729+
},
730+
want: []*ec2.Subnet{
731+
{
732+
SubnetId: aws.String("subnet-a"),
733+
CidrBlock: aws.String("192.168.0.0/19"),
734+
},
735+
{
736+
SubnetId: aws.String("subnet-b"),
737+
CidrBlock: aws.String("192.168.0.0/19"),
738+
},
664739
},
665740
},
666741
}
@@ -695,10 +770,13 @@ func Test_defaultModelBuilderTask_buildLoadBalancerSubnets(t *testing.T) {
695770
trackingProvider: trackingProvider,
696771
elbv2TaggingManager: elbv2TaggingManager,
697772
}
698-
var got = tt.provider.StackTags(tt.args.stack)
699-
assert.Equal(t, tt.want, got)
700-
701-
builder.buildLoadBalancerSubnets(context.Background(), tt.scheme)
773+
got, err := builder.buildLoadBalancerSubnets(context.Background(), tt.scheme)
774+
if tt.wantErr != nil {
775+
assert.EqualError(t, err, tt.wantErr.Error())
776+
} else {
777+
assert.NoError(t, err)
778+
assert.Equal(t, tt.want, got)
779+
}
702780
})
703781
}
704782
}

0 commit comments

Comments
 (0)