From 719446c05ff53fb74caf085e69acb531ba2e39e4 Mon Sep 17 00:00:00 2001 From: archipelago Date: Mon, 3 Aug 2026 18:55:28 -0400 Subject: [PATCH] fix(companion): stop the endless rebuild loop on *-ui companions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Observed live on archi-dev-box: archy-fedimint-ui rebuilt 45 times in 10 minutes, every ~35s, indefinitely. bitcoin-ui, lnd-ui and electrs-ui were all one reconcile away from the same loop. `context_is_newer_than_image` decides to rebuild when the build context's newest mtime is later than `podman image inspect .Created`. The rebuild that follows is a full layer-cache hit, so podman reuses the identical image and leaves .Created untouched — the condition that triggered the rebuild is still true afterwards. The check cannot converge: it rebuilds on every reconcile tick forever, burning CPU and churning the container. It bites after any deploy that refreshes /opt/archipelago/docker/*, which makes the contexts newer than the shipped images — so this is fleet-wide on every OTA, not local to one node. Fix: stamp the context mtime that was built into an image label and compare against that instead. A label is part of the image config, so a cache-hit build with a new value still produces a new image — the thing being tested does change, and the comparison settles after exactly one rebuild. Verified against real podman before writing it: two cache-hit builds with different label values produced distinct image IDs (6cfdbc9bcd3e vs a9b10eb9a558), each carrying its stamp; the indexed inspect format was checked against an image with real labels, and a missing label prints empty (handled, along with ""). Images built before this carry no label and fall back to .Created, so behaviour is unchanged for them and each self-heals on its first reconcile after upgrade — nodes fix themselves rather than needing the manual `podman build --no-cache` pass this needed by hand. Co-Authored-By: Claude Opus 5 (1M context) --- core/archipelago/src/container/companion.rs | 69 +++++++++++++++++++-- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/core/archipelago/src/container/companion.rs b/core/archipelago/src/container/companion.rs index cb86e701..92658725 100644 --- a/core/archipelago/src/container/companion.rs +++ b/core/archipelago/src/container/companion.rs @@ -252,8 +252,24 @@ async fn ensure_image_present(spec: &CompanionSpec) -> Result { } else { info!(companion = spec.name, "building locally from {dir}"); } + // Stamp the context mtime we are building, so the staleness + // check has something that advances even when every layer is a + // cache hit. Without this the rebuild is a no-op that leaves + // .Created unchanged, the check stays true, and the companion is + // rebuilt on every reconcile tick forever. + let context_stamp = newest_mtime_unix(PathBuf::from(dir)) + .await + .unwrap_or_default(); + let stamp_label = format!("{CONTEXT_STAMP_LABEL}={context_stamp}"); let out = command_output_with_timeout( - Command::new("podman").args(["build", "-t", &local_image, dir]), + Command::new("podman").args([ + "build", + "--label", + &stamp_label, + "-t", + &local_image, + dir, + ]), COMPANION_BUILD_TIMEOUT, "podman build companion image", ) @@ -322,17 +338,58 @@ async fn image_exists(image: &str) -> bool { /// already-built `image`, signalling the cached image is stale and must be /// rebuilt. Conservative: if either timestamp can't be determined we return /// false (reuse the cache) to avoid rebuild storms on every reconcile pass. +/// Label carrying the context mtime an image was built from. +/// +/// The reason this exists rather than reusing `.Created`: a rebuild whose +/// layers all hit the cache produces the SAME image, and podman leaves its +/// creation time untouched. Comparing against `.Created` therefore never +/// converges — the rebuild does not change the thing being tested, so the +/// companion is rebuilt on every reconcile tick indefinitely. A label is part +/// of the image config, so writing a new value always yields a new image, +/// which makes the comparison settle after exactly one rebuild. +const CONTEXT_STAMP_LABEL: &str = "org.archipelago.context-mtime"; + async fn context_is_newer_than_image(dir: &str, image: &str) -> bool { - let image_created = match image_created_unix(image).await { - Some(t) => t, - None => return false, + let Some(ctx) = newest_mtime_unix(PathBuf::from(dir)).await else { + return false; }; - match newest_mtime_unix(PathBuf::from(dir)).await { - Some(ctx) => ctx > image_created, + // Preferred: what the last build actually stamped. + if let Some(stamped) = image_context_stamp(image).await { + return ctx > stamped; + } + // Images built before stamping existed have no label. Fall back to the + // old comparison so behaviour is unchanged for them; the rebuild it + // triggers writes the label, so each such image self-heals exactly once. + match image_created_unix(image).await { + Some(created) => ctx > created, None => false, } } +/// The context mtime stamped into `image` at build time, if any. +async fn image_context_stamp(image: &str) -> Option { + let format = format!("{{{{index .Config.Labels \"{CONTEXT_STAMP_LABEL}\"}}}}"); + let mut cmd = Command::new("podman"); + cmd.args(["image", "inspect", "--format", &format, image]); + let out = command_output_with_timeout( + &mut cmd, + COMPANION_IMAGE_CHECK_TIMEOUT, + "podman image context stamp", + ) + .await + .ok()?; + if !out.status.success() { + return None; + } + let raw = String::from_utf8_lossy(&out.stdout); + let raw = raw.trim(); + // podman prints "" for a missing label. + if raw.is_empty() || raw == "" { + return None; + } + raw.parse::().ok() +} + /// Build timestamp of `image` as Unix seconds, via `podman image inspect`. async fn image_created_unix(image: &str) -> Option { let mut cmd = Command::new("podman");