Skip to content

driver/kubernetes: skip terminating pods when choosing a pod - #3997

Open
chagui wants to merge 1 commit into
docker:masterfrom
chagui:fix/driver/kubernetes/skip-terminating-pods
Open

driver/kubernetes: skip terminating pods when choosing a pod#3997
chagui wants to merge 1 commit into
docker:masterfrom
chagui:fix/driver/kubernetes/skip-terminating-pods

Conversation

@chagui

@chagui chagui commented Aug 5, 2026

Copy link
Copy Markdown

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.

ListRunningPods currently filters on pod.Status.Phase alone. A pod marked for deletion keeps Phase=Running for 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 the buildkitd container.

Skipping pods with a DeletionTimestamp matches how upstream Kubernetes decides set membership: IsPodActive in pkg/controller/controller_utils.go pairs the phase checks with DeletionTimestamp == nil. This doesn't close the race entirely, since a pod can be deleted between the List and 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 kind cluster
kind create cluster --name buildkit-terminating

# create a builder
docker buildx create --driver kubernetes --name kindbuilder \
  --driver-opt namespace=default,replicas=2,loadbalance=random --bootstrap

# buildx sets no terminationGracePeriodSeconds, so pods default to 30s.
# Widen it so a terminating pod stays observable.
kubectl patch deploy kindbuilder0 \
  -p '{"spec":{"template":{"spec":{"terminationGracePeriodSeconds":900}}}}'
kubectl rollout status deploy/kindbuilder0

Create a Dockerfile which takes some time, eg.:

FROM alpine:latest
RUN echo "step1 starting" && sleep 900 && echo "step1 done"

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.

for p in $(kubectl get pods -l app=kindbuilder0 -o name); do
  echo "$p busy=$(kubectl exec $p -- ps aux 2>/dev/null | grep -c '[s]leep 900')"
done

kubectl delete pod <the busy one> --wait=false

# wait for the replacement, so ReadyReplicas >= replicas and Bootstrap passes
kubectl wait --for=condition=available --timeout=120s deploy/kindbuilder0

kubectl get pods -l app=kindbuilder0 \
  -o custom-columns='NAME:.metadata.name,PHASE:.status.phase,DEL:.metadata.deletionTimestamp'
# NAME                            PHASE     DEL
# kindbuilder0-6bb75db655-5nq7s   Running   <none>
# kindbuilder0-6bb75db655-rprcs   Running   <none>
# kindbuilder0-74d679dd75-nw96b   Running   2026-08-05T10:10:19Z   <- terminating, still Running

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:

pod running: "kindbuilder0-6bb75db655-5nq7s"
pod running: "kindbuilder0-6bb75db655-rprcs"
pod running: "kindbuilder0-74d679dd75-nw96b"
RandomPodChooser.ChoosePod(): len(pods)=3

With this PR, against the same three pods:

pod running: "kindbuilder0-6bb75db655-5nq7s"
pod running: "kindbuilder0-6bb75db655-rprcs"
pod terminating, skipping: "kindbuilder0-74d679dd75-nw96b"
RandomPodChooser.ChoosePod(): len(pods)=2

Signed-off-by: Guilhem Charles <guilhem.charles@gmail.com>

@crazy-max crazy-max left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

PTAL @AkihiroSuda

@crazy-max
crazy-max requested a review from AkihiroSuda August 5, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants