fix(container): companions follow installed apps, not available manifests

archi-dev-box was running archy-fedimint-ui and archy-lnd-ui with no
fedimint and no lnd container anywhere on the box. The Fedimint Guardian
UI sat on :8175 serving its "waiting for Bitcoin" page forever with
nothing behind it, which is what the operator reported as "fedimint
guardian installs but does not work" — there was nothing to install, the
UI was already up.

The boot reconciler drove companion provisioning from manifest_ids(),
which is every manifest the node can SEE: the whole apps/ directory plus
the signed-catalog overlay, 56 of them. The app reconciler has drawn this
line since phase 3 (ReconcileMode::ExistingOnly, "merely listing a
catalog manifest never installs an unqualified app"); the companion stage
never got the equivalent guard, so it stood up a UI for every app that
merely had a manifest and then self-healed it forever.

The other half is that reconcile() could only ever ADD. remove_for fires
only on the explicit uninstall RPC, so nothing ever subtracted: an
install that failed after its companion landed, or a container removed
by any other route, left a Restart=always unit alive permanently.

- installed_app_ids() replaces manifest_ids(): app ids whose container
  actually exists. Returns Option, because a caller that removes things
  on absence must not read "I could not look" as "nothing is installed".
  Container presence in ANY state is the whole test — it deliberately
  does not inherit the user_stopped/disabled filters, since a stopped app
  is still an installed app and treating it otherwise would tear its
  companion down and rebuild it on the next start.
- manifest_ids() is deleted rather than left unused. Its contract reads
  as "installed" to anyone skimming, which is the whole bug.
- reap_orphans() removes companions whose backend is not installed, after
  ORPHAN_GRACE (300s). The grace period is required, not defensive: this
  node runs ARCHIPELAGO_USE_QUADLET_BACKENDS=true and a Quadlet app is
  briefly containerless while restarting, so reaping on the first absent
  tick would cost a healthy companion a teardown plus a possible 900s
  image rebuild. A backend that reappears clears its clock.
- Reap failures are logged but kept out of the backoff input. Repair
  keeps a companion available; reaping only tidies one away, and a wedged
  reap must not back the repair path off to its 1h ceiling.

Every uncertain signal resolves toward not removing: no unit file and a
hung is-active reads as leave-it-alone.

Container suite 215/215.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
archipelago
2026-08-08 07:08:17 -04:00
co-authored by Claude Opus 5
parent 455b813630
commit 3ac59a73b3
3 changed files with 395 additions and 12 deletions
@@ -110,8 +110,39 @@ impl BootReconciler {
Some(tokio::spawn(async move {
let mut failure_rounds: u32 = 0;
loop {
let installed = orchestrator.manifest_ids().await;
// `installed_app_ids`, NOT `manifest_ids`: a manifest exists
// on disk for every *available* app, so driving companion
// provisioning from it stood up a UI for apps nobody had
// installed and self-healed it forever (archi-dev-box ran
// archy-fedimint-ui and archy-lnd-ui with no fedimint and no
// lnd container present — the Guardian UI served its wait
// page with nothing behind it, reported as "fedimint
// installs but does not work"). `None` means the container
// listing failed: skip the whole stage rather than reap
// every companion on a transient probe error.
let Some(installed) = orchestrator.installed_app_ids().await else {
tracing::warn!(
"companion reconcile: cannot determine installed apps, skipping this pass"
);
time::sleep(interval).await;
continue;
};
let failures = crate::container::companion::reconcile(&installed).await;
// Reap failures are logged but deliberately kept OUT of
// `failures`, which drives the backoff below. Repair keeps
// a companion available; reaping only tidies one away. A
// reap that fails persistently (a wedged systemctl, say)
// must not back the repair path off to its 1h ceiling and
// starve the thing that actually matters.
for (companion, err) in
crate::container::companion::reap_orphans(&installed).await
{
tracing::warn!(
companion = %companion,
error = %err,
"companion reap failed"
);
}
for (companion, err) in &failures {
tracing::warn!(
companion = %companion,
+311
View File
@@ -47,6 +47,21 @@ const REPAIR_COOLDOWN: Duration = Duration::from_secs(600);
static REPAIR_FAILED_AT: LazyLock<Mutex<HashMap<&'static str, Instant>>> =
LazyLock::new(|| Mutex::new(HashMap::new()));
/// A companion must look orphaned for this long before it is reaped.
///
/// "Backend container absent" is not the same as "backend app uninstalled":
/// a Quadlet-managed app is briefly containerless while it restarts, and this
/// node runs `ARCHIPELAGO_USE_QUADLET_BACKENDS=true`. Reaping on the first
/// absent tick would take down a healthy companion mid-restart and reinstall
/// it on the next pass — an image pull or a 900s build in the worst case.
/// A real uninstall stays absent indefinitely, so waiting costs nothing.
const ORPHAN_GRACE: Duration = Duration::from_secs(300);
/// First tick at which each companion was observed with no installed backend.
/// Cleared as soon as a backend reappears, so the grace period restarts.
static ORPHAN_SINCE: LazyLock<Mutex<HashMap<&'static str, Instant>>> =
LazyLock::new(|| Mutex::new(HashMap::new()));
/// Static description of one companion. The full list per backend
/// app_id lives in `companions_for`.
#[derive(Debug, Clone)]
@@ -86,6 +101,11 @@ pub fn companions_for(package_id: &str) -> &'static [CompanionSpec] {
}
}
/// Every companion this build knows how to provision. Kept beside
/// `companions_for` — a new companion must be added to both, or the reaper
/// will not recognise it as one of ours and will leave it running forever.
const ALL_COMPANIONS: &[&[CompanionSpec]] = &[BITCOIN_UI, LND_UI, ELECTRS_UI, FEDIMINT_UI];
const BITCOIN_UI: &[CompanionSpec] = &[CompanionSpec {
name: "archy-bitcoin-ui",
image_base: "bitcoin-ui",
@@ -615,6 +635,133 @@ pub async fn reconcile(installed_apps: &[String]) -> Vec<(String, anyhow::Error)
failures
}
/// Companions this build knows about that no app in `installed_apps` claims.
///
/// Pure set arithmetic, split out from `reap_orphans` so the "which ones go"
/// decision is testable without a systemd manager. A companion shared by
/// several backends (archy-bitcoin-ui serves both bitcoin-core and
/// bitcoin-knots) survives while ANY of its backends is installed.
fn orphan_companions(installed_apps: &[String]) -> Vec<&'static CompanionSpec> {
let expected: std::collections::HashSet<&str> = installed_apps
.iter()
.flat_map(|app_id| companions_for(app_id))
.map(|spec| spec.name)
.collect();
ALL_COMPANIONS
.iter()
.copied()
.flatten()
.filter(|spec| !expected.contains(spec.name))
.collect()
}
/// Narrow `orphans` to those that have been orphaned for at least
/// `ORPHAN_GRACE`, updating `since` in place.
///
/// Split out of `reap_orphans` and given an explicit `now` so the grace
/// behaviour is testable without sleeping: it is the guard that stops a
/// restarting backend from costing its companion a teardown+reinstall.
fn due_after_grace(
orphans: Vec<&'static CompanionSpec>,
orphan_names: &std::collections::HashSet<&str>,
since: &mut HashMap<&'static str, Instant>,
now: Instant,
) -> Vec<&'static CompanionSpec> {
// A companion whose backend came back is no longer a candidate; drop its
// clock so a later disappearance waits out a fresh grace period rather
// than inheriting a stale one.
since.retain(|name, _| orphan_names.contains(name));
orphans
.into_iter()
.filter(|spec| {
let first_seen = *since.entry(spec.name).or_insert(now);
now.duration_since(first_seen) >= ORPHAN_GRACE
})
.collect()
}
/// Stop and remove any companion whose backend app is not installed.
///
/// The counterpart to `reconcile`, which can only ever *add*. Without this,
/// a companion outlives its backend permanently: `remove_for` fires only on
/// the explicit uninstall RPC path, so an install that fails after the
/// companion lands, a container removed by hand, or a node whose app was
/// never installed at all keeps a `Restart=always` unit alive forever.
///
/// `installed_apps` MUST be the full installed set (see
/// `ProdOrchestrator::installed_app_ids`), never the narrow per-app list
/// `reconcile_companions_for` passes — reaping against a one-app list would
/// tear down every other companion on the node. It must also never be the
/// *manifest* list, which is every available app rather than every installed
/// one; that mistake is what left the orphans this function now clears.
///
/// Callers must not invoke this when they could not determine what is
/// installed. "I could not look" and "nothing is installed" produce the same
/// empty vector but demand opposite behaviour, so the check belongs upstream
/// where the distinction still exists.
pub async fn reap_orphans(installed_apps: &[String]) -> Vec<(String, anyhow::Error)> {
if !user_systemd_available() {
return Vec::new();
}
let orphans = orphan_companions(installed_apps);
// Age the observation before acting on it. Anything whose backend is back
// has its clock cleared; anything still orphaned must have been so for a
// full ORPHAN_GRACE before it is touched.
let orphan_names: std::collections::HashSet<&str> =
orphans.iter().map(|spec| spec.name).collect();
let due: Vec<&'static CompanionSpec> = {
let mut since = ORPHAN_SINCE.lock().unwrap();
due_after_grace(orphans, &orphan_names, &mut since, Instant::now())
};
if due.is_empty() {
return Vec::new();
}
let dir = match quadlet::unit_dir().await {
Ok(d) => d,
Err(e) => {
warn!("companion reap: cannot resolve quadlet dir: {e:#}");
return Vec::new();
}
};
let mut failures = Vec::new();
for spec in due {
// Only act on companions that are actually present, so a node that
// never had the app stays silent instead of logging every tick.
let unit_path = dir.join(format!("{}.container", spec.name));
let unit_present = fs::try_exists(&unit_path).await.unwrap_or(false);
if !unit_present {
// No unit file, so the only reason to act is a service still
// running from a removed one. A hung `is-active` under IO pressure
// must read as "leave it alone" — reaping is destructive, so every
// uncertain signal resolves toward doing nothing.
let svc = format!("{}.service", spec.name);
match tokio::time::timeout(Duration::from_secs(10), quadlet::is_active(&svc)).await {
Ok(true) => {}
Ok(false) => continue,
Err(_) => {
warn!(
companion = spec.name,
"reap: is-active probe timed out; leaving it alone"
);
continue;
}
}
}
info!(
companion = spec.name,
"reap: backend app is not installed, removing orphaned companion"
);
if let Err(e) = quadlet::disable_remove(spec.name, &dir).await {
warn!(companion = spec.name, error = %e, "companion reap failed");
failures.push((spec.name.to_string(), e));
}
}
failures
}
/// Does this companion need install_one to be re-run? Returns true if
/// the unit file is missing, stale, or the service is not active.
///
@@ -675,6 +822,170 @@ async fn needs_repair(spec: &CompanionSpec) -> Result<bool> {
mod tests {
use super::*;
fn names(specs: &[&'static CompanionSpec]) -> Vec<&'static str> {
let mut v: Vec<_> = specs.iter().map(|s| s.name).collect();
v.sort_unstable();
v
}
fn ids(list: &[&str]) -> Vec<String> {
list.iter().map(|s| s.to_string()).collect()
}
#[test]
fn every_companion_in_companions_for_is_also_in_all_companions() {
// The reaper only recognises companions listed in ALL_COMPANIONS. One
// missing from it would be provisioned by `reconcile` and then never
// cleaned up — exactly the leak this module is fixing.
let backends = [
"bitcoin",
"bitcoin-core",
"bitcoin-knots",
"lnd",
"electrumx",
"electrs",
"mempool-electrs",
"fedimint",
"fedimintd",
];
let known: std::collections::HashSet<&str> = ALL_COMPANIONS
.iter()
.copied()
.flatten()
.map(|s| s.name)
.collect();
for backend in backends {
for spec in companions_for(backend) {
assert!(
known.contains(spec.name),
"{} is provisionable but not reapable — add it to ALL_COMPANIONS",
spec.name
);
}
}
}
#[test]
fn nothing_installed_orphans_every_companion() {
assert_eq!(
names(&orphan_companions(&[])),
vec![
"archy-bitcoin-ui",
"archy-electrs-ui",
"archy-fedimint-ui",
"archy-lnd-ui"
]
);
}
#[test]
fn an_installed_backend_protects_only_its_own_companion() {
// The archi-dev-box state that exposed the bug: bitcoin-knots and
// electrumx installed, fedimint and lnd not — yet all four companions
// were running because the reconciler was fed the manifest list.
let orphans = orphan_companions(&ids(&["bitcoin-knots", "electrumx"]));
assert_eq!(names(&orphans), vec!["archy-fedimint-ui", "archy-lnd-ui"]);
}
#[test]
fn a_shared_companion_survives_on_any_one_of_its_backends() {
// archy-bitcoin-ui serves bitcoin-core AND bitcoin-knots. Installing
// either must keep it; a naive per-app reap would remove it while the
// other backend was still running.
for backend in ["bitcoin", "bitcoin-core", "bitcoin-knots"] {
let orphans = orphan_companions(&ids(&[backend]));
assert!(
!names(&orphans).contains(&"archy-bitcoin-ui"),
"archy-bitcoin-ui reaped while {backend} is installed"
);
}
}
#[test]
fn apps_without_companions_orphan_everything_and_panic_nothing() {
let orphans = orphan_companions(&ids(&["nextcloud", "not-a-real-app"]));
assert_eq!(orphans.len(), 4);
}
#[test]
fn every_backend_installed_leaves_no_orphans() {
let orphans = orphan_companions(&ids(&[
"bitcoin-knots",
"lnd",
"electrumx",
"fedimint",
]));
assert!(names(&orphans).is_empty(), "unexpected orphans: {:?}", names(&orphans));
}
fn name_set(specs: &[&'static CompanionSpec]) -> std::collections::HashSet<&'static str> {
specs.iter().map(|s| s.name).collect()
}
#[test]
fn a_freshly_orphaned_companion_is_not_reaped_immediately() {
let orphans = orphan_companions(&ids(&["bitcoin-knots"]));
let names_seen = name_set(&orphans);
let mut since = HashMap::new();
let now = Instant::now();
let due = due_after_grace(orphans, &names_seen, &mut since, now);
assert!(
due.is_empty(),
"reaped on the first observation: {:?}",
names(&due)
);
}
#[test]
fn an_orphan_past_the_grace_period_is_reaped() {
let orphans = orphan_companions(&ids(&["bitcoin-knots"]));
let names_seen = name_set(&orphans);
let mut since = HashMap::new();
let start = Instant::now();
// First pass records the clock and reaps nothing.
let due = due_after_grace(orphans.clone(), &names_seen, &mut since, start);
assert!(due.is_empty());
// A pass after the grace window reaps.
let due = due_after_grace(orphans, &names_seen, &mut since, start + ORPHAN_GRACE);
assert_eq!(names(&due), vec!["archy-electrs-ui", "archy-fedimint-ui", "archy-lnd-ui"]);
}
#[test]
fn a_backend_returning_mid_grace_resets_the_clock() {
// The restart window this guard exists for: lnd vanishes for a tick
// while its container is recreated, then comes back. Its companion
// must never be reaped, and a later real uninstall must wait out a
// full fresh grace period rather than inheriting the old clock.
let start = Instant::now();
let mut since = HashMap::new();
let orphans = orphan_companions(&ids(&["bitcoin-knots"]));
let names_seen = name_set(&orphans);
assert!(due_after_grace(orphans, &names_seen, &mut since, start).is_empty());
// lnd is back — it is no longer an orphan candidate.
let orphans = orphan_companions(&ids(&["bitcoin-knots", "lnd"]));
let names_seen = name_set(&orphans);
let due = due_after_grace(orphans, &names_seen, &mut since, start + ORPHAN_GRACE);
assert!(
!names(&due).contains(&"archy-lnd-ui"),
"lnd companion reaped even though lnd came back"
);
assert!(!since.contains_key("archy-lnd-ui"), "stale clock kept for lnd");
// lnd goes away for real. It must wait a fresh full grace period.
let orphans = orphan_companions(&ids(&["bitcoin-knots"]));
let names_seen = name_set(&orphans);
let t = start + ORPHAN_GRACE;
let due = due_after_grace(orphans.clone(), &names_seen, &mut since, t);
assert!(
!names(&due).contains(&"archy-lnd-ui"),
"lnd companion reaped without a fresh grace period"
);
let due = due_after_grace(orphans, &names_seen, &mut since, t + ORPHAN_GRACE);
assert!(names(&due).contains(&"archy-lnd-ui"));
}
#[test]
fn companions_for_known_apps_returns_expected_set() {
assert_eq!(companions_for("bitcoin-knots").len(), 1);
@@ -1659,18 +1659,59 @@ impl ProdContainerOrchestrator {
.then(|| members.iter().map(|s| s.to_string()).collect())
}
/// Snapshot of the app IDs currently in the in-memory manifest map.
/// Used by the boot reconciler to drive companion-unit reconciliation.
pub async fn manifest_ids(&self) -> Vec<String> {
let user_stopped = crate::crash_recovery::load_user_stopped(&self.data_dir).await;
// `manifest_ids()` used to live here: every app id in the in-memory
// manifest map, i.e. every manifest the node can *see* (the whole `apps/`
// directory plus the signed-catalog overlay). Its only caller was the boot
// reconciler's companion stage, which is precisely the bug described below
// — "can see" was silently read as "has installed". It is deleted rather
// than left unused so nothing reaches for it again; `installed_app_ids` is
// the answer to the question callers actually mean.
/// App ids whose container actually exists — the
/// `ReconcileMode::ExistingOnly` rule, made available to callers outside
/// `reconcile_all_with_mode`.
///
/// The app reconciler has always drawn this line ("merely listing a catalog
/// manifest never installs an unqualified app"); the companion stage did
/// not, and fed itself `manifest_ids` instead. Because a manifest exists on
/// disk for every *available* app, that provisioned and perpetually
/// self-healed a companion UI for apps nobody had installed: archi-dev-box
/// ran `archy-fedimint-ui` and `archy-lnd-ui` with no `fedimint` and no
/// `lnd` container anywhere on the box (2026-08-08). The Guardian UI served
/// its "waiting for Bitcoin" page forever with nothing behind it, which is
/// what the operator reported as "fedimint guardian installs but does not
/// work" — there was nothing to install, the UI was already up.
///
/// `None` means the runtime listing failed. That is deliberately distinct
/// from `Some(vec![])`: a caller that removes things on absence must not
/// treat "I could not look" as "nothing is installed".
///
/// The presence of a container — in ANY state — is the whole test. It
/// deliberately does NOT carry over the `user_stopped` / `disabled`
/// filters the old `manifest_ids` applied: a stopped app is still an
/// installed app, its container still
/// exists (exited), and treating it as uninstalled would make stopping an
/// app tear its companion down and rebuild it on the next start. "Is it
/// installed" and "is it currently meant to be running" are different
/// questions, and only the first one belongs here.
pub async fn installed_app_ids(&self) -> Option<Vec<String>> {
let present: std::collections::HashSet<String> = self
.runtime
.list_containers()
.await
.ok()?
.into_iter()
.map(|c| c.name)
.collect();
let state = self.state.read().await;
state
.manifests
.keys()
.filter(|app_id| !state.disabled.contains(*app_id))
.filter(|app_id| !user_stopped.contains(*app_id))
.cloned()
.collect()
Some(
state
.manifests
.iter()
.filter(|(_, lm)| present.contains(&compute_container_name(&lm.manifest)))
.map(|(app_id, _)| app_id.clone())
.collect(),
)
}
/// Scan the runtime for containers whose names match one of our manifests.