Skip to content

Commit b761926

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 b761926

File tree

22 files changed

+592
-348
lines changed

22 files changed

+592
-348
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 == nil || 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("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: 8 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"
@@ -207,6 +213,7 @@ type denyResp struct {
207213

208214
type CloudFormationAPI interface {
209215
cloudformation.DescribeStacksAPIClient
216+
cloudformation.ListStackResourcesAPIClient
210217
CreateStack(context.Context, *cloudformation.CreateStackInput, ...func(*cloudformation.Options)) (*cloudformation.CreateStackOutput, error)
211218
UpdateTerminationProtection(context.Context, *cloudformation.UpdateTerminationProtectionInput, ...func(*cloudformation.Options)) (*cloudformation.UpdateTerminationProtectionOutput, error)
212219
UpdateStack(context.Context, *cloudformation.UpdateStackInput, ...func(*cloudformation.Options)) (*cloudformation.UpdateStackOutput, error)
@@ -486,6 +493,7 @@ func mapToManagedStack(stack *types.Stack) *Stack {
486493

487494
return &Stack{
488495
Name: aws.ToString(stack.StackName),
496+
LoadBalancerARN: outputs.loadBalancerARN(),
489497
DNSName: outputs.dnsName(),
490498
TargetGroupARNs: outputs.targetGroupARNs(),
491499
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
}

0 commit comments

Comments
 (0)