Support Custom Pod Status for PodReadinessGate to Block Premature Pod Termination

[BrianChristie]

Presently when using IP target mode on the ALB, it's difficult to guarantee zero downtime rolling updates. This is because there is additional state in the ALB Target Group which is not propagated back to Kubernetes: initial,draining, and healthy.

This was discussed in: #660 & #814

This issue is to discuss adding support to the ALB controller for setting a custom Pod Status which mirrors the ALB TargetGroup. This can then be used with PodReadinessGate and MaxUnavailable / PodDisruptionBudget to ensure a rolling update does not proceed faster than the new Pods becoming healthy on the ALB.

• KEP: Pod Readiness Gates kubernetes/enhancements#580
• Docs: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-readiness-gate

[alfredkrohmer]

I'm working on this.

[alfredkrohmer]

I won't have time to continue working on this in the next two weeks, but I have pushed an initial implementation here: https://github.com/devkid/aws-alb-ingress-controller/tree/feature/pod-readiness-gate (it compiles and has documentation, but I didn't test it yet nor did I adjust the unit tests).

It works by reconciling the pod conditions in the same step where it reconciles the ALB target group state. I think it wouldn't work in its current state because right now only the Ready endpoints of a service are registered with the ALB. When a pod declares the ALB readiness gate, it would never turn Ready because it would wait for the readiness gate (turn healthy in ALB), which would never be fulfilled because the pod was never registered with the ALB (because it was not Ready yet) - so endless loop. To solve this, the ALB ingress controller would need to support the publishNotReadyAddresses attribute of a Kubernetes services and it would need to be set to true if one would want to use this pod readiness gate feature (then all pods - also unready ones - would be registered with the target group, pass the health check and only then appear Ready in kubernetes).

If someone feels like picking up from where I left, feel free to do so. Otherwise I would probably continue in ~2-3 weeks.

[tecnobrat]

Hey @devkid -- just want to check-in to see if you have had a chance to get back to this change at all.

This is impacting us and we're open to trying to help with any PRs that need to be done if you need help.

Thanks!

[alfredkrohmer]

Hi @tecnobrat. I didn't find the time to continue with this significantly, but the functionality should be usable as far as I can tell. I created a draft PR here: #955 and added some TODOs. The controller itself is building, but the target group unit test is broken because we need to pass in another parameter to the targets controller (k8s client). Also it's still missing additional unit tests for the new functionality.

I'll be gone for another 2 weeks now. Maybe you can pick this up if you want to.

[boynux]

Hi,
We hit this issue in our new EKS cluster which prevents us moving forward with the production workload, any progress with the proposed PR?

[alfredkrohmer]

Well, the functionality is there, I'm still waiting for reviews. You can try the code from my branch if you want to, that would be helpful feedback already.
I'll be meeting some EKS people tomorrow and will try to push them to get this reviewed by some of them.

[alfredkrohmer]

Btw a workaround would be to have at least 2+n replicas and maxUnavailable = 0+n and an initialDelaySeconds that is large enough to cover the time it takes to register a target with an target group and for it to become healthy; this should prevent the last pod from terminating until this delay elapses for the second new pod (which is not yet registered with the target group, but in the meantime the first new pod was ready long enough to be healthy in the target group).

[gzzo]

@devkid Would a pod be registered on the ALB before it's ready?

I had a similar hypothesis a while ago and I tried with 6 replicas, 0 maxUnavailable, and 60 minReadySeconds but I still saw 504s with IP target mode.

[nirnanaaa]

@devkid there are still some issues with your approach. I'm trying to come up with a follow-up PR to yours so we can maybe share the work on this.

Especially the endpoint and reconciliation part is very much open for discussion: https://github.com/kubernetes-sigs/aws-alb-ingress-controller/compare/master...nirnanaaa:feature/pod-readiness-gate#diff-c6555fa828feb3fa0daf4ff60736047dR130

One question @devkid: How do we handle ALB TG changes that kubernetes does not receive immediately (where no reconcile will be triggered). Ingress-Gce uses a poller to query their NEGs, should we follow a similar strategy?

[wirescrossed]

We are having a similar issue as well. When pods are terminated the ALB Ingress Controller rapidly updates the ALB target group however the changes take a few seconds to take effect which causes a short period where clients may get 504s.

We are using IP mode meaning each Pod is in the target group since we require the real ALB source IP not the source IP kube-proxy uses in order to use NetworkPolicies.

To workaround this we are taking the approach of running kube-proxy without source NAT (https://kubernetes.io/docs/tutorials/services/source-ip/) though this has draw backs as well but masks the issue to a degree.

What I would really like to see is one of two things:

• ALB to keep pace with the rapid changes in k8s
• k8s give the ALB time to catch up by delaying termination of the pod.

[nirnanaaa]

I would be willing to continue @devkid s and my work on this PR. Maybe in collaboration with someone from AWS that could give us insight on the Polling-thing i was talking about earlier.

/cc @andrewtandoc

[iverberk]

We are hitting this as well and it's a blocker for production workloads. @devkid @nirnanaaa I was going to review and finish this PR also, so let's explore the problem space and collaborate on a design that we can implement. It is actually very similar to what Google needed to solve for their loadbalancers (23:00 in this video https://youtu.be/Vw9GmSeomFg)

[nirnanaaa]

@iverberk you're absolutely right. We were able to solve most of the challenges by taking a look at ingress-gce (basically this piece is the relevant one: https://github.com/kubernetes/ingress-gce/tree/5b2fd1df8634ca02aaa10072d7769a2544d39cb5/pkg/neg/readiness)

Except for the poller everything works just fine (didn't figure out a way, to make that work).

Maybe you have something to overcome that issue, already? See @devkid s PR and my changes here: https://github.com/kubernetes-sigs/aws-alb-ingress-controller/compare/master...nirnanaaa:feature/pod-readiness-gate#diff-c6555fa828feb3fa0daf4ff60736047dR130