fix(bootstrap): keep the cold-storage module out of restore mode#1594
Conversation
The server assembles RestoreModule or the full Module by mode, but appended ColdStorageModule unconditionally whenever the driver was not "none". The module's Archiver consumes the runtime graph (*dal.Store, *state.Machine, ctrl.Admission, *node.Node) that restore mode deliberately does not provide, so fx refused the whole graph and the server exited at boot — any deployment with cold storage enabled could not enter restore mode at all. Found by the Antithesis model test's restore cycle: the restore-mode pod crash-looped and every `ledgerctl restore download` exec failed with "container not found". ColdStorageModule now takes the restore flag and contributes nothing in restore mode: a restoring server neither archives nor reads cold data, and backup downloads carry their own storage configuration per request. The regression test fx-validates the restore-mode graph with cold storage enabled (the gap every restore e2e shared — they all ran with the driver at "none"), plus the counterfactual proving the ungated graph is unbuildable.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Approve — automated reviewThe change correctly gates the cold-storage fx module out of restore mode and updates the only call site. The added regression test covers both the fixed restore graph and the counterfactual ungated failure. No findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v3.0 #1594 +/- ##
================================================
+ Coverage 74.73% 74.98% +0.24%
================================================
Files 430 430
Lines 45722 45722
================================================
+ Hits 34172 34283 +111
+ Misses 8501 8384 -117
- Partials 3049 3055 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Found by the Antithesis model test's restore cycle (#1593): every restore-mode pod crash-looped and the cycle died at
ledgerctl restore downloadwith "container not found".Bug
cmd/server/server.goselectsRestoreModule()vs the fullModule()by mode, but appendedbootstrap.ColdStorageModule(...)unconditionally whenever the driver wasn'tnone. That module'sArchiverconstructor consumes the runtime graph —*dal.Store,*state.Machine,ctrl.Admission,*node.Node— none of which restore mode provides ("no Raft"), so fx refused the graph and the server exited at boot:Any deployment with cold storage enabled cannot enter restore mode — a disaster-recovery blocker, not just a chaos-suite failure. It went unnoticed because every restore exercise (the restore e2e suites, the operator's
restore-incrementalchainsaw test) runs with the driver atnone; the Antithesis cluster spec is production-shaped with S3 cold storage, and its restore cycle was the first thing to combine the two.Bisection confirmed cold storage is the only trigger —
--sentinel-modeand--receipt-signing-keyboot fine in restore mode.Fix
ColdStorageModule(driver, restore)contributes nothing in restore mode: a restoring server neither archives nor reads cold data, and backup downloads carry their own storage configuration per request.Tests
fx.ValidateAppregression pair ininternal/bootstrap/module_restore_test.go: the restore-mode graph with cold storage enabled must build, and the counterfactual (ungated) graph must fail with the exact missing-types error. Also verified end-to-end locally:ledger-server run --restore --cold-storage-driver ...now boots and serves the restore API.