ingress-controller: update ingresses only if the load balancer is active #748
Conversation
madumalt
force-pushed
the
update-ingress-only-if-elb-is-ready-to-serve
branch
from
cfdc821 to
1fee90b
Compare
|
👍 |
aws/cf.go
Outdated
| var nextToken *string | ||
|
|
||
| for { | ||
| resp, err := svc.ListStackResources(ctx, &cloudformation.ListStackResourcesInput{ |
There was a problem hiding this comment.
Here we do another API call (or maybe even several) to get loadbalancer ARN. I think we could store it in the stack output like we do for LoadBalancerDNSName and target group ARNs so that we have it right after FindManagedStacks without extra API calls.
There was a problem hiding this comment.
We we still want to use API call then we should use describe-stack-resource instead of listing all resources and filtering on client side
$ aws cloudformation describe-stack-resource --stack-name=$STACK_ANME --logical-resource-id LBThere was a problem hiding this comment.
Added the LoadBalancerARN in the output section of the Cloudformation Stack Template.
As per this reddit post, without updating a resource an update to an output is not effective.
I wonder how can we overcome this issue. maybe by doing some dummy update to the stack?
There was a problem hiding this comment.
trick: add an AWS Tag.
Tested the changes in teapot, we do not need to trigger stack update. The template get updated and when we query the AWS API then we get the output with the new the field. All good!
madumalt
force-pushed
the
update-ingress-only-if-elb-is-ready-to-serve
branch
from
1fee90b to
b761926
Compare
| paginator := elbv2.NewDescribeLoadBalancersPaginator( | ||
| a.elbv2, | ||
| &elbv2.DescribeLoadBalancersInput{ | ||
| LoadBalancerArns: lbARNs, |
There was a problem hiding this comment.
What happens if we have more than 20 ARNs?
There was a problem hiding this comment.
Paginator takes care of it if I'm not mistaken. This is the way it was done with the stacks retrieval, I guess the same limitation applies there as well right?
There was a problem hiding this comment.
I'll double check this anyways
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]>
madumalt
force-pushed
the
update-ingress-only-if-elb-is-ready-to-serve
branch
from
b761926 to
2c63caf
Compare
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 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.