Repository navigation
driver/kubernetes: skip terminating pods when choosing a pod - #3997
Conversation
Signed-off-by: Guilhem Charles <guilhem.charles@gmail.com>
crazy-max
left a comment
There was a problem hiding this comment.
LGTM thanks!
PTAL @AkihiroSuda
So, the bug is still there but it will be order of magnitudes harder to reproduce and debug it? =/ |
In our case, this takes the window from 900 seconds down to sub-second, which significantly reduces the chance of targeting a terminating pod. From what we observed, the builds getting killed are not the ones being scheduled just after a pod is marked as terminating but the ones getting scheduled on terminating pods for which the remaining grace period is short (e.g. under 30 seconds). Builds scheduled in the sub-second window would be fine. |
We noticed in our production environment that some builds are routed to terminating pods. A build routed to such a pod is killed mid-flight when the grace period expires, and the client doesn't fail fast in that case, so it hangs until the caller's own timeout (Refs #556). In CI that means the job burns its whole time budget instead of failing and retrying elsewhere.
Even with recent fixes for #556 we would benefit from not sending builds to terminating pods in the first place.
ListRunningPodscurrently filters onpod.Status.Phasealone. A pod marked for deletion keepsPhase=Runningfor its entire termination grace period, so it stays selectable right up until its containers are killed. Our set-up configure a preStop hook with a 900secs grade period for thebuildkitdcontainer.Skipping pods with a
DeletionTimestampmatches how upstream Kubernetes decides set membership:IsPodActiveinpkg/controller/controller_utils.gopairs the phase checks withDeletionTimestamp == nil. This doesn't close the race entirely, since a pod can be deleted between theListand the exec, but it shrinks the window from minutes to sub-second.Added unit tests for the pod filtering and both pod choosers; the package had none.
Repro / testing
Setup:
Create a Dockerfile which takes some time, eg.:
Start a build and leave it running. This keeps one pod busy, which is what makes it linger later: GracefulStop waits for the in-flight Solve instead of exiting on SIGTERM.
docker buildx build --builder kindbuilder --progress=plain -f Dockerfile .Once step1 starting appears, find the busy pod and delete only that one. Delete an idle pod instead and buildkitd exits immediately, so nothing lingers.
Now run a second build and look at the candidate list.
Note: I used a custom binary with log statement patched to Info level because the debug level was not reliable.
Before, the terminating pod is a valid candidate:
With this PR, against the same three pods: