fix(container): remember that an app is installed, so a vanished one comes back

An app whose container disappeared could be lost permanently. Desired-state
recovery decided whether to recreate it from running-containers.json — "what
was running at the last snapshot" — which is a different question and a
perishable answer: it records only what is running NOW, so an app that stays
down long enough simply ages out. Once out, boot's ExistingOnly mode will not
recreate it, because it cannot tell "installed and lost" from "merely
available in the catalog". Manifest still on disk, nothing to bring it back.

This is the second occurrence of one root cause. indeedhub-minio/-postgres
went permanently absent on one node (2026-08-06); the fix then was
`absent_stack_member_with_live_sibling`, which only rescues a stack member
that still has a living sibling. bitcoin-knots is standalone, so on
archi-dev-box (2026-08-08) it vanished, aged out, and stayed gone — LND
crash-looping on `lookup bitcoin-knots: no such host` for hours, electrumx
unable to reach its daemon, and an orphaned fedimint container waiting 30
hours for a host that no longer resolved. Recovering it took a manual
reinstall. This is the general fix the narrow one implied.

Installation is a DECISION, not a runtime observation, so it gets a record
that no amount of downtime erodes: installed-apps.json, written when an
install succeeds and cleared on uninstall, in the same breath as
mark_user_uninstalled — leaving a stale claim would let recovery recreate the
app that was just removed. It is the durable counterpart to the
user-uninstalled marker that already existed.

Safety, in order of how badly each could go wrong:
- Cannot resurrect a deliberate uninstall: user_uninstalled is checked
  earlier in ensure_running_with_mode and returns before anything is created,
  and uninstall clears this record too.
- Cannot install an app nobody asked for: only names in the record qualify,
  and ExistingOnly's other guards are untouched.
- Cannot mislead a node upgrading into the feature: backfill seeds from
  ADOPTED containers only — evidence that something is really there — skips
  anything user-uninstalled, is additive so a momentarily-down app is never
  dropped, and no-ops on an empty adoption list (podman unreachable must not
  read as "nothing is installed").

Four tests, including the one that states the point: the record must outlive
a running-snapshot that has gone empty.

Container suite 221/221, crash_recovery 15/15.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
archipelago
2026-08-08 13:59:23 -04:00
co-authored by Claude Opus 5
parent 51a4b59ce8
commit 6c6e237646
3 changed files with 213 additions and 0 deletions
@@ -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<LoadedManifest>,
std::collections::HashMap<String, String>,
@@ -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(())
}
+170
View File
@@ -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<String> {
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<String>) {
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<String> {
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();
+12
View File
@@ -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)");