diff --git a/bench/CLAUDE.md b/bench/CLAUDE.md index f5827e3..e8a4d2a 100644 --- a/bench/CLAUDE.md +++ b/bench/CLAUDE.md @@ -133,7 +133,9 @@ A benchmark run is a point in time, not a fixed rig. Nothing is pinned: competit Three resolution details are easy to get wrong. skopeo's module path is `go.podman.io/skopeo` since v1.23.0, and the old `github.com/containers/skopeo` path still serves tags whose `go.mod` declares the new path, so installing from it fails on a path mismatch rather than a missing version. `@latest` never crosses a major version boundary under semantic import versioning, so when one of these tags v2 the install silently sticks on the highest v1 until the path gains a `/v2` suffix. And dregsy tags releases without a `v` prefix, so the proxy reads no valid semver and `@latest` resolves to a pseudo-version off the default branch rather than to a release. -Versions are read from the binaries rather than assumed. `go install` sets none of the ldflags a release build uses, so the ECR credential helper reports `development` and dregsy has no version flag at all. Both are recovered with `go version -m`, which reads the module version stamped into the binary. +Versions are read from the binaries rather than assumed. `go install` sets none of the ldflags a release build uses, so the ECR credential helper reports `development` and dregsy has no version flag at all. Both are recovered with `go version -m`, which reads the module version stamped into the binary. That can still come up empty, for a binary built from a working tree (Go stamps `(devel)`, which names no release) or where Go is absent, and the record then reads `unknown` with the reason on stderr. Treat an `unknown` as a run whose provenance was lost rather than as a harness bug. + +The bootstrap log is `/var/log/user-data.log` on the instance, not `cloud-init-output.log`: `user-data.sh` redirects its own output there. dregsy transfers nothing itself. Its config sets `relay: skopeo`, so skopeo moves every byte dregsy is credited with, and the run record captures skopeo's version alongside dregsy's for that reason. diff --git a/bench/terraform/aws/user-data.sh b/bench/terraform/aws/user-data.sh index 2723cbb..87d2e17 100644 --- a/bench/terraform/aws/user-data.sh +++ b/bench/terraform/aws/user-data.sh @@ -17,10 +17,36 @@ if ! swapon --show | grep -q '/swapfile'; then echo '/swapfile none swap sw 0 0' >> /etc/fstab fi +# ── Retry helper ────────────────────────────────────────────────────────────── + +# Every network fetch below goes through this. Exhausting the attempts is a +# hard failure: nothing waits on cloud-init, so `terraform apply` reports +# success either way, and continuing past a failed fetch turns a clear error +# here into a confusing missing-binary error when bench-remote runs later. +retry() { + local what="$1" + shift + local attempt + for attempt in 1 2 3; do + if "$@"; then + return 0 + fi + if [ "$attempt" -lt 3 ]; then + echo " $what attempt $attempt failed, retrying in 15s..." + sleep 15 + fi + done + echo "ERROR: $what failed after 3 attempts, aborting bootstrap" >&2 + return 1 +} + # ── System packages ─────────────────────────────────────────────────────────── -dnf update -y -dnf install -y \ +# A cold instance's first mirror contact is the most transient-failure-prone +# fetch here, and gpgme-devel and openssl-devel are what skopeo's cgo build +# needs later. +retry "dnf update" dnf update -y +retry "dnf install" dnf install -y \ git \ cmake \ gcc \ @@ -47,17 +73,15 @@ chown ec2-user:ec2-user /home/ec2-user/.bashrc echo "--- Installing Rust via rustup (as ec2-user)" -for attempt in 1 2 3; do - if su - ec2-user -c 'curl --proto "=https" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --no-modify-path'; then - break - fi - echo " rustup attempt $attempt failed, retrying in 15s..." - sleep 15 -done +# pipefail is set inside the su, not inherited from this shell: without it the +# pipeline's status is the installer's, and curl failing feeds it EOF, which it +# exits zero on. retry would then report success having installed nothing. +retry rustup su - ec2-user -c 'set -o pipefail; curl --proto "=https" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --no-modify-path' su - ec2-user -c 'echo "export PATH=\"\$HOME/.cargo/bin:\$PATH\"" >> ~/.bashrc' -echo "Rust: $(su - ec2-user -c '/home/ec2-user/.cargo/bin/rustc --version')" +RUSTC_VERSION="$(su - ec2-user -c '/home/ec2-user/.cargo/bin/rustc --version')" +echo "Rust: $${RUSTC_VERSION}" # ── Go ──────────────────────────────────────────────────────────────────────── @@ -65,13 +89,7 @@ echo "--- Installing Go 1.26.x" GO_VERSION="1.26.2" GO_ARCHIVE="go$${GO_VERSION}.linux-amd64.tar.gz" -for attempt in 1 2 3; do - if curl -fsSL "https://go.dev/dl/$${GO_ARCHIVE}" -o "/tmp/$${GO_ARCHIVE}"; then - break - fi - echo " Go download attempt $attempt failed, retrying in 15s..." - sleep 15 -done +retry "Go download" curl -fsSL "https://go.dev/dl/$${GO_ARCHIVE}" -o "/tmp/$${GO_ARCHIVE}" tar -C /usr/local -xzf "/tmp/$${GO_ARCHIVE}" rm -f "/tmp/$${GO_ARCHIVE}" @@ -86,7 +104,8 @@ EOF chown ec2-user:ec2-user /home/ec2-user/.bashrc -echo "Go: $(go version)" +GO_VERSION_LINE="$(go version)" +echo "Go: $${GO_VERSION_LINE}" # Go env for root builds (dregsy, regsync, skopeo, ecr-credential-helper) export HOME=/root GOPATH=/root/go GOCACHE=/root/.cache/go-build @@ -101,7 +120,7 @@ export PATH="/usr/local/go/bin:$GOPATH/bin:$PATH" # ── ECR credential helper ──────────────────────────────────────────────────── echo "--- Installing ECR credential helper" -go install github.com/awslabs/amazon-ecr-credential-helper/ecr-login/cli/docker-credential-ecr-login@latest +retry "ecr-credential-helper install" go install github.com/awslabs/amazon-ecr-credential-helper/ecr-login/cli/docker-credential-ecr-login@latest cp /root/go/bin/docker-credential-ecr-login /usr/local/bin/ mkdir -p /home/ec2-user/.docker @@ -114,7 +133,8 @@ chown -R ec2-user:ec2-user /home/ec2-user/.docker # the only version several of these report. The credential helper leaves its # own version at "development" because go install sets none of the ldflags its # Makefile uses, and dregsy has no version flag at all. -echo "ecr-credential-helper: $(go version -m /usr/local/bin/docker-credential-ecr-login | awk '$1 == "mod" { print $3; exit }')" +ECR_HELPER_MOD="$(go version -m /usr/local/bin/docker-credential-ecr-login | grep -m1 -E '^[[:space:]]*mod[[:space:]]' || echo 'build info unavailable')" +echo "ecr-credential-helper: $${ECR_HELPER_MOD}" # ── skopeo (dregsy transfer backend) ───────────────────────────────────────── @@ -122,12 +142,13 @@ echo "--- Installing skopeo" # skopeo moved its module path to go.podman.io/skopeo at v1.23.0. The github.com # path still serves tags, but their go.mod declares the new path, so installing # from it fails on a module path mismatch rather than a missing version. -CGO_ENABLED=1 go install \ +retry "skopeo install" env CGO_ENABLED=1 go install \ -tags "exclude_graphdriver_btrfs containers_image_openpgp" \ go.podman.io/skopeo/cmd/skopeo@latest cp /root/go/bin/skopeo /usr/local/bin/skopeo -echo "skopeo: $(skopeo --version 2>&1)" +SKOPEO_VERSION_LINE="$(skopeo --version)" +echo "skopeo: $${SKOPEO_VERSION_LINE}" # ── dregsy ──────────────────────────────────────────────────────────────────── @@ -135,18 +156,20 @@ echo "--- Installing dregsy" # dregsy tags releases without the v prefix, so the module proxy cannot read # them as versions and @latest resolves to a pseudo-version off the default # branch rather than to a release. -go install github.com/xelalexv/dregsy/cmd/dregsy@latest +retry "dregsy install" go install github.com/xelalexv/dregsy/cmd/dregsy@latest cp /root/go/bin/dregsy /usr/local/bin/dregsy -echo "dregsy: $(go version -m /usr/local/bin/dregsy | awk '$1 == "mod" { print $3; exit }')" +DREGSY_MOD="$(go version -m /usr/local/bin/dregsy | grep -m1 -E '^[[:space:]]*mod[[:space:]]' || echo 'build info unavailable')" +echo "dregsy: $${DREGSY_MOD}" # ── regsync ─────────────────────────────────────────────────────────────────── echo "--- Installing regsync" -go install github.com/regclient/regclient/cmd/regsync@latest +retry "regsync install" go install github.com/regclient/regclient/cmd/regsync@latest cp /root/go/bin/regsync /usr/local/bin/regsync -echo "regsync: $(regsync version 2>&1 || true)" +REGSYNC_VERSION_LINE="$(regsync version)" +echo "regsync: $${REGSYNC_VERSION_LINE}" # ── Clean up Go build cache ────────────────────────────────────────────────── @@ -156,17 +179,37 @@ rm -rf /root/go/pkg/mod/cache /root/.cache/go-build echo "--- Cloning and building ocync (as ec2-user)" +# Cloning is separate from building so it can retry. It clones to a scratch +# path and moves it into place, so a retry after a partial clone is idempotent +# without ever deleting an existing checkout: that path holds the git-tracked +# run-record archive, and re-running this script through `cloud-init clean` +# is a normal way to retry a bootstrap. The build is not retried, since a +# compile failure is deterministic and cargo retries its own fetches. +retry "ocync clone" su - ec2-user -c ' + set -e + rm -rf $HOME/.ocync-clone + git clone https://github.com/clowdhaus/ocync.git $HOME/.ocync-clone + [ -e $HOME/ocync ] || mv $HOME/.ocync-clone $HOME/ocync + rm -rf $HOME/.ocync-clone +' + +# set -e inside the block: without it only the last command's status escapes, +# so a failed cargo build would be masked by the cp that follows it. su - ec2-user -c " + set -e source \$HOME/.cargo/env - git clone https://github.com/clowdhaus/ocync.git \$HOME/ocync cd \$HOME/ocync cargo build --release --package ocync --package bench-proxy cp target/release/ocync \$HOME/.cargo/bin/ocync cp target/release/bench-proxy \$HOME/.cargo/bin/bench-proxy " -echo "ocync: $(su - ec2-user -c '/home/ec2-user/.cargo/bin/ocync version')" -echo "bench-proxy: built" +# Assigned rather than echoed inline: a command substitution inside an echo +# argument yields echo's status, so set -e can never fire on a failed probe. +OCYNC_VERSION="$(su - ec2-user -c '/home/ec2-user/.cargo/bin/ocync version')" +echo "ocync: $${OCYNC_VERSION}" +BENCH_PROXY_PATH="$(su - ec2-user -c 'command -v /home/ec2-user/.cargo/bin/bench-proxy')" +echo "bench-proxy: $${BENCH_PROXY_PATH}" # ── Generate bench-proxy CA and install into system trust store ────────────── diff --git a/xtask/src/bench/mod.rs b/xtask/src/bench/mod.rs index 7e0fde9..39dfb2b 100644 --- a/xtask/src/bench/mod.rs +++ b/xtask/src/bench/mod.rs @@ -299,12 +299,20 @@ pub(crate) async fn run(args: BenchArgs) -> Result<(), Box = BTreeMap::new(); let mut relay_versions: BTreeMap = BTreeMap::new(); for &tool in &tools { - let version = runner::check_tool(tool).await?; + let version = runner::check_tool(tool, workspace_root).await?; eprintln!(" {}: {version}", tool); tool_versions.insert(tool.to_string(), version); @@ -314,12 +322,6 @@ pub(crate) async fn run(args: BenchArgs) -> Result<(), Box/dev/null; then fi if [ ! -d ~/ocync ]; then - echo "bench-remote: error: ~/ocync not found after cloud-init. Check user-data logs:" - echo " ssh $USER@$(hostname -I | awk '{{print $1}}') 'sudo cat /var/log/cloud-init-output.log | tail -50'" + echo "bench-remote: error: ~/ocync not found after cloud-init. Check the bootstrap log:" + echo " ssh $USER@$(hostname -I | awk '{{print $1}}') 'sudo tail -50 /var/log/user-data.log'" exit 1 fi diff --git a/xtask/src/bench/runner.rs b/xtask/src/bench/runner.rs index 04c8cd1..fb4cba6 100644 --- a/xtask/src/bench/runner.rs +++ b/xtask/src/bench/runner.rs @@ -179,10 +179,16 @@ async fn probe_version( ) } +/// Version Go stamps into a binary built from a local working tree. +/// +/// It names no released artifact, so it is not a usable record of what ran. +const DEVEL_VERSION: &str = "(devel)"; + /// Extracts the module version from `go version -m` output. /// /// The build info lists one `mod` line holding the main module's path and -/// version, followed by a `dep` line per dependency. +/// version, followed by a `dep` line per dependency. Returns `None` for a +/// binary built from a working tree, whose stamped version identifies nothing. fn module_version_from_build_info(output: &str) -> Option<&str> { output.lines().find_map(|line| { let mut fields = line.split_whitespace(); @@ -190,30 +196,71 @@ fn module_version_from_build_info(output: &str) -> Option<&str> { return None; } fields.next()?; // module path - fields.next() + fields.next().filter(|v| *v != DEVEL_VERSION) }) } +/// Explains why a `go version -m` probe yielded no module version. +/// +/// The probe runs under `sh`, so a missing Go toolchain and a binary missing +/// from `PATH` both surface as a non-zero exit rather than a spawn failure. +/// The tool's own text separates them, so it is passed through verbatim. +fn no_module_version_reason(success: bool, stdout: &str, stderr: &str) -> String { + if success { + return "build info carries no released module version".to_string(); + } + + let detail = stderr + .lines() + .chain(stdout.lines()) + .find(|l| !l.trim().is_empty()) + .unwrap_or("no output") + .trim(); + format!("go version -m failed: {detail}") +} + /// Reads the module version stamped into a Go binary on `PATH`. /// -/// This is the only version several of these tools carry. `go install` sets -/// none of the ldflags a project's release build uses, so a tool that reports -/// its version from a linker-injected variable reports a placeholder, and -/// dregsy has no version flag at all. -async fn go_module_version(binary: &str) -> Option { +/// Returns the reason on failure rather than swallowing it, so a caller can +/// say what it recorded instead. Callers differ: one falls back to asking the +/// binary, the other records that no version is known. +async fn go_module_version(binary: &str) -> Result { let output = tokio::process::Command::new("sh") .arg("-c") .arg(format!("go version -m \"$(command -v {binary})\"")) .output() .await - .ok()?; + .map_err(|e| format!("could not run sh: {e}"))?; - if !output.status.success() { - return None; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + + match module_version_from_build_info(&stdout) { + Some(version) => Ok(version.to_string()), + None => Err(no_module_version_reason( + output.status.success(), + &stdout, + &stderr, + )), } +} - let stdout = String::from_utf8_lossy(&output.stdout); - module_version_from_build_info(&stdout).map(str::to_string) +/// Resolves the binary a tool is executed as. +/// +/// ocync runs from the workspace release build rather than `PATH`, since that +/// is what `build_ocync` just produced. A `PATH` lookup would find whatever +/// was installed when the instance was created, which can be months older, so +/// version probes must resolve the same way runs do or the record names a +/// binary that produced none of the numbers. +fn tool_path(tool: Tool, workspace_root: &Path) -> std::borrow::Cow<'static, str> { + match tool { + Tool::Ocync => workspace_root + .join("target/release/ocync") + .to_string_lossy() + .into_owned() + .into(), + Tool::Dregsy | Tool::Regsync => tool.binary().into(), + } } /// Runs a benchmarked tool's version probe. @@ -223,16 +270,23 @@ async fn go_module_version(binary: &str) -> Option { /// fallback keys off the probe reporting nothing rather than off a sentinel /// string, so a tool that prints the word `unknown` is still recorded as /// having reported it. -pub(crate) async fn check_tool(tool: Tool) -> Result { - if let Some(version) = - probe_version(tool.binary(), version_args(tool), reports_version(tool)).await? +pub(crate) async fn check_tool(tool: Tool, workspace_root: &Path) -> Result { + let binary = tool_path(tool, workspace_root); + + if let Some(version) = probe_version(&binary, version_args(tool), reports_version(tool)).await? { return Ok(version); } - Ok(go_module_version(tool.binary()) - .await - .unwrap_or_else(|| UNKNOWN_VERSION.to_string())) + match go_module_version(&binary).await { + Ok(version) => Ok(version), + Err(reason) => { + eprintln!( + "WARNING: no version for {binary}: {reason}. Recording \"{UNKNOWN_VERSION}\"." + ); + Ok(UNKNOWN_VERSION.to_string()) + } + } } /// Returns the version of every binary `tool` relays its transfers through. @@ -246,13 +300,20 @@ pub(crate) async fn check_relays(tool: Tool) -> Result, St // credential helper prints "development" whatever it was built from, // because `go install` sets none of its Makefile's ldflags. let version = match go_module_version(binary).await { - Some(version) => version, - // Still probe, so a missing binary fails here rather than midway - // through a run. A relay need not accept `--version`, so a - // non-zero exit is not itself an error. - None => probe_version(binary, &["--version"], false) - .await? - .unwrap_or_else(|| UNKNOWN_VERSION.to_string()), + Ok(version) => version, + Err(reason) => { + // Still probe, so a missing binary fails here rather than + // midway through a run. A relay need not accept `--version`, + // so a non-zero exit is not itself an error. Announced, + // because this is the weaker source and may be a placeholder. + eprintln!( + "WARNING: no module version for {binary}: {reason}. \ + Falling back to asking the binary." + ); + probe_version(binary, &["--version"], false) + .await? + .unwrap_or_else(|| UNKNOWN_VERSION.to_string()) + } }; versions.push((binary.to_string(), version)); } @@ -351,16 +412,7 @@ pub(crate) async fn run_tool( Tool::Regsync => vec!["once", "-c", &config_str], }; - // Use the workspace release binary for ocync (just built by build_ocync), - // PATH lookup for external tools (dregsy, regsync). - let binary: std::borrow::Cow<'_, str> = match tool { - Tool::Ocync => workspace_root - .join("target/release/ocync") - .to_string_lossy() - .into_owned() - .into(), - _ => tool.binary().into(), - }; + let binary = tool_path(tool, workspace_root); // For ocync, set OCYNC_TIMING_FILE so the engine writes phase timing JSONL. let timing_path = if tool == Tool::Ocync { @@ -608,6 +660,46 @@ mod tests { ); } + #[test] + fn failure_reason_passes_through_the_tools_own_text() { + // The probe runs under sh, so a missing Go toolchain and a binary + // missing from PATH both arrive as a non-zero exit. Only the tool's + // text tells them apart, so it must survive verbatim. + let missing_go = no_module_version_reason(false, "", "sh: go: command not found"); + assert!(missing_go.contains("command not found"), "{missing_go}"); + + let missing_binary = + no_module_version_reason(false, "", "stat : no such file or directory"); + assert!( + missing_binary.contains("no such file or directory"), + "{missing_binary}" + ); + + // Go writing its diagnostic to stdout must not read as "no output". + let on_stdout = no_module_version_reason(false, "some diagnostic", ""); + assert!(on_stdout.contains("some diagnostic"), "{on_stdout}"); + } + + #[test] + fn failure_reason_distinguishes_a_binary_without_build_info() { + let reason = no_module_version_reason(true, "", ""); + assert!( + reason.contains("no released module version"), + "a zero-exit probe with no mod line is not a probe failure: {reason}" + ); + } + + #[test] + fn locally_built_binary_reports_no_module_version() { + // Go stamps (devel) for a build from a working tree. It names no + // released artifact, so recording it would give a run's provenance the + // same authority as a real version. + let devel = "/usr/local/bin/dregsy: go1.26.2\n\ + \tmod\tgithub.com/xelalexv/dregsy\t(devel)\t\n"; + + assert_eq!(module_version_from_build_info(devel), None); + } + #[test] fn module_version_absent_without_a_mod_line() { // `go version -m` writes nothing to stdout for a non-Go binary, and @@ -622,6 +714,21 @@ mod tests { assert_eq!(module_version_from_build_info(deps_only), None); } + #[test] + fn ocync_is_probed_at_the_binary_the_run_executes() { + // A PATH lookup would find the ocync installed when the instance was + // built, which can be months older than the one just compiled, and + // the record would credit it with HEAD's numbers. + let root = Path::new("/w"); + assert_eq!( + tool_path(Tool::Ocync, root), + "/w/target/release/ocync", + "ocync must resolve to the workspace build, matching run_tool" + ); + assert_eq!(tool_path(Tool::Dregsy, root), "dregsy"); + assert_eq!(tool_path(Tool::Regsync, root), "regsync"); + } + #[test] fn relays_cover_every_binary_that_moves_a_tools_numbers() { // dregsy delegates every transfer to skopeo, and both Go tools reach