Skip to content

Commit c8befeb

Browse files
madumaltmadumalt
authored
fix: update ingress status only if the load balancer is active (#748)
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 (for example 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 states of the corresponding ELBs. 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 <thilina.madumal@zalando.de>
1 parent 2f695c6 commit c8befeb

File tree

23 files changed

+826
-344
lines changed

23 files changed

+826
-344
lines changed

‎.github/workflows/ci.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ jobs:
1313
- run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
1414
- run: make test
1515
- run: make lint
16+
- run: grep -Ev "zalando-incubator/kube-ingress-aws-controller/(certs|aws)/fake/" profile.cov > tmp.profile.cov
17+
- run: mv tmp.profile.cov profile.cov
1618
- run: goveralls -coverprofile=profile.cov -service=github
1719
env:
1820
COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}

‎.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: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,42 @@ func (a *Adapter) FindManagedStacks(ctx context.Context) ([]*Stack, error) {
660660
return stacks, nil
661661
}
662662

663+
// GetStackLBStates 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 a nil
668+
// LBState field.
669+
func (a *Adapter) GetStackLBStates(ctx context.Context, stacks []*Stack) ([]*StackLBState, 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, err := getLoadBalancerStates(ctx, a.elbv2, lbARNs)
680+
if err != nil {
681+
return nil, fmt.Errorf("failed to get load balancer states: %w", err)
682+
}
683+
684+
stackLBStates := make([]*StackLBState, 0, len(stacks))
685+
for _, stack := range stacks {
686+
lbstate, found := lbsMap[stack.LoadBalancerARN]
687+
if !found {
688+
log.Warnf("The load balancer (ARN: %q) of %q stack is not found", stack.LoadBalancerARN, stack.Name)
689+
}
690+
stackELB := &StackLBState{
691+
Stack: stack,
692+
LBState: lbstate,
693+
}
694+
stackLBStates = append(stackLBStates, stackELB)
695+
}
696+
return stackLBStates, nil
697+
}
698+
663699
// UpdateTargetGroupsAndAutoScalingGroups updates Auto Scaling Groups
664700
// config to have relevant Target Groups and registers/deregisters single
665701
// instances (that do not belong to ASG) in relevant Target Groups.

‎aws/adapter_test.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,3 +1028,138 @@ func TestWithTargetAccessMode(t *testing.T) {
10281028
assert.False(t, a.TargetCNI.Enabled)
10291029
})
10301030
}
1031+
1032+
func TestGetStackLBStates(t *testing.T) {
1033+
tests := []struct {
1034+
name string
1035+
elbv2Outputs fake.ELBv2Outputs
1036+
stacks []*Stack
1037+
expectedStackLBStates []*StackLBState
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+
{
1046+
LoadBalancerArn: aws.String("arn1"),
1047+
State: &elbv2Types.LoadBalancerState{Code: elbv2Types.LoadBalancerStateEnumActive},
1048+
},
1049+
{
1050+
LoadBalancerArn: aws.String("arn2"),
1051+
State: &elbv2Types.LoadBalancerState{Code: elbv2Types.LoadBalancerStateEnumFailed},
1052+
},
1053+
},
1054+
}, nil),
1055+
},
1056+
stacks: []*Stack{
1057+
{Name: "stack1", LoadBalancerARN: "arn1"},
1058+
{Name: "stack2", LoadBalancerARN: "arn2"},
1059+
},
1060+
expectedStackLBStates: []*StackLBState{
1061+
{
1062+
Stack: &Stack{Name: "stack1", LoadBalancerARN: "arn1"},
1063+
LBState: &LoadBalancerState{StateCode: elbv2Types.LoadBalancerStateEnumActive},
1064+
},
1065+
{
1066+
Stack: &Stack{Name: "stack2", LoadBalancerARN: "arn2"},
1067+
LBState: &LoadBalancerState{StateCode: elbv2Types.LoadBalancerStateEnumFailed},
1068+
},
1069+
},
1070+
expectError: false,
1071+
},
1072+
{
1073+
name: "success - no matching ELB found for one of the stacks",
1074+
elbv2Outputs: fake.ELBv2Outputs{
1075+
DescribeLoadBalancers: fake.R(&elbv2.DescribeLoadBalancersOutput{
1076+
LoadBalancers: []elbv2Types.LoadBalancer{
1077+
{
1078+
LoadBalancerArn: aws.String("arn1"),
1079+
State: &elbv2Types.LoadBalancerState{Code: elbv2Types.LoadBalancerStateEnumActive},
1080+
},
1081+
},
1082+
}, nil),
1083+
},
1084+
stacks: []*Stack{
1085+
{Name: "stack1", LoadBalancerARN: "arn1"},
1086+
{Name: "stack2", LoadBalancerARN: "arn2"},
1087+
},
1088+
expectedStackLBStates: []*StackLBState{
1089+
{
1090+
Stack: &Stack{Name: "stack1", LoadBalancerARN: "arn1"},
1091+
LBState: &LoadBalancerState{StateCode: elbv2Types.LoadBalancerStateEnumActive},
1092+
},
1093+
{
1094+
Stack: &Stack{Name: "stack2", LoadBalancerARN: "arn2"},
1095+
LBState: nil, // No matching ELB found for stack2
1096+
},
1097+
},
1098+
expectError: false,
1099+
},
1100+
{
1101+
name: "error - DescribeLoadBalancers API failure",
1102+
elbv2Outputs: fake.ELBv2Outputs{
1103+
DescribeLoadBalancers: fake.R(nil, fmt.Errorf("API error")),
1104+
},
1105+
stacks: []*Stack{{Name: "stack1", LoadBalancerARN: "arn1"}},
1106+
expectedStackLBStates: nil,
1107+
expectError: true,
1108+
},
1109+
{
1110+
name: "success - no LB arn in some stacks",
1111+
elbv2Outputs: fake.ELBv2Outputs{
1112+
DescribeLoadBalancers: fake.R(&elbv2.DescribeLoadBalancersOutput{
1113+
LoadBalancers: []elbv2Types.LoadBalancer{
1114+
{
1115+
LoadBalancerArn: aws.String("arn1"),
1116+
State: &elbv2Types.LoadBalancerState{Code: elbv2Types.LoadBalancerStateEnumActive},
1117+
},
1118+
},
1119+
}, nil),
1120+
},
1121+
stacks: []*Stack{
1122+
{Name: "stack1", LoadBalancerARN: "arn1"},
1123+
{Name: "stack2"},
1124+
},
1125+
expectedStackLBStates: []*StackLBState{
1126+
{
1127+
Stack: &Stack{Name: "stack1", LoadBalancerARN: "arn1"},
1128+
LBState: &LoadBalancerState{StateCode: elbv2Types.LoadBalancerStateEnumActive},
1129+
},
1130+
{
1131+
Stack: &Stack{Name: "stack2"},
1132+
LBState: nil, // No LoadBalancerARN provided for stack2
1133+
},
1134+
},
1135+
expectError: false,
1136+
},
1137+
}
1138+
1139+
for _, test := range tests {
1140+
t.Run(test.name, func(t *testing.T) {
1141+
a := &Adapter{
1142+
elbv2: &fake.ELBv2Client{
1143+
Outputs: test.elbv2Outputs,
1144+
},
1145+
}
1146+
1147+
stackLBStates, err := a.GetStackLBStates(context.Background(), test.stacks)
1148+
1149+
if test.expectError {
1150+
require.Error(t, err)
1151+
} else {
1152+
require.NoError(t, err)
1153+
require.Len(t, stackLBStates, len(test.expectedStackLBStates))
1154+
for i, expected := range test.expectedStackLBStates {
1155+
require.Equal(t, expected.Stack, stackLBStates[i].Stack)
1156+
if expected.LBState == nil {
1157+
require.Nil(t, stackLBStates[i].LBState)
1158+
} else {
1159+
require.Equal(t, expected.LBState.StateCode, stackLBStates[i].LBState.StateCode)
1160+
}
1161+
}
1162+
}
1163+
})
1164+
}
1165+
}

