From 1512a699514221ee428eb85356496d82c07ed077 Mon Sep 17 00:00:00 2001 From: Bill Njoroge Date: Wed, 19 Aug 2026 16:14:50 -0400 Subject: [PATCH 1/4] fix(orchestrator): conformance-campaign server and runtime fixes Fixes found running the 13-target real-world conformance campaign on the official runner-large golden (Apple Silicon, arm64). Server fixes: - docker: probe overlay and force overlay2/vfs storage driver in daemon.json; fuse-overlayfs cannot mount in the smolvm kernel and killed every container job (wheel builds now run). - pool: runner completion via oneshot instead of re-polling a completed &mut JoinHandle (tokio 'JoinHandle polled after completion' panic that tore the pool down at machine ~60). - store: PRAGMA wal_checkpoint(TRUNCATE) after each commit; the per-event run rewrite grew the WAL to 429MB and froze the server. - rosetta: prepare_rosetta_multiarch() installs amd64 loader+libc (libc6/libgcc-s1/libstdc++6/zlib1g/libsystemd0:amd64) into the packed golden during the prep boot on Apple Silicon, so dynamically linked x86_64 binaries run under Rosetta (react optipng-bin, valkey x86_64 tarballs). Fingerprint gains rosetta_libs to re-prep stale bases; the arm64 apt sources are scoped (Architectures: arm64) with an archive.ubuntu.com [arch=amd64] source across all four suites. - runner_user wrapper: raise RLIMIT_NOFILE to 524288 (GitHub's systemd default) as root before the setpriv drop; --init-groups now correct in both branches via sudo. Fixes valkey's 'maximum open files' EPERM. - guest PATH: cargo bin dir now matches the runner user instead of hardcoding /root/.cargo/bin, which the unprivileged runner cannot stat (nodejs/ci EACCES on git lookup). Harness (conformance-5repos.sh): - pass --repository {owner}/{repo} on submit (checkouts with explicit refs synced the client default local/preloop) - sample the changed-file payload to 50 files (run creation was O(payload x jobs); deno's 8500-file list timed out) - PR-event retry carries an action; fingerprint replication matches the rosetta_libs field --- benchmarks/real-world/conformance-5repos.sh | 72 ++++- .../preloop-orchestrator/src/environment.rs | 7 + crates/preloop-orchestrator/src/lib.rs | 245 +++++++++++++++--- .../tests/runner_pool_lifecycle.rs | 4 +- crates/preloop-runner-server/src/store.rs | 12 + 5 files changed, 283 insertions(+), 57 deletions(-) diff --git a/benchmarks/real-world/conformance-5repos.sh b/benchmarks/real-world/conformance-5repos.sh index 2f44b946..c1073a1d 100755 --- a/benchmarks/real-world/conformance-5repos.sh +++ b/benchmarks/real-world/conformance-5repos.sh @@ -28,7 +28,7 @@ POOL_SIZE="${PRELOOP_RUNNER_POOL_SIZE:-1}" HOST_HOME="${HOME:-}" SMOLVM_PROCESS_HOME="${CONFORMANCE_SMOLVM_HOME:-$CAMPAIGN_HOME/smolvm-home}" export PRELOOP_SYSTEM_TOKEN="${PRELOOP_SYSTEM_TOKEN:-preloop-system-token}" -export PRELOOP_CLIENT_TIMEOUT_SECONDS="${PRELOOP_CLIENT_TIMEOUT_SECONDS:-600}" +export PRELOOP_CLIENT_TIMEOUT_SECONDS="${PRELOOP_CLIENT_TIMEOUT_SECONDS:-3600}" SERVER_BIN="${PRELOOP_BIN:-$ROOT/target/debug/preloop}" CLIENT_BIN="${PRELOOP_CLIENT_BIN:-$ROOT/target/debug/preloop-runner-client}" @@ -84,23 +84,44 @@ target_cfg() { # frontend-metrics.yml was renamed to frontend-lint.yml upstream. echo "grafana https://github.com/grafana/grafana.git main .github/workflows/frontend-lint.yml push refs/heads/main" ;; deno/ci) + # ci.generated.yml is 36 jobs -> 134 after matrix expansion (each job is + # well under GitHub's 256-per-job cap; the earlier "408 sections" count + # was a misread). It is the only deno workflow that triggers on push; + # the compat-test and pr workflows are schedule/pull_request-only. echo "deno https://github.com/denoland/deno.git main .github/workflows/ci.generated.yml push refs/heads/main" ;; pydantic/ci) echo "pydantic https://github.com/pydantic/pydantic.git main .github/workflows/ci.yml push refs/heads/main" ;; pydantic/test) - echo "pydantic https://github.com/pydantic/pydantic.git main .github/workflows/test.yml push refs/heads/main" ;; + # pydantic has no test.yml; its test suite lives in ci.yml (the pydantic/ci + # target). Use the other large matrix workflow for this target. + echo "pydantic https://github.com/pydantic/pydantic.git main .github/workflows/third-party.yml push refs/heads/main" ;; valkey/ci) echo "valkey https://github.com/valkey-io/valkey.git unstable .github/workflows/ci.yml push refs/heads/unstable" ;; cli/test) - echo "cli https://github.com/cli/cli.git trunk .github/workflows/test.yml push refs/heads/trunk" ;; + # cli has no test.yml; its test suite is go.yml (tests + lint). + echo "cli https://github.com/cli/cli.git trunk .github/workflows/go.yml push refs/heads/trunk" ;; cli/lint) echo "cli https://github.com/cli/cli.git trunk .github/workflows/lint.yml push refs/heads/trunk" ;; + typescript/ci) + echo "typescript https://github.com/microsoft/TypeScript.git main .github/workflows/ci.yml push refs/heads/main" ;; + nodejs/test) + echo "nodejs https://github.com/nodejs/node.git main .github/workflows/test-linux.yml push refs/heads/main" ;; + react/ci) + echo "react https://github.com/facebook/react.git main .github/workflows/runtime_build_and_test.yml push refs/heads/main" ;; + vscode/test) + # pr.yml is pull_request-only; the script's push->pull_request retry + # handles it (payload now carries an action). + echo "vscode https://github.com/microsoft/vscode.git main .github/workflows/pr.yml push refs/heads/main" ;; + spark/ci) + # build_and_test.yml is workflow_call-only; build_main.yml is the push + # entry point that calls it. + echo "spark https://github.com/apache/spark.git master .github/workflows/build_main.yml push refs/heads/master" ;; *) return 1 ;; esac } all_targets() { - echo "grafana/ci grafana/frontend-metrics deno/ci pydantic/ci pydantic/test valkey/ci cli/test cli/lint" + echo "grafana/ci grafana/frontend-metrics deno/ci pydantic/ci pydantic/test valkey/ci cli/test cli/lint typescript/ci nodejs/test react/ci vscode/test spark/ci" } repo_targets() { @@ -110,6 +131,11 @@ repo_targets() { pydantic) echo "pydantic/ci pydantic/test" ;; valkey) echo "valkey/ci" ;; cli) echo "cli/test cli/lint" ;; + typescript) echo "typescript/ci" ;; + nodejs) echo "nodejs/test" ;; + react) echo "react/ci" ;; + vscode) echo "vscode/test" ;; + spark) echo "spark/ci" ;; *) return 1 ;; esac } @@ -138,7 +164,15 @@ prepare_golden_home() { # so nothing is curated and there are no toolchains). fingerprint="$(python3 - "$OFFICIAL_GOLDEN_BASE" <<'INNERPY' import hashlib, json, sys -normalized = {"base": sys.argv[1], "toolchains": [], "curated": False, "bake": ""} +import platform +rosetta = platform.system() == "Darwin" and platform.machine() in ("arm64", "aarch64") +normalized = { + "base": sys.argv[1], + "toolchains": [], + "curated": False, + "bake": "", + "rosetta_libs": rosetta, +} print(hashlib.sha256(json.dumps(normalized, separators=(",", ":")).encode()).hexdigest()) INNERPY )" @@ -269,11 +303,20 @@ except subprocess.CalledProcessError: before = "0" * 40 # A campaign run is intentionally a synthetic "changed everything" event: # it must exercise the selected workflow even when the current upstream commit -# touched an unrelated path. The list is complete, so the server can still -# evaluate path filters without guessing. -paths = git("ls-files") +# touched an unrelated path. The full `ls-files` list is deliberately NOT +# used: the payload is embedded in every job message's github context, so a +# multi-thousand-file list makes run creation take minutes per workflow +# (measured: 3000+ paths -> >120s; 2 paths -> 9s). Path filters only need a +# non-empty set that is not fully ignored, so a spread sample of the tree is +# enough to make the workflow run. +all_paths = git("ls-files") +paths = all_paths[:25] + all_paths[-25:] print(json.dumps({ + # pull_request activity type: the server's event matcher requires one of + # the default PR types (opened/synchronize/reopened) when a workflow + # declares a pull_request trigger; the push submission ignores it. + "action": "synchronize", "before": before, "after": head, "ref": f"refs/heads/{branch}", @@ -326,13 +369,15 @@ EOF clone_repo "$slug" "$url" "$branch" ws_dir="$WORKSPACE_ROOT/$slug" [ -f "$ws_dir/$workflow" ] || fail "$target workflow is missing after checkout: $ws_dir/$workflow" + repo_slug="$(echo "$url" | sed -E 's#^https://github.com/([^/]+/[^/.]+)(\.git)?$#\1#' | sed 's/\.git$//')" + [ -n "$repo_slug" ] || repo_slug="$slug" target_dir="$OUTPUT_ROOT/$target" rm -rf "$target_dir" mkdir -p "$target_dir" write_push_payload "$ws_dir" "$branch" "$target_dir/event.json" echo "=== [$target] submitting $workflow ($event $git_ref) ===" if ! submit="$("$CLIENT_BIN" --server "http://127.0.0.1:$PORT" submit \ - -W "$ws_dir/$workflow" --workspace-root "$ws_dir" \ + -W "$ws_dir/$workflow" --workspace-root "$ws_dir" --repository "$repo_slug" \ --git-ref "$git_ref" --event "$event" --payload "$target_dir/event.json" 2>&1)"; then # Some upstream workflows intentionally omit push and only accept # pull_request. The same complete changed-file payload is valid for the @@ -341,7 +386,7 @@ EOF event="pull_request" echo "[$target] retrying with pull_request trigger" submit="$("$CLIENT_BIN" --server "http://127.0.0.1:$PORT" submit \ - -W "$ws_dir/$workflow" --workspace-root "$ws_dir" \ + -W "$ws_dir/$workflow" --workspace-root "$ws_dir" --repository "$repo_slug" \ --git-ref "$git_ref" --event "$event" --payload "$target_dir/event.json" 2>&1)" || { printf '%s\n' "$submit" | tee "$target_dir/submit.txt" fail "[$target] workflow submission failed" @@ -355,8 +400,11 @@ EOF run_id="$(printf '%s\n' "$submit" | python3 -c 'import json,sys; print(json.load(sys.stdin)["run_id"])')" printf '%s\n' "$run_id" >"$target_dir/run-id.txt" final_status="$(wait_run "$target" "$run_id")" - run_snapshot "$run_id" "$target_dir/run.json" - printf '%s\n' "$final_status" >"$target_dir/status.txt" + # The snapshot curl and status write must never kill the campaign under + # `set -e`: a transient server hiccup after the run concluded would abort + # the whole run and lose the recorded result. + run_snapshot "$run_id" "$target_dir/run.json" || true + printf '%s\n' "$final_status" >"$target_dir/status.txt" || true echo "=== [$target] final status: $final_status ===" case "$final_status" in success|skipped) ;; diff --git a/crates/preloop-orchestrator/src/environment.rs b/crates/preloop-orchestrator/src/environment.rs index 6fac6a3e..fbe3cb83 100644 --- a/crates/preloop-orchestrator/src/environment.rs +++ b/crates/preloop-orchestrator/src/environment.rs @@ -376,6 +376,13 @@ impl EnvironmentSpec { // bases skip the bake entirely, so the fingerprint records that // too. "bake": if curated { crate::base_install_script() } else { String::new() }, + // Rosetta x86_64 translation exists only on Apple Silicon hosts. + // The packed-golden prep installs the amd64 loader + libc into + // arm64 goldens so dynamically linked x86_64 binaries run under + // it; the fingerprint records whether that shim is present so a + // host-class change re-preps the golden once instead of adopting + // a base built for the other class. + "rosetta_libs": cfg!(target_os = "macos") && std::env::consts::ARCH == "aarch64", }); let bytes = serde_json::to_vec(&normalized).expect("normalized environment is serializable"); diff --git a/crates/preloop-orchestrator/src/lib.rs b/crates/preloop-orchestrator/src/lib.rs index 6df1c91c..b5bc3862 100644 --- a/crates/preloop-orchestrator/src/lib.rs +++ b/crates/preloop-orchestrator/src/lib.rs @@ -1070,11 +1070,6 @@ pub fn loopback_hosts() -> &'static str { LOOPBACK_HOSTS } -/// PATH exported to the guest runner. Exposed for the lifecycle tests. -pub fn guest_runner_path() -> &'static str { - GUEST_RUNNER_PATH -} - fn node_externals_at(runner_root: &str) -> Vec> { [vec![ "sh".to_owned(), @@ -1290,6 +1285,14 @@ fn base_install_commands() -> Vec> { /// A stale `/var/run/docker.pid` naming that same pid blocks startup outright, /// and is only removed once `docker info` has failed so it is stale by /// definition. +/// +/// The storage driver is probed, never assumed: the golden's daemon auto-selects +/// `fuse-overlayfs`, which cannot mount inside the smolvm kernel (no `/dev/fuse`, +/// and the bundled fuse-overlayfs rejects the `lazytime` option), so every +/// `docker run` in a container job dies with "fuse: device not found". We try a +/// real overlay mount first and fall back to `vfs`; if the daemon then refuses +/// the previous driver's data, the docker data-root is reset and dockerd is +/// retried once (images re-pull from the registry). fn docker_start_command() -> Vec { vec![ "sh".to_owned(), @@ -1299,6 +1302,23 @@ fn docker_start_command() -> Vec { docker info >/dev/null 2>&1 && exit 0; \ rm -f /var/run/docker.pid; \ mkdir -p {DOCKER_DATA_ROOT}; \ + modprobe overlay >/dev/null 2>&1 || true; \ + modprobe fuse >/dev/null 2>&1 || true; \ + mkdir -p /tmp/.preloop-ovprobe; \ + if mount -t overlay overlay -o lowerdir=/tmp,/usr /tmp/.preloop-ovprobe 2>/dev/null; then \ + umount /tmp/.preloop-ovprobe 2>/dev/null || true; \ + DRIVER=overlay2; \ + else \ + DRIVER=vfs; \ + fi; \ + rmdir /tmp/.preloop-ovprobe 2>/dev/null || true; \ + printf '{{\"data-root\":\"{DOCKER_DATA_ROOT}\",\"storage-driver\":\"%s\"}}\\n' \"$DRIVER\" > /etc/docker/daemon.json; \ + (dockerd >/var/log/dockerd.log 2>&1 &) ; \ + for _ in $(seq 1 50); do \ + docker info >/dev/null 2>&1 && exit 0; \ + sleep 0.2; \ + done; \ + rm -rf {DOCKER_DATA_ROOT}/*; \ (dockerd >/var/log/dockerd.log 2>&1 &) ; \ for _ in $(seq 1 50); do \ docker info >/dev/null 2>&1 && exit 0; \ @@ -1457,11 +1477,24 @@ async fn write_bake_manifest( /// dtolnay/rust-toolchain only appends that directory to `$GITHUB_PATH` when it /// has to install rustup itself, so on an image that already has rustup — ours, /// and GitHub's — the directory is on PATH or the tool is simply unreachable. -/// Guests run as root, so `$HOME/.cargo` is `/root/.cargo`; the Go layer -/// untars into `/usr/local/go`. Absent directories cost nothing. -const GUEST_RUNNER_PATH: &str = "/root/.cargo/bin:/usr/local/go/bin:\ - /usr/local/sbin:/usr/local/bin:\ - /usr/sbin:/usr/bin:/sbin:/bin"; +/// +/// The cargo bin dir must match the user the runner executes steps as: a root +/// runner (no switching) installs into `/root/.cargo`, a switched runner into +/// `/home//.cargo`. The root-only path must never be exported to an +/// unprivileged runner — `/root` is 0700, so every tool lookup stats it and +/// gets EACCES (nodejs/ci: `EACCES: permission denied, stat +/// '/root/.cargo/bin/git'`), and the Go layer untars into the world-readable +/// `/usr/local/go`. Absent directories cost nothing. +pub fn guest_runner_path(config: &RunnerPoolConfig) -> String { + let cargo_bin = match config.runner_user.as_deref() { + None | Some("root") => "/root/.cargo/bin".to_owned(), + Some(user) => format!("/home/{user}/.cargo/bin"), + }; + format!( + "{cargo_bin}:/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:\ + /usr/sbin:/usr/bin:/sbin:/bin" + ) +} /// `env` prefix for guest runner invocations, empty when nothing needs setting. /// @@ -1470,7 +1503,7 @@ const GUEST_RUNNER_PATH: &str = "/root/.cargo/bin:/usr/local/go/bin:\ /// vice versa, so neither may gate the other. fn guest_env_prefix(config: &RunnerPoolConfig, name: &MachineName) -> Vec { let mut env = Vec::new(); - env.push(format!("PATH={GUEST_RUNNER_PATH}")); + env.push(format!("PATH={}", guest_runner_path(config))); // The guest needs its own VM name so a debug session can tell a controller // which machine to open a shell into. Nothing else in the guest knows it. env.push(format!("PRELOOP_MACHINE_NAME={}", name.as_str())); @@ -1870,6 +1903,73 @@ async fn preload_images( Ok(()) } +/// Install the amd64 loader + libc into an arm64 golden so dynamically +/// linked x86_64 binaries can run under Rosetta translation. +/// +/// Only Apple Silicon hosts have Rosetta, so everything else is a no-op; the +/// script additionally self-guards on the guest arch (an x86_64 golden on a +/// Mac already ships the amd64 rootfs natively). The trailing `sync` is +/// load-bearing exactly as in [`preload_images`]: forking captures the disk, +/// not the page cache, and the installed packages must reach the frozen base. +/// The package set covers the loader, libc, C++ runtime, zlib, and systemd +/// (valkey's official x86_64 tarballs link `libsystemd.so.0`); apt pulls the +/// amd64 transitive deps. +async fn prepare_rosetta_multiarch( + provider: &P, + golden: &MachineName, +) -> Result<(), OrchestratorError> { + if !(cfg!(target_os = "macos") && std::env::consts::ARCH == "aarch64") { + return Ok(()); + } + // arm64 Ubuntu's deb822 sources (ubuntu.sources) point at + // ports.ubuntu.com, which does not carry amd64; once `dpkg + // --add-architecture amd64` runs, every source without an + // `Architectures:` line serves amd64 too and ports 404s. Scope the native + // stanzas to arm64 and add an explicit archive.ubuntu.com [arch=amd64] + // source across all four suites (the image carries security-update + // versions — noble main alone is older and the mutual glibc Breaks pins + // make the resolver fail). One suite per deb line: the one-line format + // takes a single suite and misparses extras as components. Without the + // scoping, every later apt-get update (including the per-fork + // hosted-baseline install) fails on the amd64 fetch. + let script = run_as_root_or_sudo( + "case \"$(uname -m)\" in \ + aarch64|arm64) ;; \ + *) echo 'guest is not arm64; rosetta multiarch install is a no-op' >&2; exit 0 ;; \ + esac; \ + dpkg --add-architecture amd64; \ + sed -i '/^Types: deb$/a Architectures: arm64' /etc/apt/sources.list.d/ubuntu.sources; \ + CODENAME=$(. /etc/os-release 2>/dev/null && echo \"$VERSION_CODENAME\"); \ + [ -n \"$CODENAME\" ] || CODENAME=noble; \ + for s in '' '-updates' '-backports' '-security'; do \ + printf 'deb [arch=amd64] http://archive.ubuntu.com/ubuntu/ %s%s main restricted universe multiverse\\n' \"$CODENAME\" \"$s\" \ + >> /etc/apt/sources.list.d/preloop-amd64.list; \ + done; \ + apt-get update -qq; \ + DEBIAN_FRONTEND=noninteractive \ + apt-get install -y -qq --no-install-recommends \ + libc6:amd64 libgcc-s1:amd64 libstdc++6:amd64 zlib1g:amd64 \ + libsystemd0:amd64; \ + sync; \ + test -f /lib64/ld-linux-x86-64.so.2", + ); + let output = provider + .exec(golden, &["sh".to_owned(), "-c".to_owned(), script]) + .await?; + if output.exit_code != 0 { + return Err(OrchestratorError::Config(format!( + "rosetta multiarch install failed (exit {}): {}", + output.exit_code, + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + info!( + machine = golden.as_str(), + "installed amd64 multiarch libs into golden for Rosetta x86_64 translation" + ); + Ok(()) +} + /// Prepare a running forkable golden VM with the requested environment. /// /// SmolVM takes the forkable RAM/disk snapshot when `start --forkable` runs. @@ -2081,6 +2181,10 @@ async fn prepare_packed_golden( let _ = provider.delete(golden).await; return Err(error); } + // Fatal, not a warning: on Apple Silicon the multiarch shim is the + // golden's contract, and a failed install leaves a reusable base that is + // adoptable on later restarts (amd64 arch added, loader missing). + prepare_rosetta_multiarch(provider.as_ref(), golden).await?; if let Err(error) = preload_images(provider.as_ref(), golden, &config.preload_images).await { warn!( machine = golden.as_str(), @@ -2192,9 +2296,18 @@ impl RunnerPool

{ result = slots.join_next() => { shutdown.cancel(); match result { - Some(Ok(Err(error))) => return Err(error), - Some(Err(error)) => return Err(OrchestratorError::Pool(error.to_string())), - Some(Ok(Ok(()))) => return Err(OrchestratorError::Pool("runner slot exited".into())), + Some(Ok(Err(error))) => { + error!(%error, "runner slot failed; tearing down pool"); + return Err(error); + } + Some(Err(error)) => { + error!(%error, "runner slot task panicked; tearing down pool"); + return Err(OrchestratorError::Pool(error.to_string())); + } + Some(Ok(Ok(()))) => { + error!("runner slot exited without shutdown; tearing down pool"); + return Err(OrchestratorError::Pool("runner slot exited".into())); + } None => return Err(OrchestratorError::Pool("runner pool had no slots".into())), } } @@ -3201,12 +3314,20 @@ async fn run_one_runner( } = plan; let (busy_tx, busy_rx) = tokio::sync::oneshot::channel(); + // The runner's completion is observed through this oneshot, never by + // re-polling the JoinHandle: `tokio::join!(&mut run_task, successor)` + // panics with "JoinHandle polled after completion" when the runner exits + // before the successor finishes provisioning and `select!` re-polls the + // branch — a completed `&mut JoinHandle` cannot be polled again. + let (done_tx, done_rx) = tokio::sync::oneshot::channel(); let claimed = Arc::new(std::sync::atomic::AtomicBool::new(false)); let run_provider = provider.clone(); let run_name = name.clone(); idle.fetch_add(1, Ordering::AcqRel); - let mut run_task = - tokio::spawn(async move { run_until_exit(&run_provider, &run_name, &run, busy_tx).await }); + let run_task = tokio::spawn(async move { + let result = run_until_exit(&run_provider, &run_name, &run, busy_tx).await; + let _ = done_tx.send(result); + }); // Resolves once the runner reports a job and its replacement is ready. A // runner that exits without taking a job (shutdown, transient failure) @@ -3276,12 +3397,18 @@ async fn run_one_runner( }, pair = async { // Concurrent on purpose: the successor is built while the job is - // still running, which is the whole point of the busy signal. - tokio::join!(&mut run_task, build_successor) + // still running, which is the whole point of the busy signal. The + // oneshot is polled once by value, so a runner that exits before + // the successor is ready cannot be re-polled into a panic. + tokio::join!(done_rx, build_successor) } => { let result = match pair.0 { - Ok(result) => result.map_err(OrchestratorError::from), - Err(error) => Err(OrchestratorError::Pool(error.to_string())), + Ok(Ok(())) => Ok(()), + Ok(Err(error)) => Err(OrchestratorError::Pool(error.to_string())), + // The runner task panicked (sender dropped without a value). + Err(_) => Err(OrchestratorError::Pool( + "runner task ended without a result".into(), + )), }; (result, pair.1) }, @@ -3920,15 +4047,29 @@ fn as_runner_user(config: &RunnerPoolConfig, argv: &[String]) -> Vec { // permitted without privileges. The root branch keeps --init-groups. use base64::Engine as _; let b64 = base64::engine::general_purpose::STANDARD.encode(&provisioning); + // Raise RLIMIT_NOFILE before dropping privileges. GitHub-hosted runners + // allow many open files (valkey's test suite raises the soft limit to + // 10032), but the exec channel's defaults leave the hard limit below + // that, so the runner user gets EPERM on setrlimit. 524288 mirrors + // systemd's built-in hard default, which is what GitHub's runner service + // (no explicit LimitNOFILE) inherits. Raising the hard limit needs root + // (CAP_SYS_RESOURCE), hence the sudo in the exec-as-image-user branch; + // setpriv then runs as root there too, so --init-groups is correct in + // both branches (setgroups needs root — the --keep-groups variant was + // only a workaround for the self-drop). + let inner = format!( + "ulimit -Hn 524288; ulimit -Sn 524288; \ + exec setpriv --reuid {uid} --regid {uid} --init-groups env \ + PRELOOP_RUNNER_USER={user} PRELOOP_RUNNER_UID={uid} HOME={home} {program} {args}" + ); + let inner_b64 = base64::engine::general_purpose::STANDARD.encode(&inner); let script = format!( "if [ \"$(id -u)\" -eq 0 ]; then \ {provisioning}; \ - exec setpriv --reuid {uid} --regid {uid} --init-groups env \ - PRELOOP_RUNNER_USER={user} PRELOOP_RUNNER_UID={uid} HOME={home} {program} {args}; \ + printf %s '{inner_b64}' | base64 -d | sh; \ else \ printf %s '{b64}' | base64 -d | sudo -n sh 2>/dev/null || true; \ - exec setpriv --reuid {uid} --regid {uid} --keep-groups env \ - PRELOOP_RUNNER_USER={user} PRELOOP_RUNNER_UID={uid} HOME={home} {program} {args}; \ + printf %s '{inner_b64}' | base64 -d | sudo -n sh; \ fi" ); vec!["sh".to_owned(), "-c".to_owned(), script] @@ -4072,6 +4213,7 @@ fn packed_golden_path(payload: &Path) -> PathBuf { mod lifecycle_tests { use super::*; use async_trait::async_trait; + use base64::Engine as _; use preloop_vm::{ExecOutput, OutputChunk}; use std::collections::HashMap; use std::os::unix::fs::PermissionsExt as _; @@ -4529,17 +4671,6 @@ chmod +x "$destination/bin/node" script.contains("chmod 777 /run/preloop-control"), "{script}" ); - // Root branch (locally baked goldens) drops with --init-groups; the - // exec-as-image-user branch (official golden) provisions via sudo and - // self-drops with --keep-groups (setgroups needs root). - assert!( - script.contains("setpriv --reuid 1001 --regid 1001 --init-groups"), - "{script}" - ); - assert!( - script.contains("setpriv --reuid 1001 --regid 1001 --keep-groups"), - "{script}" - ); assert!( script.contains("NOPASSWD: ALL"), "the runner account must be able to sudo non-interactively, \ @@ -4549,16 +4680,44 @@ chmod +x "$destination/bin/node" script.contains("| base64 -d | sudo -n sh 2>/dev/null || true"), "{script}" ); - assert_eq!( - script - .matches("'/opt/preloop/bin/preloop-runner' 'run' '--once'") - .count(), - 2, - "the wrapped program must appear in both branches" + // Both branches run the launch (limit raise + setpriv drop) from a + // base64'd script; the image-user branch pipes it through sudo so the + // raise and the --init-groups setgroups run as root. Decode every + // blob and assert across them. + let blobs: Vec = script + .split("| base64 -d") + .filter_map(|part| { + let close = part.rfind('\'')?; + let open = part[..close].rfind('\'')?; + Some(part[open + 1..close].to_owned()) + }) + .collect(); + let all = blobs + .iter() + .map(|b| { + String::from_utf8( + base64::engine::general_purpose::STANDARD.decode(b).unwrap(), + ) + .unwrap() + }) + .collect::>() + .join("\n"); + assert!( + all.contains("ulimit -Hn 524288; ulimit -Sn 524288"), + "{all}" ); assert!( - script.contains("PRELOOP_RUNNER_USER=runner PRELOOP_RUNNER_UID=1001"), - "{script}" + all.contains("setpriv --reuid 1001 --regid 1001 --init-groups"), + "{all}" + ); + assert!(!all.contains("--keep-groups"), "{all}"); + assert!( + all.contains("'/opt/preloop/bin/preloop-runner' 'run' '--once'"), + "{all}" + ); + assert!( + all.contains("PRELOOP_RUNNER_USER=runner PRELOOP_RUNNER_UID=1001"), + "{all}" ); } diff --git a/crates/preloop-orchestrator/tests/runner_pool_lifecycle.rs b/crates/preloop-orchestrator/tests/runner_pool_lifecycle.rs index 3a4bf3ff..99106f88 100644 --- a/crates/preloop-orchestrator/tests/runner_pool_lifecycle.rs +++ b/crates/preloop-orchestrator/tests/runner_pool_lifecycle.rs @@ -693,7 +693,7 @@ async fn runner_keeps_public_only_egress_and_wires_control_socket_and_environmen // to tell a controller which VM to open a shell into. let expected_prefix = vec![ "/usr/bin/env".to_owned(), - format!("PATH={}", preloop_orchestrator::guest_runner_path()), + format!("PATH={}", preloop_orchestrator::guest_runner_path(&config)), format!("PRELOOP_MACHINE_NAME={runner}"), "PRELOOP_CONTROL_ORIGIN=https://preloop.example".to_owned(), "PRELOOP_CONTROL_SOCKET=/run/preloop-control/engine.sock".to_owned(), @@ -781,7 +781,7 @@ async fn guest_environment_tracks_control_socket_and_debug_dir_independently() { .map(String::as_str) .collect(); let machine_name = format!("PRELOOP_MACHINE_NAME={runner}"); - let path = format!("PATH={}", preloop_orchestrator::guest_runner_path()); + let path = format!("PATH={}", preloop_orchestrator::guest_runner_path(&config)); let mut want = vec!["/usr/bin/env", path.as_str(), machine_name.as_str()]; want.extend(expected); assert_eq!( diff --git a/crates/preloop-runner-server/src/store.rs b/crates/preloop-runner-server/src/store.rs index 2beaa534..81ac9197 100644 --- a/crates/preloop-runner-server/src/store.rs +++ b/crates/preloop-runner-server/src/store.rs @@ -1346,6 +1346,12 @@ impl SqliteStore { self.write_meta_tx(&tx, &snapshot.meta)?; tx.commit() .map_err(|error| anyhow::anyhow!("committing store snapshot: {error}"))?; + // The WAL grows with every runner event; an unbounded WAL (hundreds of + // MB) makes the next write's checkpoint sync stall the server for + // minutes. Keep it small so a commit is never a multi-hundred-MB sync. + connection + .execute_batch("PRAGMA wal_checkpoint(TRUNCATE);") + .map_err(|error| anyhow::anyhow!("checkpointing WAL: {error}"))?; Ok(()) } @@ -1381,6 +1387,12 @@ impl SqliteStore { self.insert_event_tx(&tx, &projection.event)?; tx.commit() .map_err(|error| anyhow::anyhow!("committing run event: {error}"))?; + // Same rationale as the full-snapshot path: keep the WAL bounded so a + // runner-event burst cannot stall the next commit behind a giant + // checkpoint sync. + connection + .execute_batch("PRAGMA wal_checkpoint(TRUNCATE);") + .map_err(|error| anyhow::anyhow!("checkpointing WAL: {error}"))?; Ok(()) } From df6b00c79e9908ab8b44e5b48eb533a60e713fd9 Mon Sep 17 00:00:00 2001 From: Bill Njoroge Date: Wed, 19 Aug 2026 21:54:18 -0400 Subject: [PATCH 2/4] fix(orchestrator): overlay probe, dockerd restart, rosetta prep hardening --- crates/preloop-orchestrator/src/lib.rs | 54 ++++++++++++++++------- crates/preloop-runner-server/src/store.rs | 45 ++++++++++++++++--- 2 files changed, 77 insertions(+), 22 deletions(-) diff --git a/crates/preloop-orchestrator/src/lib.rs b/crates/preloop-orchestrator/src/lib.rs index b5bc3862..0a6013b7 100644 --- a/crates/preloop-orchestrator/src/lib.rs +++ b/crates/preloop-orchestrator/src/lib.rs @@ -1305,7 +1305,7 @@ fn docker_start_command() -> Vec { modprobe overlay >/dev/null 2>&1 || true; \ modprobe fuse >/dev/null 2>&1 || true; \ mkdir -p /tmp/.preloop-ovprobe; \ - if mount -t overlay overlay -o lowerdir=/tmp,/usr /tmp/.preloop-ovprobe 2>/dev/null; then \ + if mount -t overlay overlay -o lowerdir=/tmp:/usr /tmp/.preloop-ovprobe 2>/dev/null; then \ umount /tmp/.preloop-ovprobe 2>/dev/null || true; \ DRIVER=overlay2; \ else \ @@ -1313,17 +1313,28 @@ fn docker_start_command() -> Vec { fi; \ rmdir /tmp/.preloop-ovprobe 2>/dev/null || true; \ printf '{{\"data-root\":\"{DOCKER_DATA_ROOT}\",\"storage-driver\":\"%s\"}}\\n' \"$DRIVER\" > /etc/docker/daemon.json; \ - (dockerd >/var/log/dockerd.log 2>&1 &) ; \ - for _ in $(seq 1 50); do \ - docker info >/dev/null 2>&1 && exit 0; \ - sleep 0.2; \ - done; \ + start_dockerd() {{ \ + rm -f /var/run/docker.pid; \ + dockerd >/var/log/dockerd.log 2>&1 & \ + DOCKERD_PID=$!; \ + ready=0; \ + for _ in $(seq 1 50); do \ + docker info >/dev/null 2>&1 && {{ ready=1; break; }}; \ + sleep 0.2; \ + done; \ + if [ \"$ready\" -eq 0 ]; then \ + kill \"$DOCKERD_PID\" 2>/dev/null || true; \ + for _ in $(seq 1 25); do \ + kill -0 \"$DOCKERD_PID\" 2>/dev/null || break; \ + sleep 0.2; \ + done; \ + return 1; \ + fi; \ + return 0; \ + }}; \ + if start_dockerd; then exit 0; fi; \ rm -rf {DOCKER_DATA_ROOT}/*; \ - (dockerd >/var/log/dockerd.log 2>&1 &) ; \ - for _ in $(seq 1 50); do \ - docker info >/dev/null 2>&1 && exit 0; \ - sleep 0.2; \ - done; \ + start_dockerd; \ exit 0" )), ] @@ -1933,7 +1944,8 @@ async fn prepare_rosetta_multiarch( // scoping, every later apt-get update (including the per-fork // hosted-baseline install) fails on the amd64 fetch. let script = run_as_root_or_sudo( - "case \"$(uname -m)\" in \ + "set -e; \ + case \"$(uname -m)\" in \ aarch64|arm64) ;; \ *) echo 'guest is not arm64; rosetta multiarch install is a no-op' >&2; exit 0 ;; \ esac; \ @@ -2086,6 +2098,18 @@ async fn prepare_golden_for_env( return Err(error); } if env_spec.curated { + // Same contract as the packed-golden path: on Apple Silicon the + // multiarch shim must reach the forkable base, or every dynamically + // linked x86_64 binary fails at job time. Runs before the baseline + // install so the amd64 sources are scoped before any later apt + // update. Fatal, not a warning — a half-installed golden would be + // adopted on later restarts (amd64 arch added, loader missing). + // Custom bases skip it along with the rest of the bake: the image is + // the operator's contract. + if let Err(error) = prepare_rosetta_multiarch(provider.as_ref(), golden).await { + let _ = provider.delete(golden).await; + return Err(error); + } if let Err(error) = install_base_dependencies(provider.as_ref(), golden).await { let _ = provider.delete(golden).await; return Err(error); @@ -4695,10 +4719,8 @@ chmod +x "$destination/bin/node" let all = blobs .iter() .map(|b| { - String::from_utf8( - base64::engine::general_purpose::STANDARD.decode(b).unwrap(), - ) - .unwrap() + String::from_utf8(base64::engine::general_purpose::STANDARD.decode(b).unwrap()) + .unwrap() }) .collect::>() .join("\n"); diff --git a/crates/preloop-runner-server/src/store.rs b/crates/preloop-runner-server/src/store.rs index 81ac9197..782738b8 100644 --- a/crates/preloop-runner-server/src/store.rs +++ b/crates/preloop-runner-server/src/store.rs @@ -817,6 +817,39 @@ pub(crate) fn restore_session_key( Ok(SessionEncryption::from_key(payload.0)) } +/// Bound the WAL after a commit, and fail loudly when the checkpoint could not +/// complete. +/// +/// `PRAGMA wal_checkpoint(TRUNCATE)` returns a result row of +/// `(busy, log_frames, checkpointed_frames)`; a blocked checkpoint reports +/// `busy = 1` but does not raise a SQL error, so discarding the row would +/// treat "WAL still full" as success. Backing off here also keeps one write +/// from starving the next: the retry gives competing readers time to finish +/// before we TRUNCATE again. Every post-commit write path funnels through +/// this helper. +fn checkpoint_wal(connection: &Connection) -> anyhow::Result<()> { + for _ in 0..10 { + let (busy, log, checkpointed) = + connection.query_row("PRAGMA wal_checkpoint(TRUNCATE);", [], |row| { + Ok(( + row.get::<_, i64>(0)?, + row.get::<_, i64>(1)?, + row.get::<_, i64>(2)?, + )) + })?; + if busy == 0 { + return Ok(()); + } + tracing::warn!( + log_frames = log, + checkpointed_frames = checkpointed, + "WAL checkpoint blocked; retrying" + ); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + anyhow::bail!("WAL checkpoint stayed blocked after retries") +} + impl SqliteStore { pub(crate) fn open(path: &std::path::Path, cipher: Envelope) -> anyhow::Result { if let Some(parent) = path.parent() { @@ -1201,6 +1234,7 @@ impl SqliteStore { )?; tx.commit() .map_err(|error| anyhow::anyhow!("committing log chunk: {error}"))?; + checkpoint_wal(&connection)?; Ok(()) } @@ -1349,9 +1383,7 @@ impl SqliteStore { // The WAL grows with every runner event; an unbounded WAL (hundreds of // MB) makes the next write's checkpoint sync stall the server for // minutes. Keep it small so a commit is never a multi-hundred-MB sync. - connection - .execute_batch("PRAGMA wal_checkpoint(TRUNCATE);") - .map_err(|error| anyhow::anyhow!("checkpointing WAL: {error}"))?; + checkpoint_wal(&connection)?; Ok(()) } @@ -1390,9 +1422,7 @@ impl SqliteStore { // Same rationale as the full-snapshot path: keep the WAL bounded so a // runner-event burst cannot stall the next commit behind a giant // checkpoint sync. - connection - .execute_batch("PRAGMA wal_checkpoint(TRUNCATE);") - .map_err(|error| anyhow::anyhow!("checkpointing WAL: {error}"))?; + checkpoint_wal(&connection)?; Ok(()) } @@ -1415,6 +1445,7 @@ impl SqliteStore { )?; tx.commit() .map_err(|error| anyhow::anyhow!("committing workflow run counter: {error}"))?; + checkpoint_wal(&connection)?; Ok(()) } @@ -1424,6 +1455,7 @@ impl SqliteStore { self.write_meta_tx(&tx, meta)?; tx.commit() .map_err(|error| anyhow::anyhow!("committing metadata: {error}"))?; + checkpoint_wal(&connection)?; Ok(()) } @@ -1570,6 +1602,7 @@ impl SqliteStore { self.insert_event_tx(&tx, event)?; tx.commit() .map_err(|error| anyhow::anyhow!("committing control event: {error}"))?; + checkpoint_wal(&connection)?; Ok(()) } From 22087d95e84ffcbf890e2f39c875e3f7beb6117e Mon Sep 17 00:00:00 2001 From: Bill Njoroge Date: Wed, 19 Aug 2026 22:52:32 -0400 Subject: [PATCH 3/4] fix(store): periodic WAL truncation instead of per-commit Addresses the blocking review concern: PRAGMA wal_checkpoint(TRUNCATE) after every store_inner/store_run_event commit fsyncs the DB per runner event (~11-35x slower in a WAL micro-benchmark, head-of-line blocking on the single connection). Now a truncating checkpoint runs only every 128 commits (WAL_CHECKPOINT_INTERVAL); SQLite's background wal_autocheckpoint bounds routine growth between truncations. The checkpoint_wal helper (result-row read + busy retry) is retained for observability. Bench (1500 small WAL txns, synchronous=NORMAL): truncate every commit: 0.107 ms/commit periodic (every 128): 0.0095 ms/commit, WAL bounded ~366 KB 21 store tests pass. --- crates/preloop-runner-server/src/store.rs | 51 ++++++++++++++++++----- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/crates/preloop-runner-server/src/store.rs b/crates/preloop-runner-server/src/store.rs index 782738b8..56c86110 100644 --- a/crates/preloop-runner-server/src/store.rs +++ b/crates/preloop-runner-server/src/store.rs @@ -19,6 +19,7 @@ use preloop_gha_protocol::SessionId; use rusqlite::{params, Connection, OptionalExtension, Transaction}; use sha2::Digest; use std::sync::Mutex as StdMutex; +use std::sync::atomic::{AtomicU64, Ordering}; const DATABASE_FILE: &str = "preloop.db"; pub(crate) const SNAPSHOT_FORMAT: u8 = 2; @@ -193,8 +194,22 @@ impl RunProjection { pub(crate) struct SqliteStore { connection: Arc>, cipher: Envelope, + /// Commits since the last forced WAL truncation. A full + /// `wal_checkpoint(TRUNCATE)` on every commit fsyncs the DB every runner + /// event (~35x slower in a WAL micro-benchmark and head-of-line blocking + /// on the single connection); instead SQLite's background + /// `wal_autocheckpoint` (PASSIVE) bounds routine growth and we force a + /// TRUNCATE only every [`WAL_CHECKPOINT_INTERVAL`] commits to reclaim the + /// file. Amortized cost is one blocking checkpoint per N events. + checkpoint_counter: Arc, } +/// Force a WAL truncation every N commits. Between forced truncations the +/// default `wal_autocheckpoint` (1000 pages ≈ 4 MB, PASSIVE, non-blocking) +/// keeps the WAL bounded; the periodic TRUNCATE guarantees the file is +/// reclaimed even when a reader kept PASSIVE from advancing. +const WAL_CHECKPOINT_INTERVAL: u64 = 128; + /// Where the server should look for durable state. Parsed from `PRELOOP_STORE_URL` /// (or an explicit override); see [`parse_store_url`]. #[derive(Debug, Clone, PartialEq, Eq)] @@ -871,9 +886,22 @@ impl SqliteStore { Ok(Self { connection: Arc::new(StdMutex::new(connection)), cipher, + checkpoint_counter: Arc::new(AtomicU64::new(0)), }) } + /// Post-commit WAL maintenance. Forces a truncating checkpoint only every + /// [`WAL_CHECKPOINT_INTERVAL`] commits (the first commit truncates so a + /// fresh DB starts clean); routine growth between truncations is bounded + /// by SQLite's background `wal_autocheckpoint`. This replaces the previous + /// truncate-on-every-commit policy, which fsynced the DB per runner event. + fn maybe_checkpoint_wal(&self, connection: &Connection) -> anyhow::Result<()> { + if self.checkpoint_counter.fetch_add(1, Ordering::Relaxed) % WAL_CHECKPOINT_INTERVAL == 0 { + checkpoint_wal(connection)?; + } + Ok(()) + } + /// Apply pending migrations. Each step runs in its own transaction inside /// `PRAGMA user_version`; the `schema_migrations` table is a human audit /// trail. Steps are append-only and idempotent. @@ -1234,7 +1262,7 @@ impl SqliteStore { )?; tx.commit() .map_err(|error| anyhow::anyhow!("committing log chunk: {error}"))?; - checkpoint_wal(&connection)?; + self.maybe_checkpoint_wal(&connection)?; Ok(()) } @@ -1381,9 +1409,10 @@ impl SqliteStore { tx.commit() .map_err(|error| anyhow::anyhow!("committing store snapshot: {error}"))?; // The WAL grows with every runner event; an unbounded WAL (hundreds of - // MB) makes the next write's checkpoint sync stall the server for - // minutes. Keep it small so a commit is never a multi-hundred-MB sync. - checkpoint_wal(&connection)?; + // MB) makes a later checkpoint sync stall the server for minutes. + // Force a truncation periodically (autocheckpoint bounds the rest) so + // the file is reclaimed without an fsync on every commit. + self.maybe_checkpoint_wal(&connection)?; Ok(()) } @@ -1419,10 +1448,10 @@ impl SqliteStore { self.insert_event_tx(&tx, &projection.event)?; tx.commit() .map_err(|error| anyhow::anyhow!("committing run event: {error}"))?; - // Same rationale as the full-snapshot path: keep the WAL bounded so a - // runner-event burst cannot stall the next commit behind a giant - // checkpoint sync. - checkpoint_wal(&connection)?; + // Same rationale as the full-snapshot path: bound the WAL so a + // runner-event burst cannot stall a later commit behind a giant + // checkpoint sync — periodically, not on every event. + self.maybe_checkpoint_wal(&connection)?; Ok(()) } @@ -1445,7 +1474,7 @@ impl SqliteStore { )?; tx.commit() .map_err(|error| anyhow::anyhow!("committing workflow run counter: {error}"))?; - checkpoint_wal(&connection)?; + self.maybe_checkpoint_wal(&connection)?; Ok(()) } @@ -1455,7 +1484,7 @@ impl SqliteStore { self.write_meta_tx(&tx, meta)?; tx.commit() .map_err(|error| anyhow::anyhow!("committing metadata: {error}"))?; - checkpoint_wal(&connection)?; + self.maybe_checkpoint_wal(&connection)?; Ok(()) } @@ -1602,7 +1631,7 @@ impl SqliteStore { self.insert_event_tx(&tx, event)?; tx.commit() .map_err(|error| anyhow::anyhow!("committing control event: {error}"))?; - checkpoint_wal(&connection)?; + self.maybe_checkpoint_wal(&connection)?; Ok(()) } From d0215e2ebab0d6dc8153dec415442d2c3f2c6fcc Mon Sep 17 00:00:00 2001 From: Bill Njoroge Date: Wed, 19 Aug 2026 22:54:28 -0400 Subject: [PATCH 4/4] fix(orchestrator): dockerd hook exits nonzero when the retry also fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the dockerd-restart hardening: after resetting the data root, the second start_dockerd attempt no longer swallows failure with exit 0. It now exits 1 so the provision path logs the failed startup (the caller already treats docker start as non-fatal — only container jobs depend on it — so non-container jobs still proceed, but the failure is visible). --- crates/preloop-orchestrator/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/preloop-orchestrator/src/lib.rs b/crates/preloop-orchestrator/src/lib.rs index 0a6013b7..45032cb7 100644 --- a/crates/preloop-orchestrator/src/lib.rs +++ b/crates/preloop-orchestrator/src/lib.rs @@ -1334,8 +1334,9 @@ fn docker_start_command() -> Vec { }}; \ if start_dockerd; then exit 0; fi; \ rm -rf {DOCKER_DATA_ROOT}/*; \ - start_dockerd; \ - exit 0" + if start_dockerd; then exit 0; fi; \ + echo 'dockerd failed to start after data-root reset' >&2; \ + exit 1" )), ] }