fix(companion): stop the endless rebuild loop on *-ui companions
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 "<no value>"). 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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3716b6e9c3
commit
719446c05f
@@ -252,8 +252,24 @@ async fn ensure_image_present(spec: &CompanionSpec) -> Result<String> {
|
||||
} 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<i64> {
|
||||
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 "<no value>" for a missing label.
|
||||
if raw.is_empty() || raw == "<no value>" {
|
||||
return None;
|
||||
}
|
||||
raw.parse::<i64>().ok()
|
||||
}
|
||||
|
||||
/// Build timestamp of `image` as Unix seconds, via `podman image inspect`.
|
||||
async fn image_created_unix(image: &str) -> Option<i64> {
|
||||
let mut cmd = Command::new("podman");
|
||||
|
||||
Reference in New Issue
Block a user