feat(arc): taint a stopped node out of service so its pods are replaced - #18
Merged
Merged
Conversation
When a node's kubelet stops reporting, its pods are marked for deletion but stay Terminating, since only that kubelet can confirm the deletion. ARC waits for a listener's old pod to go before starting a new one, so a listener on a stopped node stalled its scale set until someone force-deleted the pod. A small watcher Deployment now gives a node the node.kubernetes.io/out-of-service=nodeshutdown:NoExecute taint once its Ready condition has been Unknown for github_runner_arc_node_recovery_after_seconds (120 by default). The control plane then evicts the node's pods and force-deletes the terminating ones. The watcher removes the taint when the node reports Ready again, only touches taints it applied, never acts on its own node or on a node reporting NotReady, and needs only get, list and patch on nodes. Turning it off removes it and any taint it left. The watcher gets its own image, built and stamped by the release like the others. Closes #2
A single-replica StatefulSet stands in for an ARC listener, since its controller also waits for the old pod to go. With the watcher off, its pod on the stopped worker stays Terminating; with the watcher on it is replaced on a healthy node within the grace period plus the threshold, and the taint is removed once the worker is back.
Mearman
marked this pull request as ready for review
September 26, 2026 06:46
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
🎉 This PR is included in version 1.3.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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
1 participant
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.
Closes #2
When a node stops, its pods get marked for deletion but stay Terminating because only the node's kubelet can confirm it. Deployments replace them anyway, which is why the controller came back on its own, but ARC won't start a listener until the old listener pod is gone, so a listener on the dead node stalls its scale set until someone force-deletes it.
This adds a small watcher Deployment to the ARC role (
node-recovery, its own image) that uses Kubernetes' non-graceful node shutdown. Once a node's Ready condition has been Unknown forgithub_runner_arc_node_recovery_after_seconds(default 120), it addsnode.kubernetes.io/out-of-service=nodeshutdown:NoExecute. The control plane then evicts everything on the node and podgc force-deletes the pods already terminating there. When the node reports Ready again the watcher takes the taint off.github_runner_arc_node_recovery_enabled: falseremoves the watcher and any taint it left.A few things it deliberately doesn't do. It only acts on Unknown, not on NotReady (False): in that case the kubelet is alive and finishes the deletions itself. It never taints the node it's running on. It only removes taints carrying its own annotation, so an operator's own out-of-service taint is left alone. Each patch tests the node's resourceVersion first, so a node that came back between the read and the write is left for the next poll. RBAC is get/list/patch on nodes, with no delete, so it can't remove a Node object (which on a k3s server would drop its etcd member); the taint itself doesn't touch etcd, and nothing in the cluster role reacts to it. The watcher's own pod tolerates unreachable/not-ready for only 10s, so if it was on the node that died, a replacement starts well before the threshold.
On the 120s default: the node controller only marks a node Unknown after 50s of missed heartbeats (node-monitor-grace-period on current Kubernetes), and the extra 120s leaves room for a k3s container restart or upgrade to come back before anything is given up. Worst case, a stopped node now stalls a listener for about four minutes (grace + threshold + a 15s poll + podgc's 20s sweep + the listener's own restart, which the issue comment puts at under a minute) instead of indefinitely. The trade-off is that runner pods on a node gone longer than that are given up before the five-minute default eviction would do it, but their containers stopped with the node, so those jobs are lost either way.
Shorter tolerations on the controller, listener and platform pods on their own don't help: they only get the pod marked for deletion sooner, and it stays Terminating just the same. The taint evicts regardless of tolerations, so it covers that option too.
Testing:
tests/node_recovery/run.sh(newNode recoveryworkflow) builds the image, renders the role's manifests into a three-node kind cluster and stops a worker container. A single-replica StatefulSet stands in for the listener, since its controller also waits for the old pod to go. With the watcher scaled to zero, the pod stays Terminating and isn't replaced; with the watcher on, it's replaced on a healthy node and the taint comes off once the worker is started again. Then, with default tolerations, it stops the worker again and checks the replacement arrives within grace + threshold + margin and that the taint wasn't applied before the threshold. Unit tests cover the decision logic inscripts/node-recovery.shagainst a fake kubectl.After the first release,
ghcr.io/exadev/github-runner-node-recoverywill need making public, like the pull-secret renewer, since the watcher pulls without a pull Secret.