diff --git a/core/archipelago/src/container/prod_orchestrator.rs b/core/archipelago/src/container/prod_orchestrator.rs index b8e5cffe..766c5441 100644 --- a/core/archipelago/src/container/prod_orchestrator.rs +++ b/core/archipelago/src/container/prod_orchestrator.rs @@ -1836,6 +1836,9 @@ impl ProdContainerOrchestrator { // app whose container vanished (e.g. a wedged teardown cleared by a // reboot) instead of leaving it down. See the immich .198 incident. let was_running = crate::crash_recovery::load_last_running_names(&self.data_dir).await; + // Durable installation record, consulted alongside the perishable + // `was_running` snapshot for desired-state recovery below. + let installed_apps = crate::crash_recovery::load_installed_apps(&self.data_dir).await; let (manifests, container_name_by_app_id): ( Vec, std::collections::HashMap, @@ -1941,6 +1944,22 @@ impl ProdContainerOrchestrator { if mode == ReconcileMode::ExistingOnly && reason == "absent" && (was_running.contains(&compute_container_name(&lm.manifest)) + // The durable answer, and the one that does not + // erode. `was_running` only records what was + // running at the last snapshot, so an app that + // stays down long enough ages out of it and can + // never be recovered — bitcoin-knots on + // archi-dev-box (2026-08-08), and + // indeedhub-minio/-postgres before it. The + // stack-member clause below was the narrow patch + // for that second case; this is the general one. + // Safe against resurrecting something removed on + // purpose: `user_uninstalled` is checked earlier in + // ensure_running_with_mode and returns before + // anything is created, and uninstall clears this + // record in the same breath as setting that marker. + || installed_apps.contains(&app_id) + || installed_apps.contains(&compute_container_name(&lm.manifest)) // Absent STACK MEMBER whose siblings have live // containers: the stack is installed, so the // missing member is a hole, not a choice. The @@ -4244,6 +4263,10 @@ impl ContainerOrchestrator for ProdContainerOrchestrator { // baseline app the user had previously uninstalled would hit the // same reconcile guard and silently no-op. crate::crash_recovery::clear_user_uninstalled(&self.data_dir, app_id).await; + // Durable record that this app is installed, so a container that later + // vanishes is recovered however long it has been gone — the + // `was_running` snapshot alone forgets after a few daemon restarts. + crate::crash_recovery::mark_installed(&self.data_dir, app_id).await; // Idempotent: if the container is already up and healthy, just // refresh hooks and return. If it's stopped, start it. If it's // missing or in a wedged state, install fresh. @@ -4307,6 +4330,10 @@ impl ContainerOrchestrator for ProdContainerOrchestrator { // install; the start/restart RPC handlers also clear it). crate::crash_recovery::clear_user_stopped(&self.data_dir, app_id).await; crate::crash_recovery::clear_user_uninstalled(&self.data_dir, app_id).await; + // Durable record that this app is installed, so a container that later + // vanishes is recovered however long it has been gone — the + // `was_running` snapshot alone forgets after a few daemon restarts. + crate::crash_recovery::mark_installed(&self.data_dir, app_id).await; let lm = self.loaded(app_id).await?; let action = self.ensure_running(&lm).await?; match action { @@ -4525,6 +4552,10 @@ impl ContainerOrchestrator for ProdContainerOrchestrator { // survives to the next restart. Only mark on the success path above // — a failed removal means the app isn't actually gone. crate::crash_recovery::mark_user_uninstalled(&self.data_dir, app_id).await; + // …and drop the installation record in the same breath. Leaving a + // stale claim behind would let desired-state recovery recreate the very + // app that was just uninstalled. + crate::crash_recovery::clear_installed(&self.data_dir, app_id).await; Ok(()) } diff --git a/core/archipelago/src/crash_recovery.rs b/core/archipelago/src/crash_recovery.rs index a41e78f5..a9666e79 100644 --- a/core/archipelago/src/crash_recovery.rs +++ b/core/archipelago/src/crash_recovery.rs @@ -173,6 +173,93 @@ pub async fn clear_user_stopped(data_dir: &Path, name: &str) { // archipelago restart, at which point the boot reconciler resurrects it. // This mirrors `user_stopped` exactly, just for uninstall instead of stop. +// The durable counterpart to `user-uninstalled`: what IS installed. +// +// Recovery of a vanished app used to be decided from `running-containers.json` +// ("what was running at the last snapshot"), which is a different question and +// a perishable answer — it only records what is running NOW, so an app that +// stays down long enough simply ages out of it. Once out, boot's +// `ExistingOnly` mode will not recreate it, because it cannot tell "installed +// and lost" from "merely available in the catalog". The app is then gone for +// good with its manifest still on disk and nothing to bring it back. +// +// That has now happened twice. indeedhub-minio/-postgres went permanently +// absent on one node (2026-08-06) — patched narrowly with +// `absent_stack_member_with_live_sibling`, which only rescues a stack member +// that still has a live sibling. bitcoin-knots is standalone, so on +// archi-dev-box (2026-08-08) it vanished, aged out, and stayed gone while LND +// crash-looped on it for hours and a fedimint container waited 30 hours for a +// host that no longer resolved. +// +// Installation is a decision, not a runtime observation, so it gets a record +// of its own that no amount of downtime erodes. +const INSTALLED_APPS_FILE: &str = "installed-apps.json"; + +/// Load the durable set of installed app ids / container names. +pub async fn load_installed_apps(data_dir: &Path) -> std::collections::HashSet { + let path = data_dir.join(INSTALLED_APPS_FILE); + match fs::read_to_string(&path).await { + Ok(content) => serde_json::from_str(&content).unwrap_or_default(), + Err(_) => std::collections::HashSet::new(), + } +} + +async fn save_installed_apps(data_dir: &Path, installed: &std::collections::HashSet) { + let path = data_dir.join(INSTALLED_APPS_FILE); + if let Ok(json) = serde_json::to_string_pretty(installed) { + let _ = fs::write(&path, json).await; + } +} + +/// Record that an app is installed. Called when an install succeeds. +pub async fn mark_installed(data_dir: &Path, name: &str) { + let mut installed = load_installed_apps(data_dir).await; + if installed.insert(name.to_string()) { + save_installed_apps(data_dir, &installed).await; + } +} + +/// Forget an app. Called on uninstall, beside `mark_user_uninstalled` — the +/// two must move together or a reinstall-after-uninstall leaves a stale claim. +pub async fn clear_installed(data_dir: &Path, name: &str) { + let mut installed = load_installed_apps(data_dir).await; + if installed.remove(name) { + save_installed_apps(data_dir, &installed).await; + } +} + +/// Seed the record on a node that predates it, from apps that demonstrably +/// exist right now. +/// +/// Deliberately additive and one-way: it only ever ADDS names that have a real +/// container, so it cannot invent an install, and it never removes. A node +/// upgrading into this feature therefore starts with a truthful, conservative +/// record instead of an empty one that would make every existing app look +/// uninstalled — which would disable recovery for exactly the apps that most +/// need it. Runs on every boot, so an app installed before the upgrade is +/// still picked up whenever it is next seen alive. +pub async fn backfill_installed_apps(data_dir: &Path, present_container_names: &[String]) { + if present_container_names.is_empty() { + return; + } + let mut installed = load_installed_apps(data_dir).await; + let uninstalled = load_user_uninstalled(data_dir).await; + let mut changed = false; + for name in present_container_names { + // Never re-claim something the operator deliberately removed: the + // container can still be running under systemd after an uninstall. + if uninstalled.contains(name) { + continue; + } + if installed.insert(name.clone()) { + changed = true; + } + } + if changed { + save_installed_apps(data_dir, &installed).await; + } +} + /// Load the set of explicitly user-uninstalled app/container names from disk. pub async fn load_user_uninstalled(data_dir: &Path) -> std::collections::HashSet { let path = data_dir.join(USER_UNINSTALLED_FILE); @@ -1106,6 +1193,89 @@ mod tests { use super::*; use tempfile::TempDir; + #[tokio::test] + async fn installed_record_survives_and_forgets_on_uninstall() { + let tmp = TempDir::new().unwrap(); + assert!(load_installed_apps(tmp.path()).await.is_empty()); + + mark_installed(tmp.path(), "bitcoin-knots").await; + mark_installed(tmp.path(), "lnd").await; + assert!(load_installed_apps(tmp.path()).await.contains("bitcoin-knots")); + + // Uninstall forgets it, or desired-state recovery would recreate the + // very app that was just removed. + clear_installed(tmp.path(), "bitcoin-knots").await; + let after = load_installed_apps(tmp.path()).await; + assert!(!after.contains("bitcoin-knots")); + assert!(after.contains("lnd"), "clearing one must not drop the rest"); + } + + #[tokio::test] + async fn the_record_does_not_erode_the_way_the_running_snapshot_does() { + // The whole point. running-containers.json answers "what is running + // NOW", so an app that stays down long enough ages out of it and can + // never be recovered — bitcoin-knots on archi-dev-box, 2026-08-08. + // Installation is a decision, so no amount of downtime may erase it. + let tmp = TempDir::new().unwrap(); + mark_installed(tmp.path(), "bitcoin-knots").await; + + // Nothing is running at all, and a snapshot is taken saying so. + save_container_snapshot_for_test(tmp.path(), &[]).await; + + assert!(load_last_running_names(tmp.path()).await.is_empty()); + assert!( + load_installed_apps(tmp.path()).await.contains("bitcoin-knots"), + "installation record must outlive the running snapshot" + ); + } + + #[tokio::test] + async fn backfill_claims_only_what_exists_and_never_what_was_uninstalled() { + let tmp = TempDir::new().unwrap(); + + // The operator uninstalled filebrowser; its container may still be up, + // because a Quadlet unit is owned by systemd. Backfilling from live + // containers must not re-claim it. + mark_user_uninstalled(tmp.path(), "filebrowser").await; + + backfill_installed_apps( + tmp.path(), + &[ + "bitcoin-knots".to_string(), + "electrumx".to_string(), + "filebrowser".to_string(), + ], + ) + .await; + + let installed = load_installed_apps(tmp.path()).await; + assert!(installed.contains("bitcoin-knots")); + assert!(installed.contains("electrumx")); + assert!( + !installed.contains("filebrowser"), + "backfill re-claimed an app the operator uninstalled" + ); + } + + #[tokio::test] + async fn backfill_is_additive_and_never_removes() { + // It runs on every boot. If it replaced rather than merged, an app + // that happened to be down at that moment would be dropped from the + // record — reintroducing the erosion this record exists to prevent. + let tmp = TempDir::new().unwrap(); + mark_installed(tmp.path(), "lnd").await; + + backfill_installed_apps(tmp.path(), &["bitcoin-knots".to_string()]).await; + + let installed = load_installed_apps(tmp.path()).await; + assert!(installed.contains("lnd"), "a down app was dropped by backfill"); + assert!(installed.contains("bitcoin-knots")); + + // An empty adoption list (podman unreachable, say) must change nothing. + backfill_installed_apps(tmp.path(), &[]).await; + assert_eq!(load_installed_apps(tmp.path()).await.len(), 2); + } + #[tokio::test] async fn test_no_crash_without_pid_file() { let tmp = TempDir::new().unwrap(); diff --git a/core/archipelago/src/main.rs b/core/archipelago/src/main.rs index f49983e4..2fc1f217 100644 --- a/core/archipelago/src/main.rs +++ b/core/archipelago/src/main.rs @@ -329,6 +329,18 @@ async fn main() -> Result<()> { report.adopted.len(), report.adopted ); + // Seed the durable installation record from what demonstrably + // exists. Nodes that predate the record would otherwise start + // with an empty one, which reads as "nothing is installed" and + // disables desired-state recovery for precisely the apps that + // need it. Additive and evidence-based: only names with a real + // adopted container are claimed, and anything the operator + // uninstalled is skipped, so it cannot invent an install. + crate::crash_recovery::backfill_installed_apps( + &config.data_dir, + &report.adopted, + ) + .await; } Ok(Err(e)) => { tracing::warn!(error = %e, "prod orchestrator: adopt_existing failed (non-fatal)");