auto_discover Docker suite is flaky: 44-47 of 47 across identical runs

[FAZuH]

Found while verifying PR #52. Pre-existing, not caused by the harness fix, and confirmed by re-running the identical command on the pristine tree with the change stashed.

The observation

Across repeated runs of the same command, on the same tree, with no code change, the pass count varies: 47/47, then 46/47, then 44/47, with a different random subset failing each time. The most frequently hit is registration::container_id_in_consul_meta. The failures are not always the same tests, which points at shared state or timing rather than one broken assertion.

Why it matters

The suite is the only thing covering the auto_discover Docker paths, and its green is currently luck. Any claim that it passes is worth less than it looks on a given run. This is the same class of problem as the 19 vacuous tests in #44: a suite whose green is not earned.

Likely cause, not yet confirmed

docs/dev/testing.md and AGENTS.md both assert the Docker tests are single-threaded via --test-threads=1 "enforced by .cargo/config.toml". That enforcement does not exist. .cargo/config.toml contains only rustflags; there is no test-threads setting anywhere in the repo, and the host has 4 CPUs, so cargo's default parallelism applies. The suite has therefore been running tests concurrently all along, while sharing:

• a single Consul HTTP port, 8500, reached through the mounted Docker socket
• one shared image tag, lab-ops-auto-discover-test:latest
• one fixed config path, /tmp/discovery.yaml, inside every test script

Any of those is enough to make concurrent tests interfere. The fixed /tmp/discovery.yaml lives inside the per-test container, so that one is probably fine; the shared Consul port and the shared image are not obviously so.

What to build

Decide the concurrency question first, then make the suite deterministic.

• Establish whether concurrent execution is intended. If not, actually enforce it, and re-measure. Note that .cargo/config.toml cannot enforce it: cargo has no test-threads key, so it belongs in the test invocation or CI. If concurrency is intended, the shared resources need namespacing per test.
• Mind the ordering against the other test tickets: Make the unit tests hermetic: no real iptables, Docker, or shared /tmp #44 and Deduplicate the auto_discover shell bootstraps and poll instead of sleeping #47 both aim to make this suite faster, and 36 fixed sleep 4 waits are on the list to become pollers. Making a flaky parallel suite faster is the wrong order. Fix determinism first.

Acceptance criteria

• Ten consecutive runs of cargo test -p lab-ops --test auto_discover --all-features on an unchanged tree all report the same result
• The concurrency setting is real, lives in the repo rather than in a user-level cargo config, and is documented
• AGENTS.md and docs/dev/testing.md no longer claim .cargo/config.toml enforces --test-threads=1
• Not "fixed" by adding sleeps or retries. A flaky test is worse than no test, and papering over it hides the real interference.

Blocked by

None (can start immediately).

The --all-features question in #53 applies here too: on this host it is required, or the run reports 0 tests and exits green.

[FAZuH]

New evidence, and a hypothesis about the cause that changes what is worth doing.

A third failure mode, now with a name

registration::event_loop_logs_structured_fields failed once in two consecutive full-workspace runs on v0.2, panicking at tests/auto_discover/mod.rs:139 — the test container exits non-zero.

It is load-dependent, not deterministic:

• the box carried load average 8.66 / 11.50 at the time
• that run took 123.88s against ~31s for the same target in isolation
• the test passes 4/4 in isolation under that same load
• it only fails when all 10 test binaries compete

So there are now two distinct flakes on record: concurrent_starts_all_registered (#47, 1 in 9) and this one. Same signature, different test.

The hypothesis worth testing before anything else

#47 replaced 160 fixed sleeps with bounded polls. On an idle box a bounded poll is strictly better than a sleep: it returns as soon as the condition holds, and it fails loudly rather than silently asserting too early.

Under load it is strictly worse in one specific way. A sleep 10 that overruns costs nothing but time. A wait_for_service_count(name, 2, 30) that expires at 30s fails the test, even though the condition would have been true at 45s. The bounds were tuned on a quiet machine.

That predicts exactly what we see: failures only when the whole workspace runs concurrently, never in isolation. It also predicts they should cluster in the tests with the tightest bound relative to their work — the sleep 10 → count >= 5, bound 40s conversion is the most exposed.

I have not confirmed this. It is a hypothesis, and the last time I asserted an unverified mechanism it cost a correction. The check is cheap: run the full suite at the recorded load and see whether raising the exposed bounds makes it green, or whether these tests fail for a reason unrelated to the bound.

What I am not doing

Not attempting the fix, and not adding a --test-threads=1 or RUST_TEST_THREADS setting. AGENTS.md was corrected to record that nothing in the committed tree enforces single-threaded execution — .cargo/config.toml sets only rustflags — so a fix built on that assumption would be fixing a mechanism that does not exist. Whatever this ends up being, it has to be a real mechanism.

Useful measurement that does not exist yet

Every failure so far has been observed on a loaded box with no load recorded alongside the result. Capture /proc/loadavg with every run — before fixing anything, the correlation between load and failure is the only evidence that would settle the bounded-poll hypothesis over a retry-count or sleep-length story.

[FAZuH]

Third distinct flake, and the one that matters most for diagnosis.

New observation

preserve_src_ip::preserve_src_ip_false_no_policy_route failed once during the #41 pass with:

sh: 15: lab-ops: not found
FAIL: natmap socket never appeared

Load average at that moment was 0.98 2.82 2.96 — the 1-minute figure low, the 5-minute figure still elevated from the Docker suites. It passed in isolation and on both subsequent full runs, at 2.05 2.83 2.96 and 3.39 3.09 3.04.

So there are now three separate tests on record, all failing only under concurrent load and passing in isolation:

test	signature	when
concurrent_starts_all_registered	reported during #47	1 in 9
registration::event_loop_logs_structured_fields	container exit non-zero at mod.rs:139	1 in 2, at load 8.66
preserve_src_ip_false_no_policy_route	lab-ops: not found, socket never appeared	1 in 3, at load 0.98/2.82

What the third one points at, which is a different mechanism

sh: 15: lab-ops: not found is not a slow poll. It is PATH. The binary the container is trying to exec was not on PATH at the moment the script reached line 15, while the same harness succeeds moments later in the same run.

That is not a timeout and no bound fixes it. It is either a PATH that the Nix wrapper sets conditionally, or a race on a symlink or mount the container sets up before exec. It is a distinct failure mode from the two bounded-poll timeouts, and lumping all three under "bounded polls are too tight under load" would be wrong.

I am recording it rather than chasing it, because a correct fix here needs the three separated first, and separating them means running the suite enough times to get a rate per test. That is a longer job than this batch has room for.

One measurement change that has already paid for itself

Every full-suite result in this batch now carries /proc/loadavg beside it. That single change is what let me say "the third flake is a PATH problem, not a timeout" instead of guessing. The earlier two observations had no load recorded and remain ambiguous.

The cost so far

One full-suite run has been lost to ambiguity, two extra full runs spent naming a failure, and one ticket (#47) merged with a criterion unmet because the timing numbers it was measured against were distorted by host load.

[FAZuH]

A fourth flake, and the only one I could not name.

What was observed

While verifying #44, lab-lib --lib reported 13 passed; 1 failed twice, both times in a shell that had just run two other cargo test invocations back to back. One of the two runs had DOCKER_HOST=unix:///nonexistent.sock set, the other had a logging-shim directory first on PATH.

I could not name the failing test. It did not reproduce across five subsequent attempts: three in isolation, three with the shim, two with the two other suites run first, and a full ./dev.sh test. Every one of those was 14 passed; 0 failed.

Why it is recorded rather than dismissed

A flake I saw twice and cannot reproduce is still a flake. Two runs is not a pattern, but it is also not nothing, and the cost of it being real is a green suite that is not.

The one hypothesis, unconfirmed

crates/lab-lib/src/port.rs contains released_port_is_immediately_reallocatable, which carries a load guard — it is the test #41 shortened from seven lines to four while keeping the "load check, not reproduction" distinction. A test that deliberately fails under machine load would explain both observations and the non-reproduction, because the two failing runs were the only two where the box had just done three suites' worth of work.

I have not confirmed this and am not treating it as the cause. It is a guess about which test it was, offered so the next person knows where to look first. If it is that test, the defect is not a flake at all: a load guard that reports red under load is indistinguishable from a real failure, and the fix is to make the guard report SKIP rather than FAIL — which is the same problem #44 just fixed for the ip6tables test on a different path.

What would settle it

Run lab-lib --lib in a loop under load until it goes red, with --nocapture. That is a longer job than this batch has room for, and it is the natural next piece of work on this issue.