Self-hosted worker: don't fail tasks on transient Kubernetes re-watch and FailedMount errors - #124
Conversation
Two cluster-side blips were turning into failed self-hosted worker tasks:
- A closed or errored Job/Pod watch was re-established exactly once, and a
single failing call (e.g. a momentary "connection refused" to the API
server) failed the whole task. Watches are an optimization, not a
correctness requirement - the 30s safety poll independently observes Job
and pod state - so a failed reopen now detaches the watch and reconnects
with bounded backoff while the poll carries the task.
- Any FailedMount event failed the task on first sight, including kubelet
conditions that Kubernetes retries on its own ("failed to sync configmap
cache: timed out waiting for the condition"). FailedMount now gets the same
bounded grace window that unschedulable pods already have, configurable via
backend.kubernetes.volume_mount_timeout (default 2m, 0s disables). Callers
that cannot wait - the startup preflight and terminally failed Jobs - still
surface the event immediately.
Co-Authored-By: Warp Agent <agent@warp.dev>
There was a problem hiding this comment.
Overview
Makes the self-hosted worker tolerate transient Kubernetes watch and mount failures instead of failing the task on first sight (REMOTE-2701). The mechanical findings are going back to the author; these two are judgment calls that need a human.
Concerns
- The preflight still fails fast on a mount error (
internal/worker/kubernetes.go:940-946). The transient in the report isistiod-ca-certfailing to mount — an Istio-injected volume that lands on every pod in the namespace, including the preflight pod — so withfailFastOnMountFailuresthere, the same blip makesNewKubernetesBackendreturn an error and the worker refuse to start, moving the amplification this issue is about to startup. The preflight already runs under a 15s deadline that bounds the wait, so honoring the grace window and folding the event into the timeout message would keep the cluster-incompatibility diagnostic while surviving the blip — but that trades a crisp startup error for a slower one, so it is your call. volume_mount_timeout: "0s"disables mount failure detection entirely. WithbackoffLimit: 0, a pod that can never mount then hangs until the Job'sactiveDeadlineSeconds(chart default 8h), or indefinitely if that is unset. Worth deciding whether operators should be able to turn this off at all, or whether the floor should be a long timeout rather than "never".
Verdict
Checks: build pass, tests pass, CI green, visual proof n/a
Found: 0 critical, 0 important, 0 suggestions, 0 nits, 2 questions
… reopen Follow-up to the review of the transient-Kubernetes-error handling. - The safety poll now tolerates the same outage the watch does. It was returning executeError on any error from Jobs().Get() or listTaskPods(), so a "connection refused" that the re-watch survived still failed the task at the next 30s tick - the task only lived if the outage fell between ticks. Consecutive poll failures are now terminal only after safetyPollFailureTolerance (3m); a missing Job and a cancelled context still fail immediately. - The FailedMount grace window is measured across the mount-failure events instead of from pod creation. Pod age was wrong in both directions: it was already spent by the time a slow-to-schedule pod first attempted a mount, and it failed pods on a stale event the kubelet had long since resolved. The failures must now have started at least VolumeMountTimeout ago and still be current; a pod that stopped reporting them is treated as recovered. - watchStream rate-limits reconnects to one attempt per watchReopenInterval whatever the outcome. Backing off only on error left the success-then- immediate-close path spinning the loop against a struggling API server. - Reconnects are driven by a dedicated 1s ticker rather than the 30s safety ticker, so the exponential backoff actually gates something and a brief blip costs about a second instead of a full poll interval. - open() runs under a watchdog that cancels only while the call is in flight, so a blackholed API server cannot stall the task loop on the dialer. The established watch keeps its own context, which client-go needs for the lifetime of the watch. - The Helm chart omits volume_mount_timeout when unset rather than rendering an empty value the worker rejects at startup. Tests: the task now completes through the safety poll alone with both watches down and the first poll failing; reopen attempts are shown to be rate-limited; and the mount window is exercised through event timestamps, including a failure the kubelet stopped reporting. Co-Authored-By: Warp Agent <agent@warp.dev>
|
Revised in af00ab3. The review's two questions are deliberately untouched and left for the requester: the preflight Everything else raised in review is addressed:
New tests for the two gaps that were untested: CI is green. |
Fixes REMOTE-2701.
Symptom
Three self-hosted worker task failures in one aggregated error report, all cluster-side and transient:
Cause
In
internal/worker/kubernetes.go:watch.Error, the loop re-established it exactly once and failed the whole task if that single call errored. The 30s safety poll that was supposed to be the fallback was no better: it returned a task failure on any error fromJobs().Get()orlistTaskPods(). So a momentaryconnection refusedkilled a running task through whichever path hit it first.FailedMountevent (or any message containingMountVolume.SetUp failed) failed the task immediately, including kubelet conditions that Kubernetes retries on its own.What changed
Watches survive a blip. A
watchStreamhelper owns each watch and its reconnect policy. A failed reopen detaches the stream —resultChan()returns a nil channel, which is never ready in theselect— instead of failing the task, and reconnects from a dedicated 1s ticker with exponential backoff (1s, doubling, capped at 30s, reset on success). Reconnect attempts are rate-limited to one per second whatever the outcome, so a watch that is re-established and then immediately closed can no longer spin the loop against a struggling API server.openruns under a watchdog that cancels only while the call is in flight, so a blackholed API server cannot stall the task loop on the dialer; the established watch keeps its own context, which client-go needs for the watch's lifetime. The initial watch failure at task start still fails the task, as before.The safety poll survives the same blip. Consecutive poll failures are tolerated for 3 minutes before the task fails, so the poll can actually carry a task while the watches are down — which is what the whole design rests on. A Job that is genuinely gone (
IsNotFound) and a cancelled context still fail immediately, and a single successful poll clears the streak.FailedMount gets a bounded grace window, aged by the events themselves. New
backend.kubernetes.volume_mount_timeout(default2m,0sdisables), wired through the config file,mergeConfig, the Helm chart, and the README. The window is measured across the mount-failure events rather than from pod creation: a mount is only attempted once the pod is scheduled and its images are available, so pod age would already be spent before the first attempt on a slow-to-schedule pod, and would also fail pods on an event the kubelet resolved long ago. The failures must have started at leastvolume_mount_timeoutago and must still be current; a pod that has stopped reporting them is treated as recovered.Two callers explicitly opt out of the grace window, because they have nothing left to wait for and the event is the most useful diagnostic: the startup preflight (which runs under its own 15s timeout and exists to surface cluster incompatibilities) and pod inspection for a Job that has already failed terminally. This is a named
volumeMountGraceargument rather than an implicit rule.Nothing else from the error report is touched; the other rows are handled separately.
Validation
gofmt -s -l .,go build ./...,go vet ./...,golangci-lint run— all cleango test ./...— all packages pass;go test ./internal/worker -racealso passesNew tests, each of which fails without its corresponding fix:
TestExecuteTaskCompletesThroughSafetyPollWhileWatchesAreDown— both watches are dropped and never come back, the first poll fails withconnection refused, and the task still completes off the next poll. With the poll tolerance removed this fails withfailed to get Job ... connect: connection refused, which is the original incident.TestExecuteTaskSurvivesFailedPodRewatch— a pod watch that closes and cannot be re-established no longer fails the task.TestWatchStreamRateLimitsReopenAttempts— 1000 back-to-back reopens produce a single attempt.TestWatchStreamDetachesAndReconnectsAfterFailedReopen,TestNextWatchReopenBackoffIsBounded,TestSafetyPollFailuresAreOnlyTerminalOncePersistent— reconnect and tolerance policy in isolation.TestInspectPodFailureRespectsVolumeMountTimeout— a just-started mount failure on an old pod is tolerated, a persistent one fails within the bounded window, a failure the kubelet stopped reporting is ignored,0sdisables it, and the fail-fast callers still fail on the first event.volume_mount_timeoutparsing and for an invalid value.The existing
TestRunStartupPreflightFailsOnFailedMountEventstill passes unchanged, so preflight diagnostics are unaffected.Conversation: https://staging.warp.dev/conversation/09c693c8-299b-40e3-a08e-529eb6a75d8e
Run: https://oz.staging.warp.dev/runs/019ff348-9c2a-77a4-a7a7-3b6e28b633ab
This PR was generated with Oz.