fix(deploy-test): make the 2-node deploy test run on real kits (rename sites, decouple data folder, mirror hygiene, staged jobs) - #573
Merged
Conversation
The 2/3/4-node deploy-test projects named their clients RUMC_1, MHA_1, CAM_1 and UMCU_1. Those are productive sites, and the collision is not cosmetic -- it lands in two places that are read by humans and by analysis code: * the live monitor (/srv/mediswarm/live/<SITE>/), where a deploy test on dl0 publishes heartbeats under a hospital's name, so the monitor shows a hospital running a job it never saw; * the run-history dataset, where test runs land in the hospital's rows and can only be separated afterwards by a hostname heuristic. It has produced two wrong conclusions already. Four site join dates were published about six weeks early, because April records from dl0 and dl3 carried hospital names. And a fleet-status check today read a dead test client on dl0 as a live Radboud client -- only the server token surviving dl0's reboot proved otherwise. Clients are now TEST_A_1..TEST_D_1. Nothing in the deploy scripts or the workflows hardcodes the old names -- they come purely from the project YAML and the site conf -- so this is a rename and no logic changes. The untracked deploy_sites_*node_test.conf files were updated in place too; only the .example is tracked, since the real ones carry passwords. Each project file carries a comment saying why the names are what they are, so the next person does not helpfully rename them back. extract_run_history.py now takes the site name into account: classify_host returns "test" for a TEST_* site on the name alone. The hostname heuristic stays exactly as it was, because the archive it was written for does not change -- records from before this rename still carry hospital names whatever machine produced them. Verified: TEST_A_1 with no hostname classifies as test, dl0.tud.de+RUMC_1 still test, bldaiod01+UMCU_1 still real, RUMC_1 with no hostname still unknown. Full suite 534 passed, 8 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renaming the deploy-test clients to TEST_A_1.. exposed a coupling the old
names had been hiding: the loader reads /data/$INSTITUTION/..., and
INSTITUTION defaults to SITE_NAME. The test only ever found its data
because dl0 happened to have a RUMC_1 folder and dl2 an MHA_1 folder --
the site name was doing double duty as a path component.
docker.sh already has --institution for exactly this (UKA registers under
one name and reads data under another). The deploy test just never used
it. start_clients now reads an optional <SITE>_INSTITUTION from the conf
and passes --institution, so the FL identity and the data folder can be
chosen independently. Each test conf pins the folder that client used
before the rename, so the test runs on identical data:
DL0 TEST_A_1 RUMC_1 DL3A TEST_C_1 CAM_1
DL2 TEST_B_1 MHA_1 DL3B TEST_D_1 UMCU_1
Also adds an optional <SITE>_DOCKER_OPTIONS, forwarded via
MEDISWARM_DOCKER_OPTIONS (the passthrough docker.sh gained in #218). That
is how the per-case prediction return gets switched on for a test client
-- ODELIA_RETURN_PER_CASE=1 is deliberately site-side only and the
harness had no way to set it.
Both are optional and empty by default, so a conf without them behaves
exactly as before. Only the .example is tracked; the real confs carry
passwords and were updated in place.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GIT_SHA is only recorded in the result JSON, but an unguarded $(git rev-parse) under set -e aborts the whole harness when there is no .git -- one line after MEDISWARM_IMAGE_VERSION had been honoured. It happened running the 1.8.0 MVP test from an rsynced tree on dl3: the log ended at '[host-lock] acquired' followed by 'fatal: not a git repository' and nothing else, and stop_all never ran. Fall back to 'unknown' instead. The value is informational. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
submit_job always prefixed the job name with MediSwarm/application/jobs/, so only the image's stock jobs could be run through the harness. A job staged by prepare_odelia_job.sh -- which is the only way to get a --warm-start continue job -- lands under the admin kit's local/ and is visible in the admin container at /fl_admin/local/mediswarm_jobs/<name>. An absolute --job is now submitted as given. Relative names behave exactly as before. This is what lets the warm-start provenance guard (#545) be exercised end to end on real kits instead of only in unit tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ep server-side outputs Two things the 2026-09-11 2-node run (TEST_A_1 on dl0, TEST_B_1 on dl2) showed. 1. All three attempts of the MST job aborted at the first gather on dl2 with "None of the 187 incoming model parameter(s) matched the local model's 450". The 450-parameter local model was 1DivideAndConquer: dl2's /scratch/mediswarm_latest_global.pt was left by the failed 09-08 run of that model, carried no provenance sidecar (written before #545), and warm_start_mode=auto loaded it with only a warning. dl0's mirror happened to be MST, so only one side blew up. The deploy test reuses one SCRATCHDIR per site across models, so --all can never get past its first model: before #545 the wrong weights loaded silently and killed the gather, since #545 the provenance guard panics. clear_stale_mirrors() removes the mirror and its sidecar on every client before a model's first attempt. Retries within one model keep it -- same architecture, and resuming from the last good round is what warm-continue is for. 2. stop_all() rm -rf's the server dir, and with it every <job_id>/ run dir. That is where the opt-in per-case predictions (#557) and the per-site metrics land, so a per-case deploy test would "pass" and leave nothing to inspect. save_server_artifacts() copies each run's cross_site_val/ into RESULTS_DIR right after the server log is saved. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The site confs carry the runner password. Only the *.example files are tracked; the ignore list named each real conf individually, so a new one (deploy_sites_2node_test.dl3.conf) showed up in git status untracked and one careless 'git add .' away from a public repo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lly ran from stop_all() runs before start_server() in every attempt and deletes the deployed server kit, so the server always falls back to the kit under workspace/.../prod_00 and its run dirs pile up there across models and days. Resolve that directory instead of assuming $DEPLOY_BASE, and copy only this run's job -- identified from the server log, since collect_checkpoints() has not set LAST_JOB_ID yet at this point. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…or wipe clear_stale_mirrors now runs before every model so a stale mirror from an earlier run cannot be warm-started by accident. A test of warm-start itself needs the opposite: the provenance guard's refuse path plants a wrong-architecture mirror on purpose and expects WARM_START_MODEL_MISMATCH. Setting DEPLOY_TEST_KEEP_MIRROR leaves every client's mirror in place and says so in the log. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ultimate-Storm
changed the title
…ad a completed job's cross_site_val from the job store Two things the 13 Sep MVP runs found: 1. An absolute --job path (the staged warm-start job) went straight into the submit-log file name, so the redirect failed, the admin session never ran, and the harness reported 'no job id returned'. basename() it. 2. save_server_artifacts copied cross_site_val/ from the server's run dir, which only exists for ABORTED jobs: when a job completes, NVFlare packs the server workspace into its job store (/tmp/nvflare/jobs-storage/<job_id>/workspace, a zip inside the container) and deletes the run dir. The 12 Sep run aborted and yielded cross_val_results.json; the clean 20-round run on 13 Sep yielded nothing although the server logged 'Published metrics for 2 site(s)'. Fall back to docker cp + unzip from the store, with a short retry because the store is written a few seconds after 'Server runner finished'. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…deploy deploy_kits rm -rf's the deployed admin kit before unzipping it again, which also deleted local/mediswarm_jobs/ -- where prepare_odelia_job.sh stages a warm-start job for submission by absolute --job path. The submission then failed with "'/fl_admin/local/mediswarm_jobs/...' is not a valid folder" (13 Sep, guard test). Move the staged jobs aside and put them back. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5 tasks
Ultimate-Storm
added a commit
that referenced
this pull request
Sep 13, 2026
The workflow built the productive jobs with min_clients = 5 and then ran them with two test clients, which aborts in 30 s with 'min_clients (5) exceeds the number of participating clients (2)'. buildDockerImageAndStartupKits.sh already has --min-clients for exactly this; pass it, with a dispatch input defaulting to the two-client conf. Found by the v1.8.0 release-triggered run (34778471496), which first failed on the runner conf still naming the test sites RUMC_1/MHA_1 after #573; the runner-local confs on dl0 and dl2 are corrected alongside (TEST_A_1/TEST_B_1, institutions, real data paths, 2-site project). Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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
2 participants
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.
Started as a rename of the deploy-test clients; became everything needed to make the 2-node deploy test actually run on real kits. Eight commits, each found by running it.
Why the rename (original scope)
The test clients were
RUMC_1andMHA_1— real hospitals. The collision put four wrong join dates on a published chart (April records from dl0/dl3 wearing hospital names) and, on 11 Sep, made a dead test client on dl0 read as a live Radboud client. NowTEST_A_1..TEST_D_1, with a comment in each project YAML saying why.What the rename exposed, and the fixes (in order)
5216d96<SITE>_INSTITUTION+--institution/data/$INSTITUTION, which defaults toSITE_NAME; the old test only found data because dl0 happened to have aRUMC_1foldere82f83cGIT_SHA=$(git …)5d55c2e--job--warm-start continuejob staged byprepare_odelia_job.shb105d49clear_stale_mirrorsbefore each model; keep server-side outputsae62050deploy_sites_*.conf22db920cross_site_val/was being deleted bystop_allbefore anyone could read itcf3f4e8DEPLOY_TEST_KEEP_MIRRORopt-out00ba237--jobpaths00ba237cross_site_val/from the job store/tmp/nvflare/jobs-storage/<job>/workspace(a zip inside the container) and deletes the run dir; only aborted runs leave one. The clean 20-round run on 13 Sep loggedPublished metrics for 2 site(s)and saved nothingc3d4d38local/mediswarm_jobs/across the admin-kit redeploydeploy_kitsrm -rf'd the deployed admin kit, taking the staged warm-start job with it:'/fl_admin/local/mediswarm_jobs/…' is not a valid folder(13 Sep)Also:
<SITE>_DOCKER_OPTIONS, forwarded viaMEDISWARM_DOCKER_OPTIONS, which is howODELIA_RETURN_PER_CASE=1gets switched on for a test client.Evidence
Run from dl3 (server on the LAN — see #553: agh1 cannot hole-punch and every path to it is DERP-relayed) on 12 Sep: 10 rounds, 3 aggregations, and
cross_val_results.jsonreturned with both sites and per-class support counts; on 13 Sep, on the release image: 20 of 20 rounds, 9 aggregations, no aborts, loader-worker cap active — the F10 fix (#525/#534) verified on real kits at multi-site. The run then aborted on a transient withmin_clients=2(zero tolerance on two nodes), which is expected for the fixture, not a defect.The two product bugs it surfaced — the guard gap and the pin-memory race (#574) — are fixed in #575, which this test is being re-run against.
validate-swarmon this PR has been evicted (cancelled, rendered as "fail") on every push by the one-pending-slot concurrency rule (#554); unit tests are green on every commit, and the deploy test itself is the validation.🤖 Generated with Claude Code