Repository navigation
Converge the runner teardown and a needrestart deferral onto runner hosts - #11063
Conversation
…osts needrestart restarts every actions-runner@ unit after an unattended libc upgrade; with KillMode=process the restart kills only the listener, the in-flight job's worker survives, and a new listener takes a new job into the same _work directory -- the vanished-toolchain CI reds. The teardown intent was modeled (gunbc.runner_unit) but never delivered. gunbc.runner_host_file_converge writes a teardown drop-in rendered from that intent and a cited needrestart override_rc deferral, over the fleet SSH edge as the declared administrator with sudo -n, reloads only when the drop-in changed, and reads KillMode/ExitType back from a live slot. Two fleet-converge modes: runner_host_file_observe (no writes) and runner_host_file_converge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vj6JFxcVFjT9KxFmJ6oM7D
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88947aa03b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match typed_remote_file_step( | ||
| target: subject.target, | ||
| context: context, | ||
| raws: typed_remote_file_compare_argv(write: w), | ||
| stdin_payload: w.content, |
There was a problem hiding this comment.
When either existing config already has the desired bytes but is owned by the runner account or is group/world-writable, this content-only cmp classifies it as HostFileIdentical, so apply skips the privileged chmod/write and can report success while leaving CI jobs able to modify a file later evaluated by needrestart or systemd as root. The observation needs to verify the expected root ownership and mode as well as content, treating metadata drift as requiring convergence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in dd66120 (current head 927e42a). observe_runner_host_file now reads ownership as well as content: the file, and every ancestor directory up to /etc, must be root:root with no group/other write bit. The check is evaluated far-side by find … -maxdepth 0 -type f|d -user root -group root -not -perm /022, which prints only the safe set.
- Identical bytes with unsafe file metadata now plan a write, and the root-owned staged file is renamed over the old one (
runner_host_file_action). - An unsafe ancestor refuses, since rewriting the file can't repair it.
- Metadata is read back after every write (
apply_runner_host_file, stepmetadata-readback).
Witnesses identical_bytes_with_unsafe_file_metadata_are_rewritten_not_left and an_ancestor_that_is_not_root_only_refuses_every_action both ran planned-and-passed as changed witnesses in floor run 34626935244 (FloorClean, 3722/3722).
…nsafe ownership Review on #11063 found three holes. The reload was owed only by a write in the same invocation, so a run after a failed reload saw identical bytes and never reloaded; it is now the manager's own NeedDaemonReload. The teardown was read from the first desired slot only; every desired and every loaded actions-runner@ unit is now read in one systemctl show, LoadState first (an unloaded unit reports the default KillMode=control-group). And identical bytes were left in place whatever their ownership; the file and each ancestor up to /etc must now be root:root with no group/other write, checked far-side by find, rewritten when the file is wrong, refused when an ancestor is, and read back after every write. Also: Id/KillMode/ExitType join extdeps.systemd's property vocabulary, the sudo prefix comes from extdeps.sudo.elevation, and the property/directive classifier gains its missing SliceProperty arm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vj6JFxcVFjT9KxFmJ6oM7D
# Conflicts: # dag/test/claim/typed_remote_file_write_fleet_ssh_fixture.dag
…tep leg Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vj6JFxcVFjT9KxFmJ6oM7D
# Conflicts: # .github/workflows/fleet-converge.yml # dag/gunbc/fleet/fleet_converge_workflow.dag
…ence chain, not the deleted classify_host_file_presence gunbc.microvm_controller_app_key_converge imported and called classify_host_file_presence from gunbc.runner_host_file_converge. The function was real (#11063) and this consumer bound it in #11679; #11845 deleted it because its exit-1 arm minted HostFileAbsent for every stat failure (GNU `test -e` is stat(path) == 0), and touched only the sibling module, so this consumer went dangling. Restoring the name would restore the false-absence arm on the one file the module keeps root:root 0400. observe_key_content now runs the sibling's stat `%F` probe over this module's elevated leg and consumes classify_host_file_metadata, classify_host_file_path and runner_host_file_settled_standing; the parent listing that decides absence when the read failed runs elevated (the same stated departure from member_observe's entry_presence that clear_staging already carries, because the parent is root:root 0700), with the argv and membership read still member_observe's. Evidence: per-entry compile of the module (primary-precedence pool) on #11941's head with the old file reports exactly the three errors (name not found in module at the import, function not found in scope at the call, effect summary incomplete); with this file it reports none, both runs resolving the same 1563-source closure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
What this fixes
Most of the "vanished toolchain" CI reds come from one mechanism: two jobs running in the same runner slot. Examples:
librustc_driver-*.so: cannot open shared object file,couldn't read …/_temp/cargo/registry/src/…,could not rename 'component' file, and rustcNo such file or directory (os error 2).What happens, from srv1's journal and
/var/log/unattended-upgrades/unattended-upgrades-dpkg.logon 2026-09-11:systemctl restart actions-runner@srv1-01.service … actions-runner@srv1-NN.service.KillMode=process, so the restart kills only the listener. systemd logsUnit process … (Runner.Worker) / (claim_executor) / (gunbc) remains running after unit stopped, thenFound left-over process … Ignoring._work/_temp. The two jobs then delete each other's toolchain and checkout, and they share one slot's memory ceiling.srv1's dpkg log shows needrestart restarting live runner slots on 10 separate days since June. This also explains a puzzle recorded in
gunbc.runner_unit("four detached generations at NRestarts=0"):NRestartsnever counts an explicitsystemctl restart.The teardown fix was already modeled (
gunbc.runner_unitgunbc_runner_lifecycle_intent:ExitType=cgroup+KillMode=control-group), but nothing delivered it to a host.What this adds
extdeps.needrestart(new, cited to upstream v3.6 and Ubuntu noble package3.6-7ubuntu4.5): a typedoverride_rcentry, keyed by a unit-name prefix escaped the way Perl'squotemetadoes.gunbc.runner_unit_file:70-fleet-teardown.conf, a drop-in rendered from the samerunner_unit_teardown_directivesthe full unit uses. It's a drop-in rather than the whole unit because the live unit depends on a hand-placeddepriv.conf(User=root,CTRL_RUNNER_JOB_USER, sccache, docker env) that nothing models yet. Delivering the full unit is its own cut.gunbc.typed_remote_file_write: aRemoteFilePrivilegeoption, so the stage/chmod/rename/cmpsteps can run undersudo -n. The Spark caller keeps session-principal behavior.gunbc.runner_host_file_converge(new): converges both files over the fleet SSH edge as the declared administrator (executor_bootstrap_principal).sudo -nfirst.test -eplus the same far-sidecmpleg the write's read-back uses.daemon-reloadonly when the drop-in changed.systemctl show -p KillMode/-p ExitTypeback from a live slot unit.runner_host_file_observe(no writes) andrunner_host_file_converge, plus a receipt upload.Why the CI job user gets no grant. systemd merges the drop-in and needrestart
evals the snippet as Perl, both as root. A sudo grant lettingghrunnerinstall either would let any CI job, PR code included, stage content and become root.Evidence
gunbc compileof the new module's closure: 0 blocking.test.claim.runner_host_file_converge_witness_test: 18/18 PASS underclaim_batch --hermetic.KillMode=process/ExitType=mainpair renders differently and is refused bylifecycle_intent_cannot_orphan.process/main, and each half alone, classify NOT-EFFECTIVE.typed_remote_file_converge_witness_test15/15,typed_remote_file_write_witness_test12/12,runner_unit_file_witness_test21/21,executor_privileged_operation_witness_test14/14.eval … dieloading and itsoverride_rclookup loop.actions-runner@srvN-NN.servicegives restart=0;docker.servicestill gives 1;xactions-runner@…doesn't match; the distro'sdbusentry is preserved.workflow_capability_closure_witness_test8/8,toolchain_home_standing_witness_test10/10,workflow_dispatch_input_witness_test17/17. For the last one, the mode-count literal moved from 16 to 18, as its annotation requires when variants are added.phases_run=2 phases_failed=0, and the generated-artifact registry shows 40/40 matches with no drift.namespace-wave-admissionADMITTED (3 modules added, 0 deltas).typed_remote_file_converge_witness_test, which calledstring_list_containswithout importing it. That module is only strictly prepared when it changes. The import is fixed and the module passes 15/15.Review repairs (dd66120, merged onto main at 927e42a)
NeedDaemonReload, read after the writes, not from whether this invocation wrote anything. A run that finds identical bytes after an earlier reload failed now reloads.systemctl show --property=Id,LoadState,NeedDaemonReload,KillMode,ExitTypereads every desired slot unit plus every loadedactions-runner@unit (fromlist-units --all).LoadStateis checked first, because an unloaded unit reports the defaultKillMode=control-group. Each block is joined by itsId, and a unit with no block is UNREAD. The host is effective only if every unit is./etcmust beroot:rootwith no group/other write, checked on the host byfind. Identical bytes with unsafe file metadata are rewritten; an unsafe ancestor refuses; metadata is read back after every write.Id/KillMode/ExitTypejoinextdeps.systemd's property vocabulary. The sudo prefix comes fromextdeps.sudo.elevation. The property/directive classifier gains itsSlicePropertyarm, missing on main.CI evidence: witnesses run 34626935244 is green. Floor verdict
FloorClean, 3722/3722 executed, 0 failed. All 29runner_host_file_converge_witness_testwitnesses were planned as changed witnesses and passed.namespace-wave-admissionADMITTED the diff (3 modules added, 5 deltas, all auto-admitted).Measured on srv2 (read-only observe, pre-repair code, run 34617430531): the fleet key reaches
ubuntuandsudo -nis available; both files are absent;actions-runner@srv2-01hasKillMode=process ExitType=mainloaded.Not done here
No host has been changed. After merge the rollout is, one host at a time, srv2 first:
runner_host_file_observe.runner_host_file_converge.ExitType=cgroupholds a slot until its cgroup empties.executor_bootstrap_principal(ubuntuvia the fleet key) is measured only on srv3. The observe run is the measurement for the other hosts, and it refuses by name if the principal can'tsudo -n.🤖 Generated with Claude Code
https://claude.ai/code/session_01Vj6JFxcVFjT9KxFmJ6oM7D