From 8fdd81c9e3a495649488814c6e5c0dadf6b1013b Mon Sep 17 00:00:00 2001 From: Tomas Srnka Date: Wed, 29 Jul 2026 11:30:22 +0200 Subject: [PATCH] fix(orch): choose the chrony time source at boot instead of at build 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. --- .../rootfs/files/chrony-source.openrc.tpl | 27 +++++++ .../rootfs/files/chrony-source.service.tpl | 16 ++++ .../core/rootfs/files/chrony-source.sh.tpl | 24 ++++++ .../template/build/core/rootfs/rootfs_test.go | 17 ++++- .../build/phases/base/distro/distro_test.go | 76 +++++++++++++++++++ .../template/build/phases/base/distro/init.go | 29 +++++++ .../template/build/phases/base/provision.sh | 33 ++------ .../build/phases/base/provision_test.go | 31 ++++++++ 8 files changed, 224 insertions(+), 29 deletions(-) create mode 100644 packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.openrc.tpl create mode 100644 packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.service.tpl create mode 100644 packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.sh.tpl create mode 100644 packages/orchestrator/pkg/template/build/phases/base/provision_test.go diff --git a/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.openrc.tpl b/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.openrc.tpl new file mode 100644 index 0000000000..6aee37d98d --- /dev/null +++ b/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.openrc.tpl @@ -0,0 +1,27 @@ +{{- /*gotype:github.com/e2b-dev/infra/packages/orchestrator/pkg/template/build/core/rootfs.templateModel*/ -}} +{{ .WriteFile "usr/local/share/e2b/chrony-source.openrc" 0o755 }} + +#!/sbin/openrc-run +# OpenRC counterpart of e2b-chrony-source.service (Alpine): runs the shared +# selector in the boot runlevel, before chronyd starts in default. Baked at a +# neutral path and installed into /etc/init.d by the OpenRC e2b_init_setup only +# — on a systemd image the sysv generator would turn an /etc/init.d script of +# this name into a unit that shadows the real one (and Debian's update-rc.d +# aborts on non-LSB scripts, as envd.openrc.tpl explains). + +description="E2B: select the chrony time source (PHC refclock if /dev/ptp0, else the NTP pool)" + +depend() { + before chronyd +} + +start() { + ebegin "Selecting the chrony time source" + /usr/local/bin/e2b-chrony-source + eend $? +} + +stop() { + # Nothing to undo — the source file lives on tmpfs. + return 0 +} diff --git a/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.service.tpl b/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.service.tpl new file mode 100644 index 0000000000..3561f426e1 --- /dev/null +++ b/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.service.tpl @@ -0,0 +1,16 @@ +{{- /*gotype:github.com/e2b-dev/infra/packages/orchestrator/pkg/template/build/core/rootfs.templateModel*/ -}} +{{ .WriteFile "etc/systemd/system/e2b-chrony-source.service" 0o644 }} + +[Unit] +Description=E2B: select the chrony time source (PHC refclock if /dev/ptp0, else the NTP pool) +# The chrony unit is chrony.service on Debian and chronyd.service elsewhere; +# ordering before a unit that doesn't exist on this image is ignored. What pulls +# this in is a Requires= drop-in written for the family's unit name by +# provisioning (distro/init.go) — a drop-in, not an enablement symlink, because +# the RHEL family's preset policy ("disable *") deletes those on first boot. +Before=chrony.service chronyd.service + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=/usr/local/bin/e2b-chrony-source diff --git a/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.sh.tpl b/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.sh.tpl new file mode 100644 index 0000000000..995e5e5004 --- /dev/null +++ b/packages/orchestrator/pkg/template/build/core/rootfs/files/chrony-source.sh.tpl @@ -0,0 +1,24 @@ +{{- /*gotype:github.com/e2b-dev/infra/packages/orchestrator/pkg/template/build/core/rootfs.templateModel*/ -}} +{{ .WriteFile "usr/local/bin/e2b-chrony-source" 0o755 }} + +#!/bin/sh +# Writes the chrony time source the machine we are booting on can actually use. +# The baked /etc/chrony/chrony.conf includes the file this produces. +# +# The hypervisor's PTP clock (kvm-ptp) is the better source — no network, tracks +# the host directly — but it is absent wherever nested virtualization can't +# expose it, and a refclock line for a missing PHC is FATAL to chronyd. Template +# builds and sandboxes run in separate node pools (docs/ARCHITECTURE.md), so the +# device present while provisioning says nothing about the node a cold-booting +# sandbox lands on; only a boot-time decision is right on both. +# +# Shared by e2b-chrony-source.service (systemd) and the OpenRC service of the +# same name (Alpine). +set -eu + +mkdir -p /run/chrony-e2b +if [ -e /dev/ptp0 ]; then + echo "refclock PHC /dev/ptp0 poll 2 dpoll 2" >/run/chrony-e2b/source.conf +else + echo "pool pool.ntp.org iburst maxsources 3" >/run/chrony-e2b/source.conf +fi diff --git a/packages/orchestrator/pkg/template/build/core/rootfs/rootfs_test.go b/packages/orchestrator/pkg/template/build/core/rootfs/rootfs_test.go index 747fcde460..7c82d820d7 100644 --- a/packages/orchestrator/pkg/template/build/core/rootfs/rootfs_test.go +++ b/packages/orchestrator/pkg/template/build/core/rootfs/rootfs_test.go @@ -90,7 +90,7 @@ func TestAdditionalOCILayers(t *testing.T) { keysIter := maps.Keys(actualFiles) keys := slices.Collect(keysIter) - assert.Len(t, keys, 18) + assert.Len(t, keys, 21) // The provisioning boot must be self-contained on the baked busybox: // minimal images (distroless) may have no /bin/sh, and busybox init @@ -115,6 +115,21 @@ func TestAdditionalOCILayers(t *testing.T) { require.NotEmpty(t, seedCerts, "cert seeding script must be baked") assert.Contains(t, actualFiles["etc/systemd/system/envd.service"], "ExecStartPre=/usr/local/bin/e2b-seed-certs") + // Both init families run the same boot-time chrony source selector, and + // the file it writes is the only source chrony.conf gets — a PHC + // refclock baked at build time is fatal to chronyd on a node without + // the device. + chronySource := actualFiles["usr/local/bin/e2b-chrony-source"] + require.NotEmpty(t, chronySource, "chrony source selector must be baked") + assert.Contains(t, chronySource, "/run/chrony-e2b/source.conf") + assert.Contains(t, chronySource, "refclock PHC /dev/ptp0") + assert.Contains(t, chronySource, "pool pool.ntp.org") + assert.Contains(t, actualFiles["etc/systemd/system/e2b-chrony-source.service"], + "ExecStart=/usr/local/bin/e2b-chrony-source") + openrcChronySource := actualFiles["usr/local/share/e2b/chrony-source.openrc"] + require.NotEmpty(t, openrcChronySource, "OpenRC chrony source service must be baked") + assert.Contains(t, openrcChronySource, "before chronyd") + // envd must be preset-enabled: first boot (machine-id is removed by // provisioning) applies the distro preset policy, and the RHEL // family's "disable *" would otherwise delete envd's autostart link. diff --git a/packages/orchestrator/pkg/template/build/phases/base/distro/distro_test.go b/packages/orchestrator/pkg/template/build/phases/base/distro/distro_test.go index 7f7096cb71..d6932261a7 100644 --- a/packages/orchestrator/pkg/template/build/phases/base/distro/distro_test.go +++ b/packages/orchestrator/pkg/template/build/phases/base/distro/distro_test.go @@ -157,6 +157,82 @@ func TestInitSystemsDeclaredAndCoherent(t *testing.T) { } } +// The per-profile fragments are spliced into single-line generated shell +// functions (`e2b_pkg_install() { … ; }`), so they stay one-liners — chain with +// `;` or `&&` instead of embedding newlines. +func TestProfileFragmentsAreSingleLine(t *testing.T) { + t.Parallel() + for _, p := range Profiles { + for name, frag := range map[string]string{ + "PkgQueryBody": p.PkgQueryBody, + "PkgInstall": p.PkgInstall, + "CARefresh": p.CARefresh, + } { + if strings.Contains(frag, "\n") { + t.Errorf("profile %q %s spans lines; chain with ; or && instead: %s", p.Key, name, frag) + } + } + } +} + +// Both init families must wire the boot-time chrony source selector in, each +// with its own mechanism: chrony.conf includes a file only that selector writes, +// so a family that skips the wiring boots with no time source at all. +func TestInitSetupWiresChronySourceSelector(t *testing.T) { + t.Parallel() + systemd := initSetup[InitSystemd] + // A drop-in on the family's own unit name, not a static symlink or a unit + // [Install] section: the name differs per family and the RHEL preset policy + // deletes enablement symlinks on first boot. + for _, want := range []string{ + `/etc/systemd/system/$E2B_TIMESYNC_UNIT.service.d`, + "Requires=e2b-chrony-source.service", + "After=e2b-chrony-source.service", + } { + if !strings.Contains(systemd, want) { + t.Errorf("systemd init setup missing chrony source wiring %q", want) + } + } + openrc := initSetup[InitOpenRC] + for _, want := range []string{ + "cp /usr/local/share/e2b/chrony-source.openrc /etc/init.d/e2b-chrony-source", + "rc-update add e2b-chrony-source boot", + } { + if !strings.Contains(openrc, want) { + t.Errorf("openrc init setup missing chrony source wiring %q", want) + } + } +} + +// Alpine's chrony build takes a SIGSYS under the seccomp filter its OpenRC init +// script hardcodes (-F 1) the moment the PHC refclock is driven. Since +// e2b-chrony-source picks the source at BOOT, provisioning cannot know whether +// the PHC branch will be taken, so the filter must come off unconditionally — +// gating it on a build-time /dev/ptp0 probe (as the original fix did) puts the +// crash back on exactly the nodes that have a PHC. +func TestOpenRCDisablesChronySeccompRegardlessOfSource(t *testing.T) { + t.Parallel() + openrc := initSetup[InitOpenRC] + if !strings.Contains(openrc, `echo 'command_args="-F 0"' >>"/etc/conf.d/$E2B_TIMESYNC_UNIT"`) { + t.Error("openrc init setup must disable the chronyd seccomp filter via conf.d command_args") + } + // The whole block runs on every Alpine build, so no COMMAND in it may branch + // on the device or on a provisioning-time PHC verdict. Comments are stripped + // first — they are where the reasoning lives and legitimately say "refclock". + var code []string + for l := range strings.SplitSeq(openrc, "\n") { + if !strings.HasPrefix(strings.TrimSpace(l), "#") { + code = append(code, l) + } + } + openrcCode := strings.Join(code, "\n") + for _, bad := range []string{"/dev/ptp0", "E2B_CHRONY_PHC", "refclock"} { + if strings.Contains(openrcCode, bad) { + t.Errorf("openrc init setup branches on %q; the time source is chosen at boot, not while provisioning", bad) + } + } +} + // The cache fingerprint must cover the whole generated provisioning contract: // stable across calls, and carrying both the selector text and the explicit // Version (a profile change must rotate the base-layer cache key). diff --git a/packages/orchestrator/pkg/template/build/phases/base/distro/init.go b/packages/orchestrator/pkg/template/build/phases/base/distro/init.go index d55d811efc..5cb456a9fa 100644 --- a/packages/orchestrator/pkg/template/build/phases/base/distro/init.go +++ b/packages/orchestrator/pkg/template/build/phases/base/distro/init.go @@ -37,6 +37,13 @@ echo "Enable time synchronization ($E2B_TIMESYNC_UNIT)" # Distro-correct chrony unit (chrony on Debian, chronyd on RHEL/Arch). systemctl enable "$E2B_TIMESYNC_UNIT" +echo "Pull the boot-time time-source selector into $E2B_TIMESYNC_UNIT" +# e2b-chrony-source.service writes the source line chrony.conf includes; the +# unit that must pull it in is only known here (the name differs per family). +mkdir -p "/etc/systemd/system/$E2B_TIMESYNC_UNIT.service.d" +printf '[Unit]\nRequires=e2b-chrony-source.service\nAfter=e2b-chrony-source.service\n' \ + >"/etc/systemd/system/$E2B_TIMESYNC_UNIT.service.d/e2b-chrony-source.conf" + echo "Enable SSH ($E2B_SSH_UNIT)" # provision.sh writes the sandbox sshd_config on every family, but nothing was # turning the unit on: Debian's postinst and the RHEL RPM scriptlet enable it @@ -116,6 +123,28 @@ fi echo "Enable time synchronization ($E2B_TIMESYNC_UNIT)" rc-update add "$E2B_TIMESYNC_UNIT" default +echo "Install the boot-time time-source selector" +# Writes the source line chrony.conf includes, in the boot runlevel so it is +# done before chronyd starts in default. Baked outside /etc/init.d for the same +# reason as the envd service script. +cp /usr/local/share/e2b/chrony-source.openrc /etc/init.d/e2b-chrony-source +chmod 0755 /etc/init.d/e2b-chrony-source +rc-update add e2b-chrony-source boot + +echo "Disabling the chronyd seccomp filter" +# Alpine's OpenRC init script hardcodes '-F 1', which loads chronyd's seccomp +# filter, and Alpine's chrony build (-NTS -SECHASH -DEBUG) takes a SIGSYS — "Bad +# system call" right after "Loaded seccomp filter (level 1)" — as soon as the PHC +# refclock is driven, leaving the service in OpenRC's "crashed" state with the +# clock unsynced. Unconditional, NOT gated on the PHC being present: which source +# chronyd drives is decided at boot by e2b-chrony-source, so provisioning cannot +# know. It costs nothing when the pool branch is taken, and the systemd families +# pass no -F at all, so this is parity rather than a new hole. The init script +# splices $command_args in after its own -F 1 and chronyd honours the last -F. +# conf.d must be named for the init script OpenRC sources it for. +mkdir -p /etc/conf.d +echo 'command_args="-F 0"' >>"/etc/conf.d/$E2B_TIMESYNC_UNIT" + echo "Enable envd autostart" # The service script is baked at a neutral path (envd.openrc.tpl) so the # Debian family's update-rc.d never sees it; install it for OpenRC here. diff --git a/packages/orchestrator/pkg/template/build/phases/base/provision.sh b/packages/orchestrator/pkg/template/build/phases/base/provision.sh index 0801b43552..6bde8838af 100644 --- a/packages/orchestrator/pkg/template/build/phases/base/provision.sh +++ b/packages/orchestrator/pkg/template/build/phases/base/provision.sh @@ -101,20 +101,11 @@ fi echo "Setting up chrony" mkdir -p /etc/chrony { - # Prefer the hypervisor's PTP clock (kvm-ptp): no network dependency and - # it tracks the host directly. It is missing where nested virtualization - # can't expose it (e.g. dev slots) — chronyd treats a missing PHC as a - # FATAL error, so only reference it when the device exists and fall back - # to NTP otherwise; a running chronyd without PHC beats a dead one. - # Device presence is probed in the provisioning VM, which runs on the same - # host/KVM as the runtime sandboxes. - if [ -e /dev/ptp0 ]; then - echo "refclock PHC /dev/ptp0 poll 2 dpoll 2" - E2B_CHRONY_PHC=1 - else - echo "pool pool.ntp.org iburst maxsources 3" - E2B_CHRONY_PHC=0 - fi + # The source line (PHC refclock vs NTP pool) is chosen at BOOT by + # e2b-chrony-source, not here: a template provisioned on a node with + # /dev/ptp0 can cold-boot on a node without it, and a refclock line for a + # missing PHC is FATAL to chronyd. + echo "include /run/chrony-e2b/source.conf" # Step (jump) the clock instead of slewing when the offset exceeds 1s, but # only for the first 3 updates after chronyd starts. chronyd restarts on # every cold boot/reboot, so this corrects a large boot-time offset fast @@ -124,20 +115,6 @@ mkdir -p /etc/chrony echo "makestep 1.0 3" } >/etc/chrony/chrony.conf -# Alpine's OpenRC init script hardcodes `-F 1`, which loads chronyd's seccomp -# filter. Alpine's chrony build (-NTS -SECHASH -DEBUG) takes a SIGSYS — -# "Bad system call" right after "Loaded seccomp filter (level 1)" — as soon as -# the PHC refclock is driven, so the service lands in OpenRC's "crashed" state -# with the clock unsynced. The pool path is unaffected, and the systemd families -# pass no -F at all, so this only restores parity. The init script splices -# $command_args in after its own -F 1 and chronyd honours the last -F, so this -# turns the filter off. -if [ "$E2B_CHRONY_PHC" = 1 ] && [ "$E2B_INIT_SYSTEM" = "openrc" ]; then - echo "Disabling the chronyd seccomp filter (PHC refclock traps under it)" - mkdir -p /etc/conf.d - echo 'command_args="-F 0"' >>/etc/conf.d/chronyd -fi - # Add a proxy config, as some environments expects it there (e.g. timemaster in Node Dockerimage) echo "include /etc/chrony/chrony.conf" >/etc/chrony.conf diff --git a/packages/orchestrator/pkg/template/build/phases/base/provision_test.go b/packages/orchestrator/pkg/template/build/phases/base/provision_test.go new file mode 100644 index 0000000000..9e9d43a71c --- /dev/null +++ b/packages/orchestrator/pkg/template/build/phases/base/provision_test.go @@ -0,0 +1,31 @@ +//go:build linux + +package base + +import ( + "strings" + "testing" +) + +// Provisioning must not decide the chrony time source: it runs on a build node, +// and the sandbox can cold-boot on a node with a different PHC situation. The +// baked config only includes what e2b-chrony-source writes at boot. +func TestProvisionScriptDefersChronySourceToBoot(t *testing.T) { + t.Parallel() + // E2B_CHRONY_PHC was the provision-time verdict the Alpine seccomp workaround + // used to read. It no longer exists, and under `set -u` a leftover reference + // is a hard provisioning failure on every distro, not just Alpine. + for _, bad := range []string{"[ -e /dev/ptp0 ]", "refclock PHC", "E2B_CHRONY_PHC"} { + if strings.Contains(provisionScriptFile, bad) { + t.Errorf("provision.sh must not decide the time source (%q) — that happens at boot", bad) + } + } + for _, want := range []string{ + `echo "include /run/chrony-e2b/source.conf"`, + `echo "makestep 1.0 3"`, + } { + if !strings.Contains(provisionScriptFile, want) { + t.Errorf("provision.sh missing chrony config line %q", want) + } + } +}