‎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: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ 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+
// LoadBalancerResourceLogicalID is the logical ID of the LoadBalancer resource in the CloudFormation template.
29+
// Changing this value will recreate the LoadBalancer resource.
30+
LoadBalancerResourceLogicalID = "LB"
2731
)
2832

2933
func hashARNs(certARNs []string) []byte {
@@ -115,9 +119,13 @@ func generateTemplate(spec *stackSpec) (string, error) {
115119
const httpsTargetGroupName = "TG"
116120

117121
template.Outputs = map[string]*cloudformation.Output{
122+
outputLoadBalancerARN: {
123+
Description: "The ARN of the LoadBalancer",
124+
Value: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
125+
},
118126
outputLoadBalancerDNSName: {
119127
Description: "DNS name for the LoadBalancer",
120-
Value: cloudformation.GetAtt("LB", "DNSName").String(),
128+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "DNSName").String(),
121129
},
122130
outputTargetGroupARN: {
123131
Description: "The ARN of the TargetGroup",
@@ -160,7 +168,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
160168
},
161169
},
162170
},
163-
LoadBalancerArn: cloudformation.Ref("LB").String(),
171+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
164172
Port: cloudformation.Integer(80),
165173
Protocol: cloudformation.String("HTTP"),
166174
})
@@ -172,7 +180,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
172180
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
173181
},
174182
},
175-
LoadBalancerArn: cloudformation.Ref("LB").String(),
183+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
176184
Port: cloudformation.Integer(80),
177185
Protocol: cloudformation.String("HTTP"),
178186
})
@@ -196,7 +204,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
196204
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
197205
},
198206
},
199-
LoadBalancerArn: cloudformation.Ref("LB").String(),
207+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
200208
Port: cloudformation.Integer(80),
201209
Protocol: cloudformation.String("TCP"),
202210
})
@@ -227,7 +235,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
227235
CertificateArn: cloudformation.String(certificateARNs[0]),
228236
},
229237
},
230-
LoadBalancerArn: cloudformation.Ref("LB").String(),
238+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
231239
Port: cloudformation.Integer(443),
232240
Protocol: cloudformation.String("HTTPS"),
233241
SslPolicy: cloudformation.Ref(parameterListenerSslPolicyParameter).String(),
@@ -256,7 +264,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
256264
CertificateArn: cloudformation.String(certificateARNs[0]),
257265
},
258266
},
259-
LoadBalancerArn: cloudformation.Ref("LB").String(),
267+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
260268
Port: cloudformation.Integer(443),
261269
Protocol: cloudformation.String("TLS"),
262270
SslPolicy: cloudformation.Ref(parameterListenerSslPolicyParameter).String(),
@@ -379,17 +387,17 @@ func generateTemplate(spec *stackSpec) (string, error) {
379387
lb.Type = cloudformation.Ref(parameterLoadBalancerTypeParameter).String()
380388
}
381389

382-
template.AddResource("LB", lb)
390+
template.AddResource(LoadBalancerResourceLogicalID, lb)
383391

384392
if spec.loadbalancerType == LoadBalancerTypeApplication && spec.wafWebAclId != "" {
385393
if strings.HasPrefix(spec.wafWebAclId, "arn:aws:wafv2:") {
386394
template.AddResource("WAFAssociation", &cloudformation.WAFv2WebACLAssociation{
387-
ResourceArn: cloudformation.Ref("LB").String(),
395+
ResourceArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
388396
WebACLArn: cloudformation.Ref(parameterLoadBalancerWAFWebACLIDParameter).String(),
389397
})
390398
} else {
391399
template.AddResource("WAFAssociation", &cloudformation.WAFRegionalWebACLAssociation{
392-
ResourceArn: cloudformation.Ref("LB").String(),
400+
ResourceArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
393401
WebACLID: cloudformation.Ref(parameterLoadBalancerWAFWebACLIDParameter).String(),
394402
})
395403
}

0 commit comments

Comments
 (0)