huskies: merge 1231 story project-rebuild must not silently downgrade a live sled to the stale image binary
This commit is contained in:
@@ -6,6 +6,17 @@ set -euo pipefail
|
||||
#
|
||||
# Run this after `script/docker_rebuild` or whenever you add a new stack.
|
||||
# Safe to re-run: each step re-tags the image with the latest layers.
|
||||
#
|
||||
# IMPORTANT (story 1231): this script is NOT part of `script/release`. The
|
||||
# huskies-project-* images bake whatever `huskies` binary happened to be built
|
||||
# locally the last time this ran — as of this comment that's 0.13.0, several
|
||||
# releases behind the current fleet artifact. `project-rebuild` self-heals a
|
||||
# sled that comes back on a stale baked binary (it re-upgrades it in place
|
||||
# from the gateway's published artifact), but that costs an extra
|
||||
# download+restart cycle every time. Run `script/build-project-images` after
|
||||
# every `script/release` — ideally as a step in the release flow itself — so
|
||||
# freshly rebuilt sleds start on a current binary instead of relying on the
|
||||
# self-heal.
|
||||
|
||||
cd "$(dirname "$0")/.."
|
||||
|
||||
|
||||
@@ -1,6 +1,16 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# NOTE (story 1231): this script publishes the `huskies` binary artifact
|
||||
# (consumed by the `upgrade`/`upgrade all` chat commands) but does NOT
|
||||
# rebuild the huskies-project-* Docker images that `project-rebuild` uses.
|
||||
# Those images bake whatever binary was locally built the last time
|
||||
# `script/build-project-images` ran, so they silently drift stale otherwise
|
||||
# (currently baking 0.13.0). Run `script/build-project-images` after this
|
||||
# script — or better, fold it into this release flow — so a `project-rebuild`
|
||||
# right after a release doesn't need to self-heal a stale sled back up to the
|
||||
# version just published here.
|
||||
|
||||
# ── Configuration ──────────────────────────────────────────────
|
||||
GITEA_URL="https://code.crashlabs.io"
|
||||
REPO="crashlabs/huskies"
|
||||
|
||||
@@ -900,6 +900,7 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message(
|
||||
rebuild_cmd.drain_timeout_secs,
|
||||
rebuild_cmd.force,
|
||||
store,
|
||||
ctx.gateway_port,
|
||||
&ctx.services.project_root,
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -20,6 +20,7 @@ use crate::service::gateway::io::save_config;
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::RwLock;
|
||||
|
||||
/// Default seconds to wait for in-flight work to drain before refusing.
|
||||
@@ -101,6 +102,7 @@ pub async fn handle_project_rebuild(
|
||||
drain_timeout_secs: u64,
|
||||
force: bool,
|
||||
projects_store: &Arc<RwLock<BTreeMap<String, ProjectEntry>>>,
|
||||
gateway_port: Option<u16>,
|
||||
config_dir: &Path,
|
||||
) -> String {
|
||||
// ── 1. Validate project ──────────────────────────────────────────────────
|
||||
@@ -140,6 +142,21 @@ pub async fn handle_project_rebuild(
|
||||
);
|
||||
}
|
||||
|
||||
// ── 1b. Capture the running binary's git_hash before we tear it down ──────
|
||||
// Best-effort: an unreachable or pre-version-endpoint sled just yields `None`,
|
||||
// in which case the post-rebuild reconciliation below can't compare old vs
|
||||
// new and falls back to whatever the published fleet artifact says.
|
||||
let http_client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(15))
|
||||
.build()
|
||||
.unwrap_or_default();
|
||||
let old_git_hash = match project_url.as_deref() {
|
||||
Some(url) => super::sled_upgrade::fetch_sled_version(&http_client, url)
|
||||
.await
|
||||
.map(|(_version, hash)| hash),
|
||||
None => None,
|
||||
};
|
||||
|
||||
// ── 2. Drain check ───────────────────────────────────────────────────────
|
||||
let container_name = format!("huskies-{name}");
|
||||
if !force
|
||||
@@ -288,8 +305,8 @@ pub async fn handle_project_rebuild(
|
||||
let container_short: String = container_id.chars().take(12).collect();
|
||||
|
||||
// ── 6. Persist updated config (URL is unchanged; project already registered) ────
|
||||
let container_url = format!("http://127.0.0.1:{port}");
|
||||
{
|
||||
let container_url = format!("http://127.0.0.1:{port}");
|
||||
let mut projects = projects_store.write().await;
|
||||
if let Some(entry) = projects.get_mut(name) {
|
||||
entry.url = Some(container_url.clone());
|
||||
@@ -300,6 +317,20 @@ pub async fn handle_project_rebuild(
|
||||
|
||||
crate::slog!("[project-rebuild] Rebuilt '{name}': image={image_hash} container={container_id}");
|
||||
|
||||
// ── 7. Make sure the rebuilt sled isn't running an older binary than before ──
|
||||
let artifact_source = super::sled_upgrade::resolve_artifact_source(gateway_port);
|
||||
let version_note = reconcile_post_rebuild_version(
|
||||
name,
|
||||
old_git_hash.as_deref(),
|
||||
&container_url,
|
||||
artifact_source,
|
||||
projects_store,
|
||||
config_dir,
|
||||
&http_client,
|
||||
60,
|
||||
)
|
||||
.await;
|
||||
|
||||
format!(
|
||||
"Project **{name}** rebuilt.\n\
|
||||
- New image: `{image}` (`{image_short}…`)\n\
|
||||
@@ -307,10 +338,103 @@ pub async fn handle_project_rebuild(
|
||||
- State: `pipeline.db` and CRDT preserved (same volume bind-mount)\n\
|
||||
- Port: {port} (unchanged)\n\
|
||||
\n\
|
||||
Use `switch {name}` then `status` to verify the pipeline."
|
||||
Use `switch {name}` then `status` to verify the pipeline.{version_note}"
|
||||
)
|
||||
}
|
||||
|
||||
/// Ensure the freshly-rebuilt sled at `container_url` isn't running an older
|
||||
/// binary than the one it had before the rebuild (AC 1–3, story 1231).
|
||||
///
|
||||
/// Base images are only refreshed by a manual `script/build-project-images`
|
||||
/// run, so `project-rebuild` can otherwise silently downgrade a sled that was
|
||||
/// live-upgraded past whatever binary is baked into the image. This waits for
|
||||
/// the new container to come up, compares its `git_hash` against the
|
||||
/// gateway's published fleet artifact, and — when they differ — self-heals by
|
||||
/// running the same in-place upgrade `upgrade <name>` uses. When there's no
|
||||
/// published artifact to converge to, it falls back to comparing against the
|
||||
/// pre-rebuild hash so a real downgrade is at least surfaced as a warning
|
||||
/// instead of passing silently.
|
||||
///
|
||||
/// `artifact_source` is the already-resolved published-artifact lookup (see
|
||||
/// [`super::sled_upgrade::resolve_artifact_source`]) — `Err` means no fleet
|
||||
/// artifact has been published yet.
|
||||
///
|
||||
/// Returns a Markdown snippet (starting with `\n\n`) to append to the rebuild
|
||||
/// reply, or an empty string when the sled already matches the current build.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn reconcile_post_rebuild_version(
|
||||
name: &str,
|
||||
old_git_hash: Option<&str>,
|
||||
container_url: &str,
|
||||
artifact_source: Result<(String, Option<String>), String>,
|
||||
projects_store: &Arc<RwLock<BTreeMap<String, ProjectEntry>>>,
|
||||
config_dir: &Path,
|
||||
client: &reqwest::Client,
|
||||
health_timeout_secs: u64,
|
||||
) -> String {
|
||||
let health_url = format!("{}/health", container_url.trim_end_matches('/'));
|
||||
if !super::sled_upgrade::wait_for_health(client, &health_url, health_timeout_secs).await {
|
||||
return format!(
|
||||
"\n\n⚠️ **Warning:** the rebuilt sled did not respond to `/health` within 60s — \
|
||||
cannot verify its binary version. Check `docker logs huskies-{name}` and run \
|
||||
`upgrade {name}` once it's reachable."
|
||||
);
|
||||
}
|
||||
|
||||
let Some((_new_version, new_hash)) =
|
||||
super::sled_upgrade::fetch_sled_version(client, container_url).await
|
||||
else {
|
||||
return "\n\n⚠️ **Warning:** the rebuilt sled is healthy but `/api/version` is unavailable — \
|
||||
cannot verify it isn't running a stale binary baked into the base image."
|
||||
.to_string();
|
||||
};
|
||||
|
||||
let (source_url, expected_hash) = match artifact_source {
|
||||
Ok(v) => v,
|
||||
Err(_) => {
|
||||
// No published fleet artifact to converge to — the best we can do is
|
||||
// flag an actual change from what was running before the rebuild.
|
||||
return match old_git_hash {
|
||||
Some(old) if old != new_hash => format!(
|
||||
"\n\n⚠️ **Warning:** the rebuilt sled is now on `{new_hash}` (was `{old}`) and \
|
||||
no fleet artifact is published to verify or fix this. Run `release` then \
|
||||
`upgrade {name}`."
|
||||
),
|
||||
_ => String::new(),
|
||||
};
|
||||
}
|
||||
};
|
||||
|
||||
if expected_hash.as_deref() == Some(new_hash.as_str()) {
|
||||
return String::new(); // base image already bakes the current fleet build.
|
||||
}
|
||||
|
||||
// The base image baked an older (or merely different) binary than the fleet
|
||||
// is currently on — self-heal the same way `upgrade <name>` would.
|
||||
let outcome = super::sled_upgrade::run_sled_upgrade(
|
||||
name,
|
||||
container_url,
|
||||
&source_url,
|
||||
expected_hash,
|
||||
projects_store,
|
||||
config_dir,
|
||||
|_| async {},
|
||||
)
|
||||
.await;
|
||||
|
||||
if outcome.contains("matches published artifact") || outcome.starts_with("upgraded to v") {
|
||||
format!(
|
||||
"\n\n♻️ The base image baked an older binary (`{new_hash}`); self-healed in place — \
|
||||
{outcome}"
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"\n\n⚠️ **Warning:** the base image baked an older binary (`{new_hash}`) and the \
|
||||
automatic self-heal failed: {outcome}\nRun `upgrade {name}` manually."
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// Wait for active Claude agent processes in the container to exit.
|
||||
///
|
||||
/// Polls every 5 seconds until no `claude` processes remain or `timeout_secs` elapses.
|
||||
@@ -510,7 +634,7 @@ mod tests {
|
||||
async fn rebuild_unknown_project_returns_error() {
|
||||
let store = make_store(vec![]);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let result = handle_project_rebuild("nonexistent", 0, true, &store, dir.path()).await;
|
||||
let result = handle_project_rebuild("nonexistent", 0, true, &store, None, dir.path()).await;
|
||||
assert!(
|
||||
result.contains("not found"),
|
||||
"expected 'not found': {result}"
|
||||
@@ -530,7 +654,7 @@ mod tests {
|
||||
},
|
||||
)]);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let result = handle_project_rebuild("myapp", 0, true, &store, dir.path()).await;
|
||||
let result = handle_project_rebuild("myapp", 0, true, &store, None, dir.path()).await;
|
||||
assert!(
|
||||
result.contains("host_path"),
|
||||
"expected 'host_path' mention: {result}"
|
||||
@@ -550,7 +674,7 @@ mod tests {
|
||||
},
|
||||
)]);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let result = handle_project_rebuild("myapp", 0, true, &store, dir.path()).await;
|
||||
let result = handle_project_rebuild("myapp", 0, true, &store, None, dir.path()).await;
|
||||
assert!(
|
||||
result.contains("does not exist"),
|
||||
"expected 'does not exist': {result}"
|
||||
@@ -586,7 +710,8 @@ mod tests {
|
||||
)]);
|
||||
let config_dir = tempfile::tempdir().unwrap();
|
||||
|
||||
let result = handle_project_rebuild("myapp", 0, true, &store, config_dir.path()).await;
|
||||
let result =
|
||||
handle_project_rebuild("myapp", 0, true, &store, None, config_dir.path()).await;
|
||||
|
||||
// (a) Step naming: one of several possible failure steps depending on what Docker
|
||||
// binaries are available in the test environment, or a success reply.
|
||||
@@ -607,4 +732,181 @@ mod tests {
|
||||
"project 'myapp' must remain registered after failed rebuild: {result}"
|
||||
);
|
||||
}
|
||||
|
||||
// ── reconcile_post_rebuild_version (AC 1–3) ────────────────────────────
|
||||
|
||||
/// Spawn a minimal HTTP server that answers every `/health` request with
|
||||
/// 200 and every `/api/version` request with `git_hash`. Serves
|
||||
/// connections in a loop so both `wait_for_health` and
|
||||
/// `fetch_sled_version` can hit it independently within one test.
|
||||
fn spawn_version_server(git_hash: &str) -> (String, tokio::task::JoinHandle<()>) {
|
||||
let git_hash = git_hash.to_string();
|
||||
let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
listener.set_nonblocking(true).unwrap();
|
||||
let listener = tokio::net::TcpListener::from_std(listener).unwrap();
|
||||
let port = listener.local_addr().unwrap().port();
|
||||
let handle = tokio::spawn(async move {
|
||||
loop {
|
||||
let Ok((mut stream, _)) = listener.accept().await else {
|
||||
break;
|
||||
};
|
||||
let git_hash = git_hash.clone();
|
||||
tokio::spawn(async move {
|
||||
use tokio::io::{AsyncReadExt, AsyncWriteExt};
|
||||
let mut buf = [0u8; 4096];
|
||||
let n = stream.read(&mut buf).await.unwrap_or(0);
|
||||
let req = String::from_utf8_lossy(&buf[..n]);
|
||||
let path = req.lines().next().unwrap_or("").to_string();
|
||||
let body = if path.contains("/api/version") {
|
||||
serde_json::json!({"version": "0.14.2", "git_hash": git_hash}).to_string()
|
||||
} else {
|
||||
"ok".to_string()
|
||||
};
|
||||
let resp = format!(
|
||||
"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}",
|
||||
body.len(),
|
||||
body
|
||||
);
|
||||
let _ = stream.write_all(resp.as_bytes()).await;
|
||||
});
|
||||
}
|
||||
});
|
||||
(format!("http://127.0.0.1:{port}"), handle)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_empty_when_new_hash_matches_published_artifact() {
|
||||
let (url, srv) = spawn_version_server("current-hash");
|
||||
let store = make_store(vec![]);
|
||||
let config_dir = tempfile::tempdir().unwrap();
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
let note = reconcile_post_rebuild_version(
|
||||
"myapp",
|
||||
Some("old-hash"),
|
||||
&url,
|
||||
Ok((
|
||||
"http://unused".to_string(),
|
||||
Some("current-hash".to_string()),
|
||||
)),
|
||||
&store,
|
||||
config_dir.path(),
|
||||
&client,
|
||||
2,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(note, "", "already-current build must produce no note");
|
||||
srv.abort();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_empty_when_no_artifact_and_hash_unchanged() {
|
||||
let (url, srv) = spawn_version_server("same-hash");
|
||||
let store = make_store(vec![]);
|
||||
let config_dir = tempfile::tempdir().unwrap();
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
let note = reconcile_post_rebuild_version(
|
||||
"myapp",
|
||||
Some("same-hash"),
|
||||
&url,
|
||||
Err("no published artifact".to_string()),
|
||||
&store,
|
||||
config_dir.path(),
|
||||
&client,
|
||||
2,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(note, "", "unchanged hash must produce no note");
|
||||
srv.abort();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_warns_when_no_artifact_and_hash_changed() {
|
||||
let (url, srv) = spawn_version_server("stale-baked-hash");
|
||||
let store = make_store(vec![]);
|
||||
let config_dir = tempfile::tempdir().unwrap();
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
let note = reconcile_post_rebuild_version(
|
||||
"myapp",
|
||||
Some("newer-hash-that-was-running"),
|
||||
&url,
|
||||
Err("no published artifact".to_string()),
|
||||
&store,
|
||||
config_dir.path(),
|
||||
&client,
|
||||
2,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(note.contains("Warning"), "expected a warning: {note}");
|
||||
assert!(
|
||||
note.contains("stale-baked-hash") && note.contains("newer-hash-that-was-running"),
|
||||
"warning should name both hashes: {note}"
|
||||
);
|
||||
srv.abort();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_warns_when_health_probe_fails() {
|
||||
let store = make_store(vec![]);
|
||||
let config_dir = tempfile::tempdir().unwrap();
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(std::time::Duration::from_millis(200))
|
||||
.build()
|
||||
.unwrap();
|
||||
|
||||
let note = reconcile_post_rebuild_version(
|
||||
"myapp",
|
||||
Some("old-hash"),
|
||||
"http://127.0.0.1:1", // nothing listens on port 1
|
||||
Err("no published artifact".to_string()),
|
||||
&store,
|
||||
config_dir.path(),
|
||||
&client,
|
||||
2,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
note.contains("did not respond to `/health`"),
|
||||
"expected a health-probe warning: {note}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_attempts_self_heal_and_reports_failure_when_artifact_unreachable() {
|
||||
let (url, srv) = spawn_version_server("stale-baked-hash");
|
||||
let store = make_store(vec![]);
|
||||
let config_dir = tempfile::tempdir().unwrap();
|
||||
let client = reqwest::Client::new();
|
||||
|
||||
let note = reconcile_post_rebuild_version(
|
||||
"myapp",
|
||||
Some("stale-baked-hash"),
|
||||
&url,
|
||||
Ok((
|
||||
"http://127.0.0.1:1/api/artifacts/huskies-linux-arm64".to_string(),
|
||||
Some("current-fleet-hash".to_string()),
|
||||
)),
|
||||
&store,
|
||||
config_dir.path(),
|
||||
&client,
|
||||
2,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
note.contains("self-heal failed"),
|
||||
"mismatch should trigger a self-heal attempt that reports failure: {note}"
|
||||
);
|
||||
assert!(
|
||||
note.contains("stale-baked-hash"),
|
||||
"note should name the stale hash: {note}"
|
||||
);
|
||||
srv.abort();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -181,7 +181,9 @@ pub async fn handle_upgrade_list_projects(
|
||||
///
|
||||
/// Returns `Err` with a user-facing message when no artifact has been
|
||||
/// published yet.
|
||||
fn resolve_artifact_source(gateway_port: Option<u16>) -> Result<(String, Option<String>), String> {
|
||||
pub(crate) fn resolve_artifact_source(
|
||||
gateway_port: Option<u16>,
|
||||
) -> Result<(String, Option<String>), String> {
|
||||
if let Ok(url) = std::env::var("HUSKIES_GATEWAY_BINARY_URL") {
|
||||
return Ok((url, None));
|
||||
}
|
||||
@@ -311,7 +313,7 @@ where
|
||||
/// resolved. Split from [`handle_sled_upgrade`] so tests can drive the wire
|
||||
/// behaviour without a published artifact on the host.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn run_sled_upgrade<F, Fut>(
|
||||
pub(crate) async fn run_sled_upgrade<F, Fut>(
|
||||
project: &str,
|
||||
sled_url: &str,
|
||||
source_url: &str,
|
||||
@@ -427,7 +429,11 @@ where
|
||||
/// Poll `GET {health_url}` every 3 seconds until it returns 200 or `timeout_secs` elapses.
|
||||
///
|
||||
/// Returns `true` when the probe succeeds, `false` on timeout.
|
||||
async fn wait_for_health(client: &reqwest::Client, health_url: &str, timeout_secs: u64) -> bool {
|
||||
pub(crate) async fn wait_for_health(
|
||||
client: &reqwest::Client,
|
||||
health_url: &str,
|
||||
timeout_secs: u64,
|
||||
) -> bool {
|
||||
let deadline = std::time::Instant::now() + Duration::from_secs(timeout_secs);
|
||||
let poll = Duration::from_secs(3);
|
||||
loop {
|
||||
@@ -446,7 +452,10 @@ async fn wait_for_health(client: &reqwest::Client, health_url: &str, timeout_sec
|
||||
///
|
||||
/// Returns `None` when the endpoint is unreachable or malformed — e.g. a sled
|
||||
/// still running a binary that predates the endpoint.
|
||||
async fn fetch_sled_version(client: &reqwest::Client, sled_url: &str) -> Option<(String, String)> {
|
||||
pub(crate) async fn fetch_sled_version(
|
||||
client: &reqwest::Client,
|
||||
sled_url: &str,
|
||||
) -> Option<(String, String)> {
|
||||
let url = format!("{}/api/version", sled_url.trim_end_matches('/'));
|
||||
let val: serde_json::Value = client.get(&url).send().await.ok()?.json().await.ok()?;
|
||||
let version = val.get("version").and_then(|v| v.as_str())?.to_string();
|
||||
|
||||
@@ -1211,6 +1211,7 @@ async fn handle_project_rebuild_tool(
|
||||
drain_timeout_secs,
|
||||
force,
|
||||
&state.projects,
|
||||
Some(state.port),
|
||||
&state.config_dir,
|
||||
)
|
||||
.await;
|
||||
|
||||
Reference in New Issue
Block a user