Skip to content

Commit 1fee90b

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 1fee90b

File tree

13 files changed

+660
-228
lines changed

13 files changed

+660
-228
lines changed

‎aws/adapter.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,29 @@ func (a *Adapter) UpdateStack(ctx context.Context, stackName string, certificate
852852
return updateStack(ctx, a.cloudformation, spec)
853853
}
854854

855+
func (a *Adapter) GetLoadBalancer(ctx context.Context, stackName string) (*elbv2Types.LoadBalancer, error) {
856+
lbStackResources, err := getLoadBalancerStackResource(ctx, a.cloudformation, stackName)
857+
if err != nil {
858+
return nil, fmt.Errorf("failed to get load balancer stack resource: %w", err)
859+
}
860+
861+
loadBalancerID := aws.ToString(lbStackResources.PhysicalResourceId)
862+
lbs, err := a.elbv2.DescribeLoadBalancers(ctx, &elbv2.DescribeLoadBalancersInput{
863+
LoadBalancerArns: []string{loadBalancerID},
864+
})
865+
866+
if err != nil {
867+
return nil, fmt.Errorf("failed to describe load balancer %q: %w", loadBalancerID, err)
868+
}
869+
if len(lbs.LoadBalancers) == 0 {
870+
return nil, fmt.Errorf("load balancer %q not found", loadBalancerID)
871+
}
872+
if len(lbs.LoadBalancers) > 1 {
873+
return nil, fmt.Errorf("multiple load balancers found for physical-Id %q", loadBalancerID)
874+
}
875+
return &lbs.LoadBalancers[0], nil
876+
}
877+
855878
func (a *Adapter) httpTargetPort(loadBalancerType string) uint {
856879
if loadBalancerType == LoadBalancerTypeApplication && a.albHTTPTargetPort != 0 {
857880
return a.albHTTPTargetPort

‎aws/cf.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ type denyResp struct {
207207

208208
type CloudFormationAPI interface {
209209
cloudformation.DescribeStacksAPIClient
210+
cloudformation.ListStackResourcesAPIClient
210211
CreateStack(context.Context, *cloudformation.CreateStackInput, ...func(*cloudformation.Options)) (*cloudformation.CreateStackOutput, error)
211212
UpdateTerminationProtection(context.Context, *cloudformation.UpdateTerminationProtectionInput, ...func(*cloudformation.Options)) (*cloudformation.UpdateTerminationProtectionOutput, error)
212213
UpdateStack(context.Context, *cloudformation.UpdateStackInput, ...func(*cloudformation.Options)) (*cloudformation.UpdateStackOutput, error)
@@ -434,6 +435,46 @@ func getStack(ctx context.Context, svc CloudFormationAPI, stackName string) (*St
434435
return mapToManagedStack(stack), nil
435436
}
436437

438+
// getLoadBalancerStackResource retrieves the load balancer resource from a
439+
// CloudFormation stack. It returns the first resource of type
440+
// AWS::ElasticLoadBalancingV2::LoadBalancer found in the stack. The stack
441+
// should only have one such resource, as it is expected to be a managed load
442+
// balancer stack.
443+
func getLoadBalancerStackResource(
444+
ctx context.Context,
445+
svc CloudFormationAPI,
446+
stackName string,
447+
) (
448+
*types.StackResourceSummary,
449+
error,
450+
) {
451+
452+
var nextToken *string
453+
454+
for {
455+
resp, err := svc.ListStackResources(ctx, &cloudformation.ListStackResourcesInput{
456+
StackName: aws.String(stackName),
457+
NextToken: nextToken,
458+
})
459+
460+
if err != nil {
461+
return nil, fmt.Errorf("failed to list stack resources for stack %s: %w", stackName, err)
462+
}
463+
464+
for _, resource := range resp.StackResourceSummaries {
465+
if aws.ToString(resource.LogicalResourceId) == LoadBalancerResourceLogicalID {
466+
return &resource, nil
467+
}
468+
}
469+
470+
if resp.NextToken == nil {
471+
return nil, fmt.Errorf("no load balancer resource found in stack %s", stackName)
472+
}
473+
474+
nextToken = resp.NextToken
475+
}
476+
}
477+
437478
func getCFStackByName(ctx context.Context, svc CloudFormationAPI, stackName string) (*types.Stack, error) {
438479
params := &cloudformation.DescribeStacksInput{StackName: aws.String(stackName)}
439480

‎aws/cf_template.go

Lines changed: 16 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 {
@@ -117,7 +124,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
117124
template.Outputs = map[string]*cloudformation.Output{
118125
outputLoadBalancerDNSName: {
119126
Description: "DNS name for the LoadBalancer",
120-
Value: cloudformation.GetAtt("LB", "DNSName").String(),
127+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "DNSName").String(),
121128
},
122129
outputTargetGroupARN: {
123130
Description: "The ARN of the TargetGroup",
@@ -160,7 +167,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
160167
},
161168
},
162169
},
163-
LoadBalancerArn: cloudformation.Ref("LB").String(),
170+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
164171
Port: cloudformation.Integer(80),
165172
Protocol: cloudformation.String("HTTP"),
166173
})
@@ -172,7 +179,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
172179
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
173180
},
174181
},
175-
LoadBalancerArn: cloudformation.Ref("LB").String(),
182+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
176183
Port: cloudformation.Integer(80),
177184
Protocol: cloudformation.String("HTTP"),
178185
})
@@ -196,7 +203,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
196203
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
197204
},
198205
},
199-
LoadBalancerArn: cloudformation.Ref("LB").String(),
206+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
200207
Port: cloudformation.Integer(80),
201208
Protocol: cloudformation.String("TCP"),
202209
})
@@ -227,7 +234,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
227234
CertificateArn: cloudformation.String(certificateARNs[0]),
228235
},
229236
},
230-
LoadBalancerArn: cloudformation.Ref("LB").String(),
237+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
231238
Port: cloudformation.Integer(443),
232239
Protocol: cloudformation.String("HTTPS"),
233240
SslPolicy: cloudformation.Ref(parameterListenerSslPolicyParameter).String(),
@@ -256,7 +263,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
256263
CertificateArn: cloudformation.String(certificateARNs[0]),
257264
},
258265
},
259-
LoadBalancerArn: cloudformation.Ref("LB").String(),
266+
LoadBalancerArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
260267
Port: cloudformation.Integer(443),
261268
Protocol: cloudformation.String("TLS"),
262269
SslPolicy: cloudformation.Ref(parameterListenerSslPolicyParameter).String(),
@@ -379,17 +386,17 @@ func generateTemplate(spec *stackSpec) (string, error) {
379386
lb.Type = cloudformation.Ref(parameterLoadBalancerTypeParameter).String()
380387
}
381388

