diff --git a/core/archipelago/src/container/quadlet.rs b/core/archipelago/src/container/quadlet.rs index 7771b328..0ea235b1 100644 --- a/core/archipelago/src/container/quadlet.rs +++ b/core/archipelago/src/container/quadlet.rs @@ -73,15 +73,39 @@ pub enum NetworkMode { Pasta, } -/// systemd Restart= policy for the generated `.service` unit. Companions -/// use Always (any exit triggers a restart). Backends use OnFailure -/// (clean exits — e.g. operator-issued `systemctl stop` — stay stopped, -/// only crashes get restarted automatically). +/// systemd Restart= policy for the generated `.service` unit. +/// +/// Everything archipelago generates uses Always. Backends used to use +/// OnFailure, justified as "clean exits — e.g. operator-issued `systemctl +/// stop` — stay stopped". That justification was wrong on systemd's own +/// semantics: `Restart=` is never consulted for a unit stopped via +/// `systemctl stop` (systemd.service(5): "the service is not restarted if it +/// is stopped with systemctl stop or an equivalent operation"), and that is +/// exactly how archipelago stops these apps +/// (`prod_orchestrator` → `stop_service_with_timeout`). So OnFailure bought +/// none of the behaviour it claimed to. +/// +/// What it cost was apps vanishing. Quadlet renders `podman run … --rm`, so +/// the container is deleted the moment it stops; OnFailure then declines to +/// restart after a CLEAN exit, and bitcoind exits 0 on SIGTERM. Any clean +/// stop therefore deleted the container AND left it deleted, so the app +/// disappeared from podman and from My Apps until a later archipelago +/// reconcile tick recreated it — logged as "previously-running app has no +/// container after boot — recreating" for bitcoin-knots and electrumx +/// repeatedly on 2026-08-07, and reported by the operator as "Bitcoin Knots +/// disappeared again". A crash always self-healed; only a clean exit +/// stranded it, which is why this hid for so long. +/// +/// Always also restores the premise of the Quadlet migration: systemd owns +/// supervision, so an app comes back without needing archipelago alive to +/// notice. Safe against restart-looping because no manifest declares a +/// one-shot container and there is no manifest-level restart field. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RestartPolicy { Always, - /// Used by `from_manifest` for backend manifests. Wired through - /// `install_via_quadlet` (gated by `Config::use_quadlet_backends`). + /// Retained as a deliberate opt-in for a unit that genuinely must stay + /// down after a clean exit. Nothing selects it today — do not wire it to + /// backends again without re-reading the note above. OnFailure, } @@ -498,7 +522,9 @@ impl QuadletUnit { read_only_root: app.security.readonly_root, no_new_privileges: app.security.no_new_privileges, cpu_quota: app.resources.cpu_limit, - restart_policy: RestartPolicy::OnFailure, + // Always, not OnFailure: with quadlet's `--rm`, OnFailure left a + // cleanly-exited app deleted and unrestarted. See RestartPolicy. + restart_policy: RestartPolicy::Always, } } } @@ -1302,7 +1328,10 @@ app: .add_hosts .iter() .any(|(n, ip)| n == "host.archipelago" && ip == "10.89.0.1")); - assert_eq!(u.restart_policy, RestartPolicy::OnFailure); + // Always, not OnFailure. Quadlet renders `--rm`, so OnFailure left a + // cleanly-exited app both deleted and unrestarted — the vanishing + // bitcoin-knots/electrumx bug. Do not relax this back. + assert_eq!(u.restart_policy, RestartPolicy::Always); } #[test] @@ -1760,6 +1789,10 @@ app: assert!(body.contains("AddHost=host.archipelago:10.89.0.1")); assert!(body.contains("DropCapability=ALL")); assert!(body.contains("NoNewPrivileges=true")); - assert!(body.contains("Restart=on-failure")); + // A rendered backend unit must never carry on-failure again: paired + // with quadlet's `--rm` it deletes a cleanly-stopped app and leaves it + // deleted. + assert!(body.contains("Restart=always")); + assert!(!body.contains("Restart=on-failure")); } }