Skip to content

Commit 2c63caf

Browse files
author
Thilina Madumal
committed
ingress-controller: update ingresses only if the load balancer is active
As per the existing behavior, once the stack update completes, the ingress controller immediately updates the status of all ingresses and routegroups to reference the new load balancer. This update may sometimes happens before the new load balancer has marked its targets (skipper-ingress) as healthy, leading clients being routed to a load balancer that is not ready to serve traffic yet. To improve this behavior we retrieve the ELBs via aws api and build the models including the ELB state of the corresponding ELB. Then before updating the ingresses/routegroups (updateIngress func) we check whether the ELB is in active state, if not we skip updating the ingresses/routegroups status with the ELB hostname. Signed-off-by: Thilina Madumal <[email protected]>
1 parent 06ce657 commit 2c63caf

File tree

21 files changed

+588
-344
lines changed

21 files changed

+588
-344
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,4 @@ Dockerfile.upstream
3636
profile.cov
3737

3838
.idea/
39+
.vscode/

‎aws/adapter.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,60 @@ func (a *Adapter) FindManagedStacks(ctx context.Context) ([]*Stack, error) {
660660
return stacks, nil
661661
}
662662

663+
// GetStackELBs returns the list of load balancers of the managed stacks.
664+
// While implementing this method it was taken that each stack has its own load
665+
// balancer and never refer to the same load balancer from multiple stacks.
666+
//
667+
// If a stack does not have a load balancer ARN it will be returned with an empty
668+
// ELB field.
669+
func (a *Adapter) GetStackELBs(ctx context.Context, stacks []*Stack) ([]*StackELB, error) {
670+
671+
lbARNs := make([]string, 0, len(stacks))
672+
for _, stack := range stacks {
673+
if stack.LoadBalancerARN == "" {
674+
continue
675+
}
676+
lbARNs = append(lbARNs, stack.LoadBalancerARN)
677+
}
678+
679+
lbsMap := make(map[string]*elbv2Types.LoadBalancer, len(lbARNs))
680+
681+
paginator := elbv2.NewDescribeLoadBalancersPaginator(
682+
a.elbv2,
683+
&elbv2.DescribeLoadBalancersInput{
684+
LoadBalancerArns: lbARNs,
685+
},
686+
)
687+
for paginator.HasMorePages() {
688+
elbOutput, err := paginator.NextPage(ctx)
689+
if err != nil {
690+
return nil, fmt.Errorf("describe-load-balancers paginated call to AWS API failed: %w", err)
691+
}
692+
for _, lb := range elbOutput.LoadBalancers {
693+
lbsMap[aws.ToString(lb.LoadBalancerArn)] = &lb
694+
}
695+
}
696+
697+
stackELBs := make([]*StackELB, 0, len(stacks))
698+
for _, stack := range stacks {
699+
if stack == nil {
700+
continue
701+
}
702+
var elb *elbv2Types.LoadBalancer
703+
if lbsMap[stack.LoadBalancerARN] != nil {
704+
elb = lbsMap[stack.LoadBalancerARN]
705+
} else {
706+
log.Warnf("The load balancer of %s stack is not found", stack.Name)
707+
}
708+
stackELB := &StackELB{
709+
Stack: stack,
710+
ELB: elb,
711+
}
712+
stackELBs = append(stackELBs, stackELB)
713+
}
714+
return stackELBs, nil
715+
}
716+
663717
// UpdateTargetGroupsAndAutoScalingGroups updates Auto Scaling Groups
664718
// config to have relevant Target Groups and registers/deregisters single
665719
// instances (that do not belong to ASG) in relevant Target Groups.

