fix(orch): choose the chrony time source at boot instead of at build time - #3440
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit 9fd0b83. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
No bugs found. This is a solid, well-tested fix (per-distro real-KVM verification, plus new tests asserting the old build-time probe is gone and the new selector is wired in for both systemd and OpenRC), but it changes boot-time chrony startup ordering across every supported guest init system, which is boot-critical for all sandboxes. Given that scope, I'd like a human to also look this over before it merges.
Extended reasoning...
Overview
The PR removes a build-time /dev/ptp0 probe from provision.sh and replaces it with a boot-time selector (e2b-chrony-source) that writes the chrony source line into /run/chrony-e2b/source.conf, included by chrony.conf. It adds a systemd oneshot unit + drop-in wiring (Requires=/After= on the family's timesync unit) and an OpenRC boot-runlevel service for Alpine. Tests were added/extended in rootfs_test.go, distro_test.go, and a new provision_test.go asserting both that the old probe is gone and that the new wiring exists per init family.
Security risks
None identified — no new external input, no privilege changes, no network-facing surface. The chrony source file lives on tmpfs and is written by root during boot, same trust level as the rest of provisioning.
Level of scrutiny
This touches the guest boot sequence baked into every template's rootfs (systemd unit ordering and OpenRC runlevel wiring), which is boot-critical: a wiring mistake could leave chronyd dead on cold boot across an entire distro family. The PR's own description frames it as fixing exactly that kind of bug in a prior PR. Even though the change is narrow and unusually well-tested (real KVM verification per family, targeted regression tests), the blast radius (all cold-booting sandboxes, all supported distros) puts it above the bar for full bot approval.
Other factors
No bugs were found by the automated bug hunt, and my own read of the systemd ordering (Requires+After on the dynamically-named timesync unit, avoiding a static enablement symlink that RHEL's preset policy would delete) and the OpenRC boot-runlevel placement (which runs before the default runlevel where chronyd is enabled) looks correct. The description also notes a required build-provision-version LaunchDarkly bump for prod rollout, which is an operational step outside this diff.
86823ef to
8445455
Compare
…time provision.sh probed /dev/ptp0 while provisioning and baked either the PHC refclock or the NTP pool into chrony.conf. Template builds and sandboxes run in separate node pools, so a template built where the PHC exists can cold-boot where it doesn't, and a refclock line for a missing PHC is fatal to chronyd. chrony.conf now includes /run/chrony-e2b/source.conf, written on every boot by e2b-chrony-source: a systemd oneshot pulled into the family's chrony unit with a Requires= drop-in, and an OpenRC boot service on Alpine. Mirrors what the NixOS base image already does.
8445455 to
8fdd81c
Compare
🤖 I have created a release *beep* *boop* --- ## [0.0.2](orchestrator-v0.0.1...orchestrator-v0.0.2) (2026-07-31) ### Bug Fixes * **orch:** choose the chrony time source at boot instead of at build time ([#3440](#3440)) ([10b1bae](10b1bae)) * **orch:** pin nixpkgs to a supported release for the NixOS base image ([#3478](#3478)) ([6e8e9d3](6e8e9d3)) * **template-build:** create trailing-slash COPY targets before moving files ([#3458](#3458)) ([dc53fa7](dc53fa7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: e2b-release-please[bot] <298072688+e2b-release-please[bot]@users.noreply.github.com>
The three initSetup bodies leave their Go string literals for
init-{systemd,openrc,nixos}.sh, go:embed'd with the trailing newline
trimmed so they splice exactly like the literals did. Selector output is
unchanged apart from the entry comments moving in as # lines.
Rebased over #3440, which added chrony-source wiring to both init blocks
after this was written: the systemd drop-in, OpenRC's e2b-chrony-source
install and the unconditional command_args="-F 0" move into the new
files verbatim, checked byte-for-byte against main's literals.
First half of the #3411 follow-up (dobrac: init.go:21).
…plate
The selection structure (case arms, rejected-id guard, ID_LIKE loop,
error/warning text) moves verbatim from the ShellSelector string-builder
into provision.sh as inline template actions; the distro package now
contributes data only — distro.NewTemplateData(), with all shell-quoting
kept in Go. Fingerprint hashes the view data (%#v, so new fields are
covered automatically); the structure is already hashed via the raw
embedded template. Version 1 -> 2.
The render seam and its tests live in ungated files so they keep running
on darwin. Selection-text assertions move to base/provision_test.go
against the rendered script, with the guard-ordering anchor rewritten to
the fallback loop line (E2B_ID_LIKE is assigned earlier in the full
script). New guards: %q-vs-sh quoting assumptions pinned per profile
field, a leftover-"{{" render check, and a test that every ProfileView
field — including the init-setup body now sourced from a file — reaches
the rendered case arm. A dropped template action would otherwise leave
the Go-side tests green while the guest hits an undefined variable under
set -u.
#3440's chrony-deferral test now asserts on the rendered script rather
than the raw template: the init blocks moved out, so only the rendered
form covers both halves.
Rendered output verified byte-identical to main's ShellSelector output
modulo the added comments; customer-visible messages pinned byte-exact.
Completes the #3411 follow-up (dobrac: distro.go:160, provision.sh:29).
…time (#3440) `provision.sh` probed `/dev/ptp0` at build time and baked either the PHC refclock or the NTP pool into `chrony.conf`, assuming provisioning and runtime share a host — build and sandbox nodes are separate pools, so that probe doesn't belong in a durable template artifact (codex P1 on #3411). The source line is now written on every boot by `e2b-chrony-source` (systemd oneshot; OpenRC boot service on Alpine) and pulled in with an `include`, mirroring what the NixOS base image already does. Rebased over #3453, whose seccomp workaround keys off exactly the build-time PHC verdict this PR deletes — left alone it never fires, and under `set -eu` the now-unset var breaks every distro's build. Alpine's `-F 0` is therefore unconditional in the OpenRC init setup, which is free: on the pool line chronyd runs fine under the filter, while against a real PHC (an `igc` NIC clock) `-F 1` gives "Loaded seccomp filter (level 1)" then "Bad system call", and `-F 1 -F 0` selects PHC0 at ±55ns. Alpine and ubuntu re-verified on real KVM — 7/7 each, plus 7/7 seccomp checks. Cold boots only: memory-resumed sandboxes keep their snapshot's chronyd. Rollout needs a `build-provision-version` bump.
🤖 I have created a release *beep* *boop* --- ## [0.0.2](orchestrator-v0.0.1...orchestrator-v0.0.2) (2026-07-31) ### Bug Fixes * **orch:** choose the chrony time source at boot instead of at build time ([#3440](#3440)) ([7fa567c](7fa567c)) * **orch:** pin nixpkgs to a supported release for the NixOS base image ([#3478](#3478)) ([bcdb4aa](bcdb4aa)) * **template-build:** create trailing-slash COPY targets before moving files ([#3458](#3458)) ([a19b99b](a19b99b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: e2b-release-please[bot] <298072688+e2b-release-please[bot]@users.noreply.github.com>
The three initSetup bodies leave their Go string literals for
init-{systemd,openrc,nixos}.sh, go:embed'd with the trailing newline
trimmed so they splice exactly like the literals did. Selector output is
unchanged apart from the entry comments moving in as # lines.
Rebased over #3440, which added chrony-source wiring to both init blocks
after this was written: the systemd drop-in, OpenRC's e2b-chrony-source
install and the unconditional command_args="-F 0" move into the new
files verbatim, checked byte-for-byte against main's literals.
First half of the #3411 follow-up (dobrac: init.go:21).
…plate
The selection structure (case arms, rejected-id guard, ID_LIKE loop,
error/warning text) moves verbatim from the ShellSelector string-builder
into provision.sh as inline template actions; the distro package now
contributes data only — distro.NewTemplateData(), with all shell-quoting
kept in Go. Fingerprint hashes the view data (%#v, so new fields are
covered automatically); the structure is already hashed via the raw
embedded template. Version 1 -> 2.
The render seam and its tests live in ungated files so they keep running
on darwin. Selection-text assertions move to base/provision_test.go
against the rendered script, with the guard-ordering anchor rewritten to
the fallback loop line (E2B_ID_LIKE is assigned earlier in the full
script). New guards: %q-vs-sh quoting assumptions pinned per profile
field, a leftover-"{{" render check, and a test that every ProfileView
field — including the init-setup body now sourced from a file — reaches
the rendered case arm. A dropped template action would otherwise leave
the Go-side tests green while the guest hits an undefined variable under
set -u.
#3440's chrony-deferral test now asserts on the rendered script rather
than the raw template: the init blocks moved out, so only the rendered
form covers both halves.
Rendered output verified byte-identical to main's ShellSelector output
modulo the added comments; customer-visible messages pinned byte-exact.
Completes the #3411 follow-up (dobrac: distro.go:160, provision.sh:29).
provision.shprobed/dev/ptp0at build time and baked either the PHC refclock or the NTP pool intochrony.conf, assuming provisioning and runtime share a host — build and sandbox nodes are separate pools, so that probe doesn't belong in a durable template artifact (codex P1 on #3411). The source line is now written on every boot bye2b-chrony-source(systemd oneshot; OpenRC boot service on Alpine) and pulled in with aninclude, mirroring what the NixOS base image already does.Rebased over #3453, whose seccomp workaround keys off exactly the build-time PHC verdict this PR deletes — left alone it never fires, and under
set -euthe now-unset var breaks every distro's build. Alpine's-F 0is therefore unconditional in the OpenRC init setup, which is free: on the pool line chronyd runs fine under the filter, while against a real PHC (anigcNIC clock)-F 1gives "Loaded seccomp filter (level 1)" then "Bad system call", and-F 1 -F 0selects PHC0 at ±55ns. Alpine and ubuntu re-verified on real KVM — 7/7 each, plus 7/7 seccomp checks. Cold boots only: memory-resumed sandboxes keep their snapshot's chronyd. Rollout needs abuild-provision-versionbump.