382-
template.AddResource("LB", lb)
389+
template.AddResource(LoadBalancerResourceLogicalID, lb)
383390

384391
if spec.loadbalancerType == LoadBalancerTypeApplication && spec.wafWebAclId != "" {
385392
if strings.HasPrefix(spec.wafWebAclId, "arn:aws:wafv2:") {
386393
template.AddResource("WAFAssociation", &cloudformation.WAFv2WebACLAssociation{
387-
ResourceArn: cloudformation.Ref("LB").String(),
394+
ResourceArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
388395
WebACLArn: cloudformation.Ref(parameterLoadBalancerWAFWebACLIDParameter).String(),
389396
})
390397
} else {
391398
template.AddResource("WAFAssociation", &cloudformation.WAFRegionalWebACLAssociation{
392-
ResourceArn: cloudformation.Ref("LB").String(),
399+
ResourceArn: cloudformation.Ref(LoadBalancerResourceLogicalID).String(),
393400
WebACLID: cloudformation.Ref(parameterLoadBalancerWAFWebACLIDParameter).String(),
394401
})
395402
}

‎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")

‎aws/cf_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,84 @@ func TestGetStack(t *testing.T) {
784784
}
785785
}
786786

787+
func TestGetLoadBalancerStackResource(t *testing.T) {
788+
789+
for _, ti := range []struct {
790+
name string
791+
given fake.CFOutputs
792+
want *types.StackResourceSummary
793+
wantErr bool
794+
}{
795+
{
796+
"managed load balancer resource found",
797+
fake.CFOutputs{
798+
ListStackResources: fake.R(&cloudformation.ListStackResourcesOutput{
799+
StackResourceSummaries: []types.StackResourceSummary{
800+
{
801+
LogicalResourceId: aws.String(LoadBalancerResourceLogicalID),
802+
ResourceType: aws.String("AWS::ElasticLoadBalancingV2::LoadBalancer"),
803+
ResourceStatus: types.ResourceStatusCreateComplete,
804+
ResourceStatusReason: aws.String(""),
805+
},
806+
}}, nil),
807+
},
808+
&types.StackResourceSummary{
809+
LogicalResourceId: aws.String(LoadBalancerResourceLogicalID),
810+
ResourceType: aws.String("AWS::ElasticLoadBalancingV2::LoadBalancer"),
811+
ResourceStatus: types.ResourceStatusCreateComplete,
812+
ResourceStatusReason: aws.String(""),
813+
},
814+
false,
815+
},
816+
{
817+
"no load balancer resource",
818+
fake.CFOutputs{
819+
ListStackResources: fake.R(&cloudformation.ListStackResourcesOutput{
820+
StackResourceSummaries: []types.StackResourceSummary{
821+
{
822+
LogicalResourceId: aws.String("SomeOtherResourceLogicalID"),
823+
ResourceType: aws.String("AWS::Some::OtherResource"),
824+
ResourceStatus: types.ResourceStatusCreateComplete,
825+
ResourceStatusReason: aws.String(""),
826+
},
827+
}}, nil),
828+
},
829+
nil,
830+
true,
831+
},
832+
{
833+
"unmanaged load balancer",
834+
fake.CFOutputs{
835+
ListStackResources: fake.R(&cloudformation.ListStackResourcesOutput{
836+
StackResourceSummaries: []types.StackResourceSummary{
837+
{
838+
LogicalResourceId: aws.String("unmanaged-load-balancer"),
839+
ResourceType: aws.String("AWS::ElasticLoadBalancingV2::LoadBalancer"),
840+
ResourceStatus: types.ResourceStatusCreateComplete,
841+
ResourceStatusReason: aws.String(""),
842+
},
843+
}}, nil),
844+
},
845+
nil,
846+
true,
847+
},
848+
} {
849+
t.Run(ti.name, func(t *testing.T) {
850+
c := &fake.CFClient{Outputs: ti.given}
851+
got, err := getLoadBalancerStackResource(context.Background(), c, "dontcare")
852+
if err != nil {
853+
if !ti.wantErr {
854+
t.Error("unexpected error", err)
855+
}
856+
} else {
857+
if !reflect.DeepEqual(ti.want, got) {
858+
t.Errorf("unexpected result. wanted %+v, got %+v", ti.want, got)
859+
}
860+
}
861+
})
862+
}
863+
}
864+
787865
func TestShouldDelete(t *testing.T) {
788866
for _, ti := range []struct {
789867
msg string

‎aws/cloudwatch.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func normalizeCloudWatchAlarmDimensions(alarmDimensions *cloudformation.CloudWat
7777
return &cloudformation.CloudWatchAlarmDimensionList{
7878
{
7979
Name: cloudformation.String("LoadBalancer"),
80-
Value: cloudformation.GetAtt("LB", "LoadBalancerFullName").String(),
80+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "LoadBalancerFullName").String(),
8181
},
8282
}
8383
}
@@ -89,7 +89,7 @@ func normalizeCloudWatchAlarmDimensions(alarmDimensions *cloudformation.CloudWat
8989

9090
switch dimension.Name.Literal {
9191
case "LoadBalancer":
92-
value = cloudformation.GetAtt("LB", "LoadBalancerFullName").String()
92+
value = cloudformation.GetAtt(LoadBalancerResourceLogicalID, "LoadBalancerFullName").String()
9393
case "TargetGroup":
9494
value = cloudformation.GetAtt("TG", "TargetGroupFullName").String()
9595
}

‎aws/cloudwatch_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func TestNormalizeCloudWatchAlarmDimensions(t *testing.T) {
103103
expected: &cloudformation.CloudWatchAlarmDimensionList{
104104
{
105105
Name: cloudformation.String("LoadBalancer"),
106-
Value: cloudformation.GetAtt("LB", "LoadBalancerFullName").String(),
106+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "LoadBalancerFullName").String(),
107107
},
108108
},
109109
},
@@ -113,7 +113,7 @@ func TestNormalizeCloudWatchAlarmDimensions(t *testing.T) {
113113
expected: &cloudformation.CloudWatchAlarmDimensionList{
114114
{
115115
Name: cloudformation.String("LoadBalancer"),
116-
Value: cloudformation.GetAtt("LB", "LoadBalancerFullName").String(),
116+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "LoadBalancerFullName").String(),
117117
},
118118
},
119119
},
@@ -135,7 +135,7 @@ func TestNormalizeCloudWatchAlarmDimensions(t *testing.T) {
135135
expected: &cloudformation.CloudWatchAlarmDimensionList{
136136
{
137137
Name: cloudformation.String("LoadBalancer"),
138-
Value: cloudformation.GetAtt("LB", "LoadBalancerFullName").String(),
138+
Value: cloudformation.GetAtt(LoadBalancerResourceLogicalID, "LoadBalancerFullName").String(),
139139
},
140140
{
141141
Name: cloudformation.String("TargetGroup"),

0 commit comments

Comments
 (0)