‎aws/adapter_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,3 +1028,112 @@ func TestWithTargetAccessMode(t *testing.T) {
10281028
assert.False(t, a.TargetCNI.Enabled)
10291029
})
10301030
}
1031+
1032+
func TestGetStackELBs(t *testing.T) {
1033+
tests := []struct {
1034+
name string
1035+
elbv2Outputs fake.ELBv2Outputs
1036+
stacks []*Stack
1037+
expectedStackELBs []*StackELB
1038+
expectError bool
1039+
}{
1040+
{
1041+
name: "success - matching ELBs",
1042+
elbv2Outputs: fake.ELBv2Outputs{
1043+
DescribeLoadBalancers: fake.R(&elbv2.DescribeLoadBalancersOutput{
1044+
LoadBalancers: []elbv2Types.LoadBalancer{
1045+
{LoadBalancerArn: aws.String("arn1"), LoadBalancerName: aws.String("lb1")},
1046+
{LoadBalancerArn: aws.String("arn2"), LoadBalancerName: aws.String("lb2")},
1047+
},
1048+
}, nil),
1049+
},
1050+
stacks: []*Stack{
1051+
{Name: "stack1", LoadBalancerARN: "arn1"},
1052+
{Name: "stack2", LoadBalancerARN: "arn2"},
1053+
},
1054+
expectedStackELBs: []*StackELB{
1055+
{
1056+
Stack: &Stack{Name: "stack1", LoadBalancerARN: "arn1"},
1057+
ELB: &elbv2Types.LoadBalancer{LoadBalancerArn: aws.String("arn1"), LoadBalancerName: aws.String("lb1")},
1058+
},
1059+
{
1060+
Stack: &Stack{Name: "stack2", LoadBalancerARN: "arn2"},
1061+
ELB: &elbv2Types.LoadBalancer{LoadBalancerArn: aws.String("arn2"), LoadBalancerName: aws.String("lb2")},
1062+
},
1063+
},
1064+
expectError: false,
1065+
},
1066+
{
1067+
name: "success - no matching ELB found for one of the stacks",
1068+
elbv2Outputs: fake.ELBv2Outputs{
1069+
DescribeLoadBalancers: fake.R(&elbv2.DescribeLoadBalancersOutput{
1070+
LoadBalancers: []elbv2Types.LoadBalancer{
1071+
{LoadBalancerArn: aws.String("arn1"), LoadBalancerName: aws.String("lb1")},
1072+
},
1073+
}, nil),
1074+
},
1075+
stacks: []*Stack{
1076+
{Name: "stack1", LoadBalancerARN: "arn1"},
1077+
{Name: "stack2", LoadBalancerARN: "arn2"},
1078+
},
1079+
expectedStackELBs: []*StackELB{
1080+
{
1081+
Stack: &Stack{Name: "stack1", LoadBalancerARN: "arn1"},
1082+
ELB: &elbv2Types.LoadBalancer{LoadBalancerArn: aws.String("arn1"), LoadBalancerName: aws.String("lb1")},
1083+
},
1084+
{
1085+
Stack: &Stack{Name: "stack2", LoadBalancerARN: "arn2"},
1086+
ELB: nil, // No matching ELB found for stack2
1087+
},
1088+
},
1089+
expectError: false,
1090+
},
1091+
{
1092+
name: "error - paginated DescribeLoadBalancers API failure",
1093+
elbv2Outputs: fake.ELBv2Outputs{
1094+
DescribeLoadBalancers: fake.R(nil, fmt.Errorf("API error")),
1095+
},
1096+
stacks: []*Stack{{Name: "stack1", LoadBalancerARN: "arn1"}},
1097+
expectedStackELBs: nil,
1098+
expectError: true,
1099+
},
1100+
{
1101+
name: "success - no stacks",
1102+
elbv2Outputs: fake.ELBv2Outputs{
1103+
DescribeLoadBalancers: fake.R(&elbv2.DescribeLoadBalancersOutput{
1104+
LoadBalancers: []elbv2Types.LoadBalancer{},
1105+
}, nil),
1106+
},
1107+
stacks: []*Stack{},
1108+
expectedStackELBs: []*StackELB{},
1109+
expectError: false,
1110+
},
1111+
}
1112+
1113+
for _, test := range tests {
1114+
t.Run(test.name, func(t *testing.T) {
1115+
a := &Adapter{
1116+
elbv2: &fake.ELBv2Client{
1117+
Outputs: test.elbv2Outputs,
1118+
},
1119+
}
1120+
1121+
stackELBs, err := a.GetStackELBs(context.Background(), test.stacks)
1122+
1123+
if test.expectError {
1124+
require.Error(t, err)
1125+
} else {
1126+
require.NoError(t, err)
1127+
require.Len(t, stackELBs, len(test.expectedStackELBs))
1128+
for i, expected := range test.expectedStackELBs {
1129+
require.Equal(t, expected.Stack, stackELBs[i].Stack)
1130+
if expected.ELB == nil {
1131+
require.Nil(t, stackELBs[i].ELB)
1132+
} else {
1133+
require.Equal(t, expected.ELB.LoadBalancerArn, stackELBs[i].ELB.LoadBalancerArn)
1134+
}
1135+
}
1136+
}
1137+
})
1138+
}
1139+
}

