fix(container): a no-op ownership repair must not fail the whole reconcile
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
8908fb4ff9
commit
b57f363745
@@ -361,6 +361,50 @@ fn ownership_already_correct(path: &str, host_uid_gid: &str) -> bool {
|
|||||||
md.uid() == uid && md.gid() == gid
|
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 {
|
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 {
|
let Ok(out) = crate::update::host_sudo_output(&["stat", "-c", "%u:%g", path]).await else {
|
||||||
return false;
|
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 [
|
for dir in [
|
||||||
"/var/lib/archipelago/btcpay",
|
"/var/lib/archipelago/btcpay",
|
||||||
"/var/lib/archipelago/nbxplorer",
|
"/var/lib/archipelago/nbxplorer",
|
||||||
] {
|
] {
|
||||||
// These hooks exist to repair old installs, not to churn on every
|
repair_dir_ownership(dir, "1000:1000").await?;
|
||||||
// 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}"));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
let db_dir = "/var/lib/archipelago/postgres-btcpay";
|
repair_dir_ownership("/var/lib/archipelago/postgres-btcpay", "100998:100998").await?;
|
||||||
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}"
|
|
||||||
));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3302,15 +3329,7 @@ impl ProdContainerOrchestrator {
|
|||||||
// (1000), not to subuid 100000. Repair old installs that were
|
// (1000), not to subuid 100000. Repair old installs that were
|
||||||
// chowned into the subuid range and crash on database.db.lock.
|
// chowned into the subuid range and crash on database.db.lock.
|
||||||
// Drift-gated like the btcpay hook — see ensure_btcpay_stack_dirs.
|
// Drift-gated like the btcpay hook — see ensure_btcpay_stack_dirs.
|
||||||
if ownership_already_correct_from_host(dir, "1000:1000").await {
|
repair_dir_ownership(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}"));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user