From 8908fb4ff95e01300bfd3f5fd31ea6802a46c4ce Mon Sep 17 00:00:00 2001 From: archipelago Date: Sat, 8 Aug 2026 07:56:08 -0400 Subject: [PATCH] =?UTF-8?q?fix(container):=20apps=20stopped=20cleanly=20mu?= =?UTF-8?q?st=20come=20back=20=E2=80=94=20Restart=3Dalways?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Bitcoin Knots disappeared again, plus other apps." Root cause is a pairing, not a single bug: quadlet renders `podman run ... --replace --rm`, so the container is deleted the moment it stops, and from_manifest set Restart=on-failure, which declines to restart after a CLEAN exit. bitcoind exits 0 on SIGTERM. So any clean stop deleted the container AND left it deleted — the app vanished from podman and from My Apps until a later archipelago reconcile tick noticed and recreated it. That is the "previously-running app has no container after boot — recreating (desired-state recovery)" line, which fired for bitcoin-knots at 18:53, 19:57 and 20:39 and for electrumx at 19:57 and 20:42 on 2026-08-07. A crash always self-healed: on-failure restarted the unit and podman run recreated the container. Only a clean exit stranded it, which is why this survived so long. The justification for on-failure was wrong on systemd's own semantics. It read "clean exits — e.g. operator-issued systemctl stop — stay stopped", but Restart= is never consulted for a unit stopped via systemctl stop (systemd.service(5)), and that is exactly how archipelago stops these apps (prod_orchestrator -> stop_service_with_timeout). Always keeps the stopped-stays-stopped behaviour and drops the failure mode. Always also restores the premise of the Quadlet migration — systemd owns supervision, so an app returns without archipelago alive to notice it left. Checked before flipping: no manifest declares a one-shot container and there is no manifest-level restart field, so nothing gets restart-looped. Propagation to existing nodes is via sync_quadlet_unit's drift re-render, which rewrites the unit and daemon-reloads WITHOUT restarting the service — running containers are undisturbed and the new policy governs the next start. OnFailure is kept as a deliberate opt-in with a note not to wire it back to backends. Two tests now pin the new default and assert on-failure is absent from a rendered backend unit. Container suite 215/215. NOTE FOR THE OPERATOR: this changes supervision semantics for every app on the Quadlet canary path. Wants sign-off and a lifecycle-gate run before OTA. Co-Authored-By: Claude Opus 5 (1M context) --- core/archipelago/src/container/quadlet.rs | 51 +++++++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) 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")); } }