From b57f3637450b5b10dd053ef85364488f3211e5e8 Mon Sep 17 00:00:00 2001 From: archipelago Date: Sat, 8 Aug 2026 08:37:44 -0400 Subject: [PATCH] fix(container): a no-op ownership repair must not fail the whole reconcile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit archi-dev-box logged `reconcile failed app_id=btcpay-server error=chown /var/lib/archipelago/postgres-btcpay failed with status exit status: 1` while BTCPay was running and healthy and there was nothing to repair: `find /var/lib/archipelago/postgres-btcpay ! -uid 100998` returns zero files, and the identical command run by hand exits 0. The chown through `sudo systemd-run` had simply failed once, and that transient failure propagated out of the pre-start hook and took the app's entire reconcile with it. These hooks exist to repair OLD installs. On a healthy node the repair is already a no-op, so its failure is not evidence of anything being wrong. repair_dir_ownership folds the gate, the chown and the verdict into one place: skip when ownership is already right, chown when it is not, and on a failed chown RE-PROBE before deciding it matters. If the ownership is correct anyway — a concurrent repair, or a transient sudo/systemd-run failure on an already-correct tree — warn and continue. Only a chown that fails AND leaves the ownership wrong is an error, which is the case the loud failure was written for: a mis-owned volume the app genuinely cannot open. Replaces the three hand-rolled gate+chown+bail blocks in ensure_btcpay_stack_dirs and the one in ensure_fedimint_dirs. Container suite 215/215. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/container/prod_orchestrator.rs | 85 ++++++++++++------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/core/archipelago/src/container/prod_orchestrator.rs b/core/archipelago/src/container/prod_orchestrator.rs index 4a39c0b5..b1a2ff4c 100644 --- a/core/archipelago/src/container/prod_orchestrator.rs +++ b/core/archipelago/src/container/prod_orchestrator.rs @@ -361,6 +361,50 @@ fn ownership_already_correct(path: &str, host_uid_gid: &str) -> bool { md.uid() == uid && md.gid() == gid } +/// Drift-gated ownership repair for the per-app hooks: skip when already +/// correct, `chown -R` when not, and — critically — do not fail the caller +/// when the chown fails but the ownership is right anyway. +/// +/// These hooks exist to repair OLD installs. On a healthy node the repair is a +/// no-op, so a transient failure of it must not abort the app's whole +/// reconcile. It did: `chown -R` through `sudo systemd-run` failed once on +/// archi-dev-box (2026-08-08 04:44) on a `postgres-btcpay` tree where every +/// single file was already owned by the target uid — `find ! -uid 100998` +/// returned nothing, and the identical command run by hand exited 0 — yet the +/// error propagated all the way out as `reconcile failed +/// app_id=btcpay-server`, taking a running, healthy BTCPay's reconcile with +/// it. +/// +/// So a chown failure is only an error if the ownership is ALSO wrong +/// afterwards. That keeps the loud failure for the case it was written for (a +/// genuinely mis-owned volume the app cannot open) and drops it for the case +/// that only ever produced noise. +async fn repair_dir_ownership(dir: &str, host_uid_gid: &str) -> Result<()> { + if ownership_already_correct_from_host(dir, host_uid_gid).await { + return Ok(()); + } + let status = host_sudo(&["chown", "-R", host_uid_gid, dir]) + .await + .with_context(|| format!("chown {dir}"))?; + if status.success() { + return Ok(()); + } + // Re-probe before deciding this matters. A concurrent repair, or a + // transient systemd-run/sudo failure on an already-correct tree, both land + // here with nothing actually wrong. + if ownership_already_correct_from_host(dir, host_uid_gid).await { + tracing::warn!( + dir = %dir, + target = %host_uid_gid, + "ownership repair chown failed but the ownership is already correct — continuing" + ); + return Ok(()); + } + Err(anyhow::anyhow!( + "chown {dir} failed with status {status} and ownership is still not {host_uid_gid}" + )) +} + async fn ownership_already_correct_from_host(path: &str, host_uid_gid: &str) -> bool { let Ok(out) = crate::update::host_sudo_output(&["stat", "-c", "%u:%g", path]).await else { return false; @@ -3252,36 +3296,19 @@ impl ProdContainerOrchestrator { } } + // These hooks exist to repair old installs, not to churn on every + // prepare — a healthy stack was being re-chowned on each reconcile + // tick (operator-visible journal flood, 2026-08-07). repair_dir_ownership + // skips when the ownership is already right, and tolerates a chown that + // fails on an already-correct tree. for dir in [ "/var/lib/archipelago/btcpay", "/var/lib/archipelago/nbxplorer", ] { - // These hooks exist to repair old installs, not to churn on every - // prepare — a healthy stack was being re-chowned on each reconcile - // tick (operator-visible journal flood, 2026-08-07). Skip when the - // ownership is already right. - if ownership_already_correct_from_host(dir, "1000:1000").await { - continue; - } - let status = host_sudo(&["chown", "-R", "1000:1000", dir]) - .await - .with_context(|| format!("chown {dir}"))?; - if !status.success() { - return Err(anyhow::anyhow!("chown {dir} failed with status {status}")); - } + repair_dir_ownership(dir, "1000:1000").await?; } - let db_dir = "/var/lib/archipelago/postgres-btcpay"; - if !ownership_already_correct_from_host(db_dir, "100998:100998").await { - let status = host_sudo(&["chown", "-R", "100998:100998", db_dir]) - .await - .with_context(|| format!("chown {db_dir}"))?; - if !status.success() { - return Err(anyhow::anyhow!( - "chown {db_dir} failed with status {status}" - )); - } - } + repair_dir_ownership("/var/lib/archipelago/postgres-btcpay", "100998:100998").await?; Ok(()) } @@ -3302,15 +3329,7 @@ impl ProdContainerOrchestrator { // (1000), not to subuid 100000. Repair old installs that were // chowned into the subuid range and crash on database.db.lock. // Drift-gated like the btcpay hook — see ensure_btcpay_stack_dirs. - if ownership_already_correct_from_host(dir, "1000:1000").await { - continue; - } - let chown = host_sudo(&["chown", "-R", "1000:1000", dir]) - .await - .with_context(|| format!("chown {dir}"))?; - if !chown.success() { - return Err(anyhow::anyhow!("chown {dir} failed with status {chown}")); - } + repair_dir_ownership(dir, "1000:1000").await?; } Ok(()) }