Re-queue worker pods on transient 502/503/504 from the Kubernetes API#68995
Closed
1fanwang wants to merge 7 commits into
Closed
Re-queue worker pods on transient 502/503/504 from the Kubernetes API#689951fanwang wants to merge 7 commits into
1fanwang wants to merge 7 commits into
Conversation
The KubernetesExecutor creates worker pods one at a time within each scheduler loop: run_next() issues a synchronous create_namespaced_pod and blocks on the API response before starting the next. When per-create latency is high (API round-trip, mutating admission webhooks), this serializes the scheduler loop and caps pod-creation throughput per loop. Add an opt-in concurrent path: when [kubernetes_executor] async_pod_creation is True, the pods dequeued in a loop are built synchronously (pod-mutation hook and reconciliation unchanged), then their create calls are issued concurrently via the asynchronous Kubernetes client, bounded by pod_creation_max_concurrency. The sequential path stays the default and is unchanged. A new pod_creation_batch_duration metric (emitted by both paths) measures per-loop batch creation time.
Move the request-timeout ApiClient wrappers into kube_client.py and build the executor's async client from _TimeoutAsyncK8sApiClient, so async creates carry the same client-side request timeout as the rest of the provider and the timeout logic lives in one place. Drop the config-parse test that only restated values already exercised by the async-creation tests.
Replace the str(e.status) == "NNN" comparisons in the pod-publish error handler with http.HTTPStatus constants; type the timeout-wrapper call_api signatures; and return the concurrent batch's per-pod errors as a position-aligned list instead of a dict keyed on id(job).
…ync type The shared pod-publish error handler now matches both the sync and async client ApiException (they expose the same status/body/reason/headers), mirroring _should_retry_api. The concurrent path re-raises its native exception, so the async-to-sync conversion helper is gone.
Trim the batch-metric and concurrent-create docstrings to the why, drop the refactor-history reference in _build_pod_request, and condense the positional result-mapping comment.
Add explicit type hints to the remaining locals in the new code (batch counters and timings, the dequeued job, the error-handler body, and the gather outcomes) for consistency with the rest of the diff. Verified with mypy.
The KubernetesExecutor's shared pod-publish error handler re-queues exceeded-quota / stale-version conflicts, 500, and 429, but fails the task outright on 502/503/504 — the gateway / unavailable / timeout codes that spike when the API server or an admission webhook is under load. A shutting-down apiserver returns 503 with Retry-After on every graceful roll, so an in-flight pod CREATE during a rollout currently kills the task. Treat these as transient and re-queue them (bounded by task_publish_max_retries), honoring Retry-After to back the loop off the same way the 429 path does — matching what client-go already does for all 5xx.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
KubernetesExecutor._handle_pod_publish_errorre-queues a worker-pod CREATE that fails withan exceeded-quota / stale-version conflict,
500, or429, but lets502/503/504fall through and fail the task immediately. Those are exactly the gateway / unavailable /
timeout codes that spike when the API server or an admission webhook is under load — the regime
this executor path is meant to survive. Concretely, the API server returns
503withRetry-After: 1on every graceful shutdown (rollout, node drain, control-plane autoscale —see
apiserverwaitgroup.go), so an in-flight pod CREATE during any apiserver roll currentlykills the task outright.
client-goretries all 5xx and honoursRetry-After; the executor doesnot.
What
Add
502/503/504to the transient set in the shared_handle_pod_publish_error, so atask that hits one is re-queued (bounded by
task_publish_max_retries) instead of failed. Becauseboth the sequential and the concurrent creation paths route through that one method, this hardens
both at once, using the executor's existing re-queue-next-loop model — no in-line blocking retry
that would stall the scheduler loop.
When the response carries
Retry-After(a shutting-down apiserver always sends it on503),pause the whole loop until then via
create_pods_after, exactly as the429path already does;other transient 5xx without the header simply retry on the next loop like
500. TheRetry-Afterparse is guarded so a malformed value can't crash the scheduler loop.
Stacked on #68480 — the shared handler this extends was introduced there.
Tests
Unit, covering both clients through the shared handler:
502/504re-queue and retry on thenext loop;
503 + Retry-Afterre-queues and backs the loop off;503with retries exhaustedfails; and the async client's
503setscreate_pods_after. Each fails without this change.E2E
Live
kind(real API server, realKubernetesExecutor.sync()), with a proxy injecting503 + Retry-After: 1on the first CREATE per task — identical config (task_publish_max_retries=3),only the provider code differs:
retries=3)503→ failed503→ all 6 re-queued, backoff503→ re-queued each loop, no failuresRaw — after (async path)
Risk
Behaviour change on the default sequential path: a
502/503/504now re-queues instead offailing the task — bounded by
task_publish_max_retries(default0, i.e. inert unless retriesare already enabled). The existing
429/500/ quota / conflict handling is unchanged.