Skip to content

fix(spur-k8s): fail fast on a controller that does not answer - #854

Open
pre wants to merge 1 commit into
ROCm:mainfrom
silogen:pr/k8s-operator-registration
Open

fix(spur-k8s): fail fast on a controller that does not answer#854
pre wants to merge 1 commit into
ROCm:mainfrom
silogen:pr/k8s-operator-registration

Conversation

@pre

@pre pre commented Sep 8, 2026

Copy link
Copy Markdown

What this fixes

The operator's channel to spurctld had no bound of any kind, and two things stalled it for minutes on a real cluster.

The first registration of the nodes came three minutes late. On a new cluster the operator starts before the controller is ready, when the client Service has no endpoint. The SYN is dropped and the connect hangs for the kernel's SYN retry time, more than two minutes, before it fails with a transport error. Only then does the task's retry loop open a new connection, which succeeds at once, and the registration follows within milliseconds. Every SpurJob submitted in that window waited for a node that did not exist. From the operator log:

10:40:53 starting K8s node watcher
10:41:07 (spurctld-0 ready)
10:43:06 task failed, retrying name=node watcher error=transport error delay_secs=1
10:43:07 starting K8s node watcher
10:43:07 registering K8s node ...

A killed controller Pod stalled the job controller for a minute or two. The job controller keeps one channel open. When the Pod behind it was killed, the packets to that peer were dropped and every RPC on the channel waited for the kernel to give up on it. A SpurJob created meanwhile was submitted only when the connection finally failed, 70 to 110 s after the kill. This is the failure of test_new_leader_accepts_writes after a leader kill:

11:36:05 (leader Pod killed; the job controller's channel led to it)
11:37:13 heartbeat failed: Timeout expired
11:37:44 failed to report job status: Timeout expired
11:37:54 heartbeat failed: tcp connect error   <- the dead connection is finally gone
11:37:54 SpurJob submitted it-pre-failover      <- 64 s after it was created

Approach

One helper, controller::connect, opens the channel with a 10 s connect timeout, HTTP/2 keepalive pings, and a 30 s bound on each request. No RPC on this channel streams. The node watcher, the job controller, the heartbeat sender and the readiness probe all use it; each had its own copy of the URL prefixing and the connect.

The request bound alone was not enough: it cancels the request but leaves the dead connection in place, and the next request waits on it again, 30 s at a time. The job controller's long-lived client is now a small wrapper, ControllerClient, that drops its channel after a transport error (UNAVAILABLE, UNKNOWN, CANCELLED, DEADLINE_EXCEEDED) and connects again on the next call. A refusal such as NOT_FOUND is an answer and keeps the channel. The heartbeat sender already connects per tick, and the node watcher restarts through its retry loop.

The node watcher also ends on a refused registration instead of logging it. It stored the node's resource fingerprint before the call, so a node whose first registration failed was not tried again until its resources changed or the watcher restarted for another reason. Returning the error restarts the watcher through the existing retry loop, which lists every node again; a repeat registration is accepted. The fingerprint is stored only after the registration succeeds.

Testing

cargo clippy --workspace --exclude spur-ffi --all-targets --locked and cargo test --locked are clean. Unit tests cover the URL prefixing, the transport-error classification, and that the wrapper drops its channel after a transport error and keeps it after a refusal. One more test opens a channel to a socket that accepts the connection and never answers, which is what a killed Pod looks like until the kernel gives up, and runs the call on tokio's paused clock so the real bounds apply without waiting: the call fails with UNAVAILABLE: http2 error, the keepalive having closed the connection before the request bound, and the channel is dropped. The connect timeout has no unit seam, because loopback cannot drop a SYN; it is verified on a cluster.

On a three-node k0s cluster built by spur k8s up, with an image carrying this change:

  • The operator registered the nodes 9 s after the controller became ready, with the connect failing every 10 s until then instead of once after 133 s.

Cluster runs with test_raft_ha.py

Three-node k0s cluster built by spur k8s up, one control plane and two workers, the e2e harness of #853, runs back to back on the same cluster. The operator build names which part of this change the image carried; the controller image was the same throughout.

Run Operator build Passed Failed test Cause
1 connect timeout only 5 of 6 test_new_leader_accepts_writes job controller's channel pinned to the killed Pod; RPCs waited for the kernel
2 connect timeout only 6 of 6
3 + keepalive, request bound 5 of 6 test_new_leader_accepts_writes channel pinned to the killed Pod; each RPC hit the 30 s bound and the next one reused the dead connection
4 + keepalive, request bound 4 of 6 test_state_survives_leader_failover, test_new_leader_accepts_writes harness: the previous session's CRD was still terminating, 404 page not found (fixed in #853)
5 + keepalive, request bound 5 of 6 test_new_leader_accepts_writes channel pinned to the killed Pod, as in run 3
6 + channel drop after a transport error 4 of 6 same two as run 4 harness, CRD terminating
7 + channel drop after a transport error 6 of 6 log shows the stalled report call time out, the channel dropped, and the waiting SpurJob submitted at once
8 + channel drop after a transport error 4 of 6 same two as run 4 harness, CRD terminating
9 + channel drop after a transport error 4 of 6 test_state_survives_leader_failover, test_new_leader_accepts_writes after the previous test's Pod kill a follower answered cannot reach leader during the election, and the job controller's error backoff had doubled to 32 s; not covered by this change, see below
10 + channel drop after a transport error 0 of 6 all, in setup harness: the CRD stayed terminating for over 120 s because of leftover objects on the shared cluster

Two focused runs of test_state_survives_leader_failover and test_new_leader_accepts_writes alone with the keepalive build: one failed on the terminating CRD, one passed 2 of 2.

The dead-channel failure, runs 1, 3 and 5, did not appear again once the channel is dropped after a transport error. The remaining failures are the harness race, fixed in #853, and the backoff after a Pod kill:

One thing this change does not cover, seen in run 9: right after a Pod kill, a follower answers cannot reach leader while the election runs, and the job controller's own error backoff doubles to 32 s and beyond, so a SpurJob created in that moment is submitted after the test's 60 s window. That is a separate matter of the backoff and is left for a follow-up.

Related:

The operator's channel to spurctld had no bound of any kind, so two
things stalled it for minutes. When the operator started before the
controller was ready, the client Service had no endpoint and the SYN
was dropped; the connect hung for the kernel's SYN retry time, more
than two minutes, before the task failed and its retry loop opened a
new connection. The nodes reached the controller about three minutes
after it was ready. When the controller Pod behind the job controller's
open channel was killed, every RPC on that channel waited for the
kernel to give up on the peer, and a SpurJob created in that window was
submitted only a minute or two later.

The four places that opened a channel now share one helper. It sets a
10 s connect timeout, HTTP/2 keepalive pings, and a 30 s bound on each
request. The bound alone does not close the dead connection, and the
next request would wait on it again, so the job controller's long-lived
client drops its channel after a transport error and opens a new one on
the next call.

A test opens a channel to a socket that accepts and never answers, on
tokio's paused clock, and checks that the call fails within the bounds
and that the channel is dropped.

The node watcher also ends on a refused registration instead of
logging it and keeping the node's fingerprint, which made the node wait
for its next event before it was tried again.

Signed-off-by: Petrus Repo <petrus.repo@amd.com>
@pre
pre force-pushed the pr/k8s-operator-registration branch from 52f9f25 to 14ad737 Compare September 8, 2026 12:16
@pre
pre marked this pull request as ready for review September 8, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant