test: fix the fleet integration suite's flaky waits - #141
Merged
Conversation
A node reads `running` as soon as the engine process is alive, but the token counters come from the daemon's background sampler, which retries about once a second until its first reading lands — and the fake engine needs a moment to answer at all. The metrics assertions ran inside that window, so CI saw resource bars (taken from the node) with no counters beside them. Poll for the counters instead of the state.
Every docker compose call sent its output and exit status to /dev/null, so a failure explained nothing. A bring-up that could not build left only "Tearing down..." and an exit code behind, and a failed `start laptop` became ninety seconds of connection-refused with no clue why — one attempt was made and never retried, so a node that lost the race for its published host port stayed down for the rest of the run. Route the lifecycle commands through a helper that stays quiet on success and reports the captured output on failure, retry the restart while waiting for it, and dump container state and logs when a wait times out. Also read `fleet status` into a variable rather than piping it to grep: under pipefail a matching `grep -q` can exit first and leave the pipeline reporting the writer's SIGPIPE, which the caller reads as "nothing unreachable" — the opposite of what was found.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes both flakes in the
fleet-integrationjob: metrics assertions that raced the daemon's sampler, and docker compose failures that were discarded rather than reported.Summary
wait_for_tokensand use it intest_metrics, which asserted on the engine's counters as soon as the node reportedrunning.fleet statusinto a variable inwait_for_fleetrather than piping it togrep -q.Implementation details
The metrics race.
runningmeans the engine process is alive (internal/daemon/supervisor.go) and nothing more. The counters take a different path:Daemon.Metricsreports the reading its background sampler last took rather than scraping when asked, andSampleActivityretries every second until the first reading lands. The fake engine is Imposter exec'd by the shim, so it also needs a moment to bind its port. That leaves a window where the state isrunning, the resource bars render because they come from the node rather than the engine, and the counters do not exist yet — a run landing in it failsprompt tokensand4096whileRAMpasses on the same output, which is what CI showed.Waiting on readiness would not close it:
sampleOnceruns beforecheckReadyOncein the same loop iteration, so an engine that comes up between the two reads as ready with no sample behind it until the next tick.The silent failures. Every
docker composecall discarded stdout, stderr and exit status. Two CI failures came out of that: astart laptopthat left the node refusing connections for ninety seconds with no explanation, and a bring-up that failed ten seconds in and printed nothing butTearing down.... The restart is now retried while waiting, so a start that loses the race for its published host port no longer strands the node for the rest of the run.The pipeline. Under
pipefail, agrep -qthat matches exits immediately and the writer can take a SIGPIPE, making the pipeline report 141; the!then reads that as "nothing unreachable", the opposite of what was found. Today's three-line output never fills the pipe buffer so it does not trigger, but it is a wait helper that can report ready when it is not.Verified by running the suite against Docker locally (all assertions pass), and by exercising
restart_nodewith injected start failures to confirm it retries and recovers.