浏览代码

feat(serve): warn when image checksum doesn't match Dockerfile dir

serve without --build-node skipped handle_build entirely, so a bumped
Dockerfile (e.g. FROM production-hybrid-1.3 → 1.4) silently served the
stale image: the launcher trusted the existing tag and synced it by
digest (digests match — same stale image on every node).

Add a non-blocking staleness check in the serve flow: when the recipe
declares docker_image_dir and --build-node is not given, compute the
current docker-images/<dir>/ checksum (folded with [build.versions])
and compare it to the image's sm121.checksum label on the head node.
On mismatch (or missing label), WARN with the rebuild hint, then
proceed — the operator may intentionally serve a known image.

Runs on dry-run too (read-only `docker inspect`), so staleness is
visible without starting containers.

Verified on live cluster: `sm121 -d serve --recipe glm52-mxfp4-experts-gptq`
emits "Image 'sparkrun-vllm-ds4-gb10:glm52' is STALE: sm121.checksum
label 'a4eaa928…' != expected '4daa35f1…'".

build_checksum is now pub(crate) so stale.rs can reuse the exact
checksum logic the build step stamps.
Alexander Korolev 2 月之前
父节点
当前提交
63b5518bb5
共有 3 个文件被更改,包括 97 次插入 和 1 次删除
  1. 1 1
      src/docker/build.rs
  2. 6 0
      src/docker/mod.rs
  3. 90 0
      src/docker/stale.rs

+ 1 - 1
src/docker/build.rs

@@ -96,7 +96,7 @@ fn serialize_versions_env(pins: &BTreeMap<String, String>) -> String {
 ///   `versions.env` with recipe pins applied (recipe wins) for the launcher
 ///   to write to the remote build dir; `combined` hashes the dir checksum
 ///   together with the merged env so any override change forces a rebuild.
-fn build_checksum(
+pub(crate) fn build_checksum(
     local_dir: &Path,
     overrides: &HashMap<String, String>,
 ) -> Result<(Option<String>, String)> {

+ 6 - 0
src/docker/mod.rs

@@ -6,6 +6,7 @@ pub mod health;
 pub mod logs;
 pub mod mods;
 pub use logs::show_logs;
+pub mod stale;
 pub mod sync;
 pub mod tail;
 use crate::{
@@ -158,6 +159,11 @@ pub async fn serve(
     if image != recipe.docker_image {
         recipe.docker_image = image.clone();
     }
+    // Surface image staleness (bumped Dockerfile vs image checksum label)
+    // when not building. Non-blocking; runs on dry-run too (read-only inspect).
+    if opts.build_node.is_none() {
+        stale::warn_if_stale(config, &cluster, &recipe).await?;
+    }
     // Sync Docker image across nodes if needed.
     let should_sync =
         opts.sync_image && recipe.sync_image && !opts.dry_run && cluster.nodes.len() > 1;

+ 90 - 0
src/docker/stale.rs

@@ -0,0 +1,90 @@
+//! Serve-time image staleness check.
+//!
+//! When a recipe declares `docker_image_dir`, sm121 owns the image's build
+//! and stamps a `sm121.checksum` label on it (see [`super::build`]). A
+//! `serve` **without** `--build-node` skips [`build::handle_build`] — so
+//! nothing compares that label to the current `docker-images/<dir>/`
+//! checksum. This module closes that gap: it computes the expected checksum
+//! locally, reads the label off the head node's image, and WARNs on
+//! mismatch (non-blocking — the operator may intentionally serve a known
+//! image). A stale image is still started; this only surfaces the drift.
+
+use super::build::build_checksum;
+use super::shq;
+use crate::config::{Cluster, Config, Recipe};
+use crate::ssh::run_ssh;
+use anyhow::{Context, Result};
+use std::path::Path;
+use tracing::warn;
+
+/// Warn if the image that `serve` is about to use carries a
+/// `sm121.checksum` label that doesn't match the current
+/// `docker-images/<dir>/` checksum (folded with the recipe's
+/// `[build.versions]` overrides). No-op when the recipe has no
+/// `docker_image_dir` or the build dir is absent.
+pub(crate) async fn warn_if_stale(
+    config: &Config,
+    cluster: &Cluster,
+    recipe: &Recipe,
+) -> Result<()> {
+    let Some(dir) = &recipe.docker_image_dir else {
+        return Ok(());
+    };
+    let local = Path::new("docker-images").join(dir);
+    if !local.join("build.sh").exists() {
+        return Ok(());
+    }
+    let (_merged, expected) = build_checksum(&local, &recipe.build.versions)?;
+    let head = cluster.nodes.first().context("No nodes in cluster")?;
+    let label = run_ssh(
+        config,
+        head,
+        format!(
+            "docker inspect --format '{{{{index .Config.Labels \"sm121.checksum\"}}}}' {} 2>/dev/null || echo MISSING",
+            shq(&recipe.docker_image)
+        ),
+        false,
+    )
+    .await?;
+    if is_stale(&label, &expected) {
+        warn!(
+            "Image '{}' is STALE: sm121.checksum label '{}' != expected '{}' \
+             (docker-images/{} changed since last build). \
+             Rebuild with: sm121 build --recipe <name> --build-node <node>",
+            recipe.docker_image,
+            label.trim(),
+            expected,
+            dir,
+        );
+    }
+    Ok(())
+}
+
+/// Pure decision used by [`warn_if_stale`]: does the label read off the
+/// image differ from the expected checksum? Trims whitespace (SSH output
+/// may carry a trailing newline). `MISSING` (no label / pre-sm121 image)
+/// counts as stale.
+fn is_stale(label: &str, expected: &str) -> bool {
+    label.trim() != expected
+}
+
+#[cfg(test)]
+mod tests {
+    use super::is_stale;
+
+    #[test]
+    fn stale_when_label_differs() {
+        assert!(is_stale("a4eaa928\n", "4daa35f1"));
+    }
+
+    #[test]
+    fn fresh_when_label_matches() {
+        assert!(!is_stale("4daa35f1\n", "4daa35f1"));
+    }
+
+    #[test]
+    fn stale_when_label_missing() {
+        assert!(is_stale("MISSING\n", "4daa35f1"));
+        assert!(is_stale("", "4daa35f1"));
+    }
+}