‎aws/cf.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type Stack struct {
2424
Name string
2525
status types.StackStatus
2626
statusReason string
27+
LoadBalancerARN string
2728
DNSName string
2829
Scheme string
2930
SecurityGroup string
@@ -105,6 +106,10 @@ func newStackOutput(outputs []types.Output) stackOutput {
105106
return result
106107
}
107108

109+
func (o stackOutput) loadBalancerARN() string {
110+
return o[outputLoadBalancerARN]
111+
}
112+
108113
func (o stackOutput) dnsName() string {
109114
return o[outputLoadBalancerDNSName]
110115
}
@@ -131,6 +136,7 @@ func convertStackParameters(parameters []types.Parameter) map[string]string {
131136

132137
const (
133138
// The following constants should be part of the Output section of the CloudFormation template
139+
outputLoadBalancerARN = "LoadBalancerARN"
134140
outputLoadBalancerDNSName = "LoadBalancerDNSName"
135141
outputTargetGroupARN = "TargetGroupARN"
136142
outputHTTPTargetGroupARN = "HTTPTargetGroupARN"
@@ -486,6 +492,7 @@ func mapToManagedStack(stack *types.Stack) *Stack {
486492

487493
return &Stack{
488494
Name: aws.ToString(stack.StackName),
495+
LoadBalancerARN: outputs.loadBalancerARN(),
489496
DNSName: outputs.dnsName(),
490497
TargetGroupARNs: outputs.targetGroupARNs(),
491498
Scheme: parameters[parameterLoadBalancerSchemeParameter],

‎aws/cf_template.go

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ const (
2424
//
2525
// [0]: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-listenerrule.html#cfn-elasticloadbalancingv2-listenerrule-priority
2626
internalTrafficDenyRulePriority int64 = 1
27+
28+
// LoadBalancerResou rceLogicalID is the logical ID of the LoadBalancer
29+
// resource in the CloudFormation template.
30+
//
31+
// !!!Caution
32+
// Changing this value will recreate the LoadBalancer resource.
33+
LoadBalancerResourceLogicalID = "LB"
2734
)
2835

2936
func hashARNs(certARNs []string) []byte {
@@ -115,9 +122,13 @@ func generateTemplate(spec *stackSpec) (string, error) {
115122
const httpsTargetGroupName = "TG"
116123

117124
template.Outputs = map[string]*cloudformation.Output{
125+
outputLoadBalancerARN: {
126+
Description: "The ARN of the LoadBalancer",
127+
Value: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
128+
},
118129
outputLoadBalancerDNSName: {
119130
Description: "DNS name for the LoadBalancer",
120-
Value: cloudformation.GetAtt("LB", "DNSName").String(),
131+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "DNSName").String(),
121132
},
122133
outputTargetGroupARN: {
123134
Description: "The ARN of the TargetGroup",
@@ -160,7 +171,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
160171
},
161172
},
162173
},
163-
LoadBalancerArn: cloudformation.Ref("LB").String(),
174+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
164175
Port: cloudformation.Integer(80),
165176
Protocol: cloudformation.String("HTTP"),
166177
})
@@ -172,7 +183,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
172183
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
173184
},
174185
},
175-
LoadBalancerArn: cloudformation.Ref("LB").String(),
186+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
176187
Port: cloudformation.Integer(80),
177188
Protocol: cloudformation.String("HTTP"),
178189
})
@@ -196,7 +207,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
196207
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
197208
},
198209
},
199-
LoadBalancerArn: cloudformation.Ref("LB").String(),
210+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
200211
Port: cloudformation.Integer(80),
201212
Protocol: cloudformation.String("TCP"),
202213
})
@@ -227,7 +238,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
227238
CertificateArn: cloudformation.String(certificateARNs[0]),
228239
},
229240
},
230-
LoadBalancerArn: cloudformation.Ref("LB").String(),
241+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
231242
Port: cloudformation.Integer(443),
232243
Protocol: cloudformation.String("HTTPS"),
233244
SslPolicy: cloudformation.Ref(parameterListenerSslPolicyParameter).String(),
@@ -256,7 +267,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
256267
CertificateArn: cloudformation.String(certificateARNs[0]),
257268
},
258269
},
259-
LoadBalancerArn: cloudformation.Ref("LB").String(),
270+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
260271
Port: cloudformation.Integer(443),
261272
Protocol: cloudformation.String("TLS"),
262273
SslPolicy: cloudformation.Ref(parameterListenerSslPolicyParameter).String(),
@@ -379,17 +390,17 @@ func generateTemplate(spec *stackSpec) (string, error) {
379390
lb.Type = cloudformation.Ref(parameterLoadBalancerTypeParameter).String()
380391
}
381392

382-
template.AddResource("LB", lb)
393+
template.AddResource(LoadBalancerResourceLogicalID, lb)
383394

384395
if spec.loadbalancerType == LoadBalancerTypeApplication && spec.wafWebAclId != "" {
385396
if strings.HasPrefix(spec.wafWebAclId, "arn:aws:wafv2:") {
386397
template.AddResource("WAFAssociation", &cloudformation.WAFv2WebACLAssociation{
387-
ResourceArn: cloudformation.Ref("LB").String(),
398+
ResourceArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
388399
WebACLArn: cloudformation.Ref(parameterLoadBalancerWAFWebACLIDParameter).String(),
389400
})
390401
} else {
391402
template.AddResource("WAFAssociation", &cloudformation.WAFRegionalWebACLAssociation{
392-
ResourceArn: cloudformation.Ref("LB").String(),
403+
ResourceArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
393404
WebACLID: cloudformation.Ref(parameterLoadBalancerWAFWebACLIDParameter).String(),
394405
})
395406
}

‎aws/cf_template_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func TestGenerateTemplate(t *testing.T) {
123123
&cloudformation.CloudWatchAlarmDimensionList{
124124
{
125125
Name: cloudformation.String("LoadBalancer"),
126-
Value: cloudformation.GetAtt("LB", "LoadBalancerFullName").String(),
126+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "LoadBalancerFullName").String(),
127127
},
128128
},
129129
props.Dimensions,
@@ -270,8 +270,8 @@ func TestGenerateTemplate(t *testing.T) {
270270
nlbCrossZone: true,
271271
},
272272
validate: func(t *testing.T, template *cloudformation.Template) {
273-
require.NotNil(t, template.Resources["LB"])
274-
properties := template.Resources["LB"].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
273+
require.NotNil(t, template.Resources[LoadBalancerResourceLogicalID])
274+
properties := template.Resources[LoadBalancerResourceLogicalID].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
275275
attributes := []cloudformation.ElasticLoadBalancingV2LoadBalancerLoadBalancerAttribute(*properties.LoadBalancerAttributes)
276276
require.Equal(t, attributes[0].Key.Literal, "load_balancing.cross_zone.enabled")
277277
require.Equal(t, attributes[0].Value.Literal, "true")
@@ -284,8 +284,8 @@ func TestGenerateTemplate(t *testing.T) {
284284
nlbCrossZone: false,
285285
},
286286
validate: func(t *testing.T, template *cloudformation.Template) {
287-
require.NotNil(t, template.Resources["LB"])
288-
properties := template.Resources["LB"].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
287+
require.NotNil(t, template.Resources[LoadBalancerResourceLogicalID])
288+
properties := template.Resources[LoadBalancerResourceLogicalID].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
289289
attributes := []cloudformation.ElasticLoadBalancingV2LoadBalancerLoadBalancerAttribute(*properties.LoadBalancerAttributes)
290290
require.Equal(t, attributes[0].Key.Literal, "load_balancing.cross_zone.enabled")
291291
require.Equal(t, attributes[0].Value.Literal, "false")
@@ -298,8 +298,8 @@ func TestGenerateTemplate(t *testing.T) {
298298
nlbZoneAffinity: "any_availability_zone",
299299
},
300300
validate: func(t *testing.T, template *cloudformation.Template) {
301-
require.NotNil(t, template.Resources["LB"])
302-
properties := template.Resources["LB"].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
301+
require.NotNil(t, template.Resources[LoadBalancerResourceLogicalID])
302+
properties := template.Resources[LoadBalancerResourceLogicalID].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
303303
attributes := []cloudformation.ElasticLoadBalancingV2LoadBalancerLoadBalancerAttribute(*properties.LoadBalancerAttributes)
304304
require.Equal(t, attributes[1].Key.Literal, "dns_record.client_routing_policy")
305305
require.Equal(t, attributes[1].Value.Literal, "any_availability_zone")
@@ -312,8 +312,8 @@ func TestGenerateTemplate(t *testing.T) {
312312
nlbZoneAffinity: "availability_zone_affinity",
313313
},
314314
validate: func(t *testing.T, template *cloudformation.Template) {
315-
require.NotNil(t, template.Resources["LB"])
316-
properties := template.Resources["LB"].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
315+
require.NotNil(t, template.Resources[LoadBalancerResourceLogicalID])
316+
properties := template.Resources[LoadBalancerResourceLogicalID].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
317317
attributes := []cloudformation.ElasticLoadBalancingV2LoadBalancerLoadBalancerAttribute(*properties.LoadBalancerAttributes)
318318
require.Equal(t, attributes[1].Key.Literal, "dns_record.client_routing_policy")
319319
require.Equal(t, attributes[1].Value.Literal, "availability_zone_affinity")
@@ -326,8 +326,8 @@ func TestGenerateTemplate(t *testing.T) {
326326
nlbZoneAffinity: "partial_availability_zone_affinity",
327327
},
328328
validate: func(t *testing.T, template *cloudformation.Template) {
329-
require.NotNil(t, template.Resources["LB"])
330-
properties := template.Resources["LB"].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
329+
require.NotNil(t, template.Resources[LoadBalancerResourceLogicalID])
330+
properties := template.Resources[LoadBalancerResourceLogicalID].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
331331
attributes := []cloudformation.ElasticLoadBalancingV2LoadBalancerLoadBalancerAttribute(*properties.LoadBalancerAttributes)
332332
require.Equal(t, attributes[1].Key.Literal, "dns_record.client_routing_policy")
333333
require.Equal(t, attributes[1].Value.Literal, "partial_availability_zone_affinity")
@@ -352,8 +352,8 @@ func TestGenerateTemplate(t *testing.T) {
352352
http2: true,
353353
},
354354
validate: func(t *testing.T, template *cloudformation.Template) {
355-
require.NotNil(t, template.Resources["LB"])
356-
properties := template.Resources["LB"].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
355+
require.NotNil(t, template.Resources[LoadBalancerResourceLogicalID])
356+
properties := template.Resources[LoadBalancerResourceLogicalID].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
357357
attributes := []cloudformation.ElasticLoadBalancingV2LoadBalancerLoadBalancerAttribute(*properties.LoadBalancerAttributes)
358358
require.Equal(t, attributes[1].Key.Literal, "routing.http2.enabled")
359359
require.Equal(t, attributes[1].Value.Literal, "true")
@@ -366,8 +366,8 @@ func TestGenerateTemplate(t *testing.T) {
366366
http2: false,
367367
},
368368
validate: func(t *testing.T, template *cloudformation.Template) {
369-
require.NotNil(t, template.Resources["LB"])
370-
properties := template.Resources["LB"].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
369+
require.NotNil(t, template.Resources[LoadBalancerResourceLogicalID])
370+
properties := template.Resources[LoadBalancerResourceLogicalID].Properties.(*cloudformation.ElasticLoadBalancingV2LoadBalancer)
371371
attributes := []cloudformation.ElasticLoadBalancingV2LoadBalancerLoadBalancerAttribute(*properties.LoadBalancerAttributes)
372372
require.Equal(t, attributes[1].Key.Literal, "routing.http2.enabled")
373373
require.Equal(t, attributes[1].Value.Literal, "false")

0 commit comments

Comments
 (0)