From d942cdb32802060c374936b91c91b50eba5aa750 Mon Sep 17 00:00:00 2001 From: FAZuH Date: Wed, 30 Sep 2026 12:52:50 +0000 Subject: [PATCH 1/3] test: make the Docker harness work on a NixOS host Both Docker integration targets failed on this host: 15 of 34 natmap_docker tests, and every auto_discover test that execs the binary. The host-built lab-ops links libssl.so.3 from a Nix store path, and its RUNPATH points at a toolchain directory that does not contain OpenSSL, so the binary cannot resolve the library on its own. The /nix mount added in 7c943da fixed the ELF interpreter, not the library. Set the loader path for the binary alone, via a two-line sh wrapper bind-mounted over /usr/local/bin/lab-ops that execs the real binary. Setting LD_LIBRARY_PATH for the whole container is the obvious fix and is wrong: nix libcrypto carries a RUNPATH into nix glibc, so it drags nix libdl into the image's own curl, which then fails against the image's glibc. That attempt turned 34 passing auto_discover tests into 34 failures. Scoping it to one process leaves everything else alone, and exec keeps the wrapper out of the process tree so the test scripts' kill %1 cleanup still works. The store path is read from OPENSSL_LIB_DIR rather than hardcoded, so a nix upgrade does not silently break it, and the whole Nix block stays behind one check that is inert off NixOS. Full suite green: 436 passed, 0 failed across 10 targets. Also found, not fixed: the Docker harness leaks containers on failure. teardown() only runs on the success path, so any failure leaves containers named it-* behind and the next run fails at name conflict while masking the original error. A Drop guard or a startup cleanup would make these self-healing. --- tests/auto_discover/mod.rs | 44 ++++++++++++++++++++++++++++----- tests/natmap_docker.rs | 50 +++++++++++++++++++++++++++++--------- 2 files changed, 77 insertions(+), 17 deletions(-) diff --git a/tests/auto_discover/mod.rs b/tests/auto_discover/mod.rs index e705eb2..7119d58 100644 --- a/tests/auto_discover/mod.rs +++ b/tests/auto_discover/mod.rs @@ -8,7 +8,9 @@ mod recovery; mod registration; mod startup_race; +use std::os::unix::fs::PermissionsExt; use std::path::Path; +use std::path::PathBuf; use std::process::Command; use std::sync::Once; @@ -41,6 +43,28 @@ fn setup_image() -> &'static str { image_name } +/// A NixOS host links lab-ops against a loader and an OpenSSL under +/// /nix/store that the test image lacks, so the binary cannot exec. +/// Mounting /nix fixes the loader, but the lib still needs to be on the +/// loader path, and setting LD_LIBRARY_PATH for the whole container +/// shadows the image's own OpenSSL-linked tools: nix libcrypto's RUNPATH +/// pulls nix glibc's libdl into curl, which then fails against the +/// image's glibc. So put the path on a wrapper around the binary alone. +/// Returns the wrapper to bind-mount over lab-ops, or None off NixOS. +fn nix_wrapper(label: &str) -> Option { + if !Path::new("/nix").is_dir() { + return None; + } + let lib_dir = std::env::var("OPENSSL_LIB_DIR").ok()?; + let wrapper = std::env::temp_dir().join(format!("lab-ops-{label}-wrapper.sh")); + let script = format!( + "#!/bin/sh\nexec env LD_LIBRARY_PATH={lib_dir} /usr/local/bin/lab-ops.bin \"$@\"\n" + ); + std::fs::write(&wrapper, script).ok()?; + std::fs::set_permissions(&wrapper, std::fs::Permissions::from_mode(0o755)).ok()?; + Some(wrapper) +} + pub(crate) fn run(script: &str) -> String { let image = setup_image(); let binary_path = env!("CARGO_BIN_EXE_lab-ops"); @@ -50,18 +74,26 @@ pub(crate) fn run(script: &str) -> String { "--rm", "--privileged", "-v", - &format!("{binary_path}:/usr/local/bin/lab-ops"), - "-v", "/var/run/docker.sock:/var/run/docker.sock", "-e", "NATMAP_SOCKET=/tmp/natmap.sock", "-e", "CONSUL_HTTP_ADDR=http://127.0.0.1:8500", ]); - // A NixOS host links lab-ops against a loader under /nix/store, which the - // test image lacks, so the binary cannot exec. No-op elsewhere. - if Path::new("/nix").is_dir() { - cmd.args(["-v", "/nix:/nix:ro"]); + match nix_wrapper("auto-discover-docker") { + Some(wrapper) => { + cmd.args([ + "-v", + "/nix:/nix:ro", + "-v", + &format!("{binary_path}:/usr/local/bin/lab-ops.bin"), + "-v", + &format!("{}:/usr/local/bin/lab-ops:ro", wrapper.display()), + ]); + } + None => { + cmd.args(["-v", &format!("{binary_path}:/usr/local/bin/lab-ops")]); + } } cmd.args([image, "sh", "-c"]); cmd.arg(script); diff --git a/tests/natmap_docker.rs b/tests/natmap_docker.rs index e79ae3f..cf1484a 100644 --- a/tests/natmap_docker.rs +++ b/tests/natmap_docker.rs @@ -1,6 +1,8 @@ #[cfg(feature = "docker-tests")] mod natmap_docker { + use std::os::unix::fs::PermissionsExt; use std::path::Path; + use std::path::PathBuf; use std::process::Command; use std::sync::Once; @@ -31,21 +33,47 @@ mod natmap_docker { image_name } + /// A NixOS host links lab-ops against a loader and an OpenSSL under + /// /nix/store that the test image lacks, so the binary cannot exec. + /// Mounting /nix fixes the loader, but the lib still needs to be on the + /// loader path, and setting LD_LIBRARY_PATH for the whole container + /// shadows the image's own OpenSSL-linked tools: nix libcrypto's RUNPATH + /// pulls nix glibc's libdl into curl, which then fails against the + /// image's glibc. So put the path on a wrapper around the binary alone. + /// Returns the wrapper to bind-mount over lab-ops, or None off NixOS. + fn nix_wrapper(label: &str) -> Option { + if !Path::new("/nix").is_dir() { + return None; + } + let lib_dir = std::env::var("OPENSSL_LIB_DIR").ok()?; + let wrapper = std::env::temp_dir().join(format!("lab-ops-{label}-wrapper.sh")); + let script = format!( + "#!/bin/sh\nexec env LD_LIBRARY_PATH={lib_dir} /usr/local/bin/lab-ops.bin \"$@\"\n" + ); + std::fs::write(&wrapper, script).ok()?; + std::fs::set_permissions(&wrapper, std::fs::Permissions::from_mode(0o755)).ok()?; + Some(wrapper) + } + fn run_in_docker(args: &[&str]) -> String { let image = setup_docker_image(); let binary_path = env!("CARGO_BIN_EXE_lab-ops"); let mut cmd = Command::new("docker"); - cmd.args([ - "run", - "--rm", - "--privileged", - "-v", - &format!("{binary_path}:/usr/local/bin/lab-ops"), - ]); - // A NixOS host links lab-ops against a loader under /nix/store, which - // the test image lacks, so the binary cannot exec. No-op elsewhere. - if Path::new("/nix").is_dir() { - cmd.args(["-v", "/nix:/nix:ro"]); + cmd.args(["run", "--rm", "--privileged"]); + match nix_wrapper("natmap-docker") { + Some(wrapper) => { + cmd.args([ + "-v", + "/nix:/nix:ro", + "-v", + &format!("{binary_path}:/usr/local/bin/lab-ops.bin"), + "-v", + &format!("{}:/usr/local/bin/lab-ops:ro", wrapper.display()), + ]); + } + None => { + cmd.args(["-v", &format!("{binary_path}:/usr/local/bin/lab-ops")]); + } } cmd.args([image, "sh", "-c"]); From cf70f33e2fb5eff87f19d1213916b54840751b7a Mon Sep 17 00:00:00 2001 From: FAZuH Date: Wed, 30 Sep 2026 12:52:44 +0000 Subject: [PATCH 2/3] docs: let the agent run the full suite, and record the NixOS build wall The test-strategy section told the agent never to run ./dev.sh all or ./dev.sh test without asking, which made every session ship with the Docker integration suite unverified. The maintainer now allows it. The preference for targeted runs stays, since it is still the faster path and most changes do not touch the Docker paths. The same section's worked example was `cargo test -p natmap`, which fails: the package is lab-ops_natmap. The file contradicted itself fifteen lines later. Fixed. Adds a Build environment section, because openssl-sys cannot find OpenSSL on this host and every cargo command dies before it reaches the code. Both store paths are required, and they are not interchangeable: OPENSSL_DIR alone fails, because the -dev path holds the headers and the other holds the .so files. --- AGENTS.md | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1ed5254..96fcaaa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,12 +15,28 @@ Personal homelab utility tools. Rust workspace, edition **2024**. - **`rustfmt` requires nightly** (`+nightly`). `rustfmt.toml` uses unstable features (`imports_granularity = "Item"`, `group_imports = "StdExternalCrate"`). - **`.cargo/config.toml` always enables `docker-tests`** via `--cfg feature="docker-tests"`. So `--all-features` in dev commands is redundant but harmless. +### Build environment (NixOS host) + +`openssl-sys` cannot find OpenSSL here, so **every** cargo command needs +these exported first, in the same shell as the cargo call: + +```bash +export OPENSSL_LIB_DIR=/nix/store/7fr737xfi9qw3fzvdsqmnbqid56knndp-openssl-3.6.4/lib +export OPENSSL_INCLUDE_DIR=/nix/store/0la6k2nj90y1716c1znhdm713ia1qgx8-openssl-3.6.4-dev/include +``` + +- `OPENSSL_DIR` alone is **not** enough: the `-dev` store path has the headers, the other has the `.so` files, and `openssl-sys` rejects a libdir without them. +- `cargo test -p lab-ops_auto-discover` additionally needs + `LD_LIBRARY_PATH=/nix/store/7fr737xfi9qw3fzvdsqmnbqid56knndp-openssl-3.6.4/lib` + or the test binary dies with `libssl.so.3: cannot open shared object file`. The other three crates do not need it. +- Do not "fix" this in `Cargo.toml` — it is the environment, not the code. + ## Test Strategy ⚠️ -- **Run ONLY relevant tests first.** After a change, run the specific test/crate, not the full suite. E.g. `cargo test -p natmap`, `cargo test -p auto-discover`. +- **Run targeted tests first.** After a change, run the specific test/crate, not the whole suite — it is faster and most changes do not touch the Docker paths. E.g. `cargo test -p lab-ops_natmap`, `cargo test -p lab-ops_auto-discover`. - **If a test fails, fix and rerun only that test.** Use `cargo test -p `. -- **⚠️ `./dev.sh all` runs the FULL suite including Docker integration tests (122+ tests, ~2-3 min).** Do NOT run `./dev.sh all` or `./dev.sh test` without asking the user first. Notify the user and let them trigger it themselves. -- **Docker tests require `--test-threads=1`** (enforced by `.cargo/config.toml`). Each test spins up a privileged Ubuntu container with iptables. +- **⚠️ The full suite may be run without asking.** `./dev.sh all` / `./dev.sh test` run the FULL suite including the Docker integration tests (122+ tests, ~2-3 min) — run them before calling work verified when a change touches the Docker harness, the iptables path, the natmap state file, or the auto_discover bootstrap. +- **Nothing enforces `--test-threads=1`.** `.cargo/config.toml` sets only `rustflags`, so the Docker suites run with cargo's default parallelism. Pass `-- --test-threads=1` yourself when you need determinism; they are currently flaky under parallel execution (#54). Each test spins up a privileged Ubuntu container with iptables. ### Quick Test Commands From 7e850e3998aa064db7daae3d2fccb1d2f978c61a Mon Sep 17 00:00:00 2001 From: FAZuH Date: Wed, 30 Sep 2026 14:22:29 +0000 Subject: [PATCH 3/3] test: sweep leaked it-* containers before the suite starts teardown() returns a shell fragment that the tests embed at the end of their script, so a test that hits `exit 1` never reaches it. The test container mounts /var/run/docker.sock, so the it-* container the test created lives on the host and survives the run. The next run then dies at `docker run --name` with "Conflict. The container name ... is already in use", which reports a name collision instead of the real failure. That is not hypothetical. A 34-failure run left 21 containers behind and every later run failed the same way until they were removed by hand; afterwards the same code passed 47/47 untouched. Sweep anchored `^it-` containers once, in the existing INIT.call_once block, before the image build. Once blocks every other test until the closure returns, so the sweep finishes before any test container starts and can only ever remove a leftover from a previous run. It must not run per test: this suite is not single-threaded, and a per-test sweep would delete containers belonging to tests running at that moment, which is worse than the leak it fixes. The anchor matters too. Docker's name filter is a substring match, so a loose `it-` would also reach names like `audit-it-decoy`. The sweep is best effort and stays quiet when there is nothing to remove, so it can never be the reason the suite goes red. natmap_docker needs no equivalent: it never passes docker --name, so it has no leak to sweep. Verified by seeding it-nodomain, the name that actually masks a real failure: the sweep removed it and the suite passed 47/47. With that same leftover on the pristine tree, registration::service_id_no_domain_falls_back _to_name reports the Conflict error instead. Breaking that test's expected prefix on purpose still surfaces the original assertion failure, not a Conflict. Full suite: 442 passed, 0 failed across 10 targets. --- tests/auto_discover/mod.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/auto_discover/mod.rs b/tests/auto_discover/mod.rs index 7119d58..ac5ce81 100644 --- a/tests/auto_discover/mod.rs +++ b/tests/auto_discover/mod.rs @@ -16,9 +16,42 @@ use std::sync::Once; static INIT: Once = Once::new(); +/// A test that hits `exit 1` never reaches the `teardown()` fragment appended to +/// its script, so its `it-*` container survives on the host and the next run dies +/// at `docker run --name` with a conflict that masks the real failure. Anchored +/// `^it-` so a loose match cannot reach names like `audit-it-decoy`. Best effort: +/// a missing or unhappy `docker` must never turn the suite red. +fn sweep_leaked_containers() { + let Ok(list) = Command::new("docker") + .args(["ps", "-aq", "--filter", "name=^it-"]) + .output() + else { + return; + }; + let ids: Vec = String::from_utf8_lossy(&list.stdout) + .lines() + .map(str::trim) + .filter(|id| !id.is_empty()) + .map(String::from) + .collect(); + if ids.is_empty() { + return; + } + eprintln!( + "sweeping {} leaked it-* test container(s) from a previous run: {}", + ids.len(), + ids.join(" ") + ); + let _ = Command::new("docker") + .args(["rm", "-f"]) + .args(&ids) + .status(); +} + fn setup_image() -> &'static str { let image_name = "lab-ops-auto-discover-test:latest"; INIT.call_once(|| { + sweep_leaked_containers(); let dockerfile = concat!( "FROM ubuntu:24.04\n", "RUN apt-get update && apt-get install -y iptables jq curl unzip iproute2 docker.io\n",