fix(hive-c0re): close review findings on the job-DAG queue
- deploy-window gate (meta::exclusive) + path-limited meta commits: a perm/lock/topology commit can no longer sweep an ApprovalDeploy's staged flake.lock and neuter abort_deploy (regression test included) - cancel surfaces now buffer terminal roll-ups the scheduler drains, so a queued approval DAG cancelled by the operator resolves its approval instead of dangling, and cancelled power ops revert their wanted flip to the observed state - hivectl restart / restart-all ride the queue (lease serialization, transient guard) and restart sets wanted=Up like the old kill+start - exactly one Rebuilt event per rebuild DAG, emitted at terminal - StopForUpdate pre-seeds a missing agent_power row from the pre-stop observation so a rebuild can't strand an unknown agent offline - history trim keeps terminal fan-out parents with live children - audit_log back on db::open; swarm.js badge for reconcile DAGs
This commit is contained in:
parent
58e86a3adf
commit
084e12503c
12 changed files with 448 additions and 160 deletions
|
|
@ -29,6 +29,25 @@ const GIT_EMAIL: &str = "c0re@hyperhive.local";
|
|||
/// take turns instead of colliding.
|
||||
static META_LOCK: Mutex<()> = Mutex::const_new(());
|
||||
|
||||
/// Coarse exclusivity for meta-repo *windows* that span multiple
|
||||
/// `META_LOCK` acquisitions — above all the two-phase deploy
|
||||
/// (`prepare_deploy` stages `flake.lock` uncommitted for the whole
|
||||
/// container build; `finalize_deploy` / `abort_deploy` resolve it).
|
||||
/// `META_LOCK` serializes individual git ops but cannot keep another
|
||||
/// op out of that staged window: a perm-file or lock-bump commit
|
||||
/// landing mid-window would sweep the staged deploy lock into its own
|
||||
/// commit and neuter `abort_deploy`. Job-queue executors that mutate
|
||||
/// the meta repo hold this gate for their mutation span; the opaque
|
||||
/// approval-deploy node holds it across its whole prepare→finalize
|
||||
/// span. Never acquired inside this module's functions (they run
|
||||
/// *under* a caller's window — nesting would deadlock).
|
||||
static DEPLOY_GATE: Mutex<()> = Mutex::const_new(());
|
||||
|
||||
/// Acquire the deploy/meta-mutation window gate. See [`DEPLOY_GATE`].
|
||||
pub async fn exclusive() -> tokio::sync::MutexGuard<'static, ()> {
|
||||
DEPLOY_GATE.lock().await
|
||||
}
|
||||
|
||||
/// Where the manager sees this directory inside its container (RO bind).
|
||||
pub const CONTAINER_MANAGER_META_MOUNT: &str = "/meta";
|
||||
|
||||
|
|
@ -239,11 +258,16 @@ pub async fn prepare_deploy(name: &str) -> Result<()> {
|
|||
pub async fn finalize_deploy(name: &str, sha: &str, tag: &str) -> Result<()> {
|
||||
let _guard = META_LOCK.lock().await;
|
||||
let dir = meta_dir();
|
||||
if !has_staged_changes(&dir).await? {
|
||||
if !paths_dirty(&dir, &["flake.lock"]).await? {
|
||||
return Ok(());
|
||||
}
|
||||
let short = &sha[..sha.len().min(12)];
|
||||
git_commit(&dir, &format!("deploy {name} {tag} {short}")).await
|
||||
git_commit_paths(
|
||||
&dir,
|
||||
&format!("deploy {name} {tag} {short}"),
|
||||
&["flake.lock"],
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Phase 2-failure. Unstage + restore the lock so meta returns to
|
||||
|
|
@ -256,21 +280,6 @@ pub async fn abort_deploy() -> Result<()> {
|
|||
git(&dir, &["restore", "flake.lock"]).await
|
||||
}
|
||||
|
||||
async fn has_staged_changes(dir: &Path) -> Result<bool> {
|
||||
let st = lifecycle::git_command()
|
||||
.current_dir(dir)
|
||||
.args(["diff", "--cached", "--quiet"])
|
||||
.status()
|
||||
.await
|
||||
.with_context(|| format!("git diff --cached in {}", dir.display()))?;
|
||||
// exit 1 = differences present, 0 = no diff, other = error
|
||||
match st.code() {
|
||||
Some(0) => Ok(false),
|
||||
Some(1) => Ok(true),
|
||||
_ => bail!("git diff --cached exited unexpectedly"),
|
||||
}
|
||||
}
|
||||
|
||||
/// One-shot used by the manual-rebuild path: relock just one
|
||||
/// agent's input and commit the lock change if any. Single-phase
|
||||
/// (no separate finalize) because rebuild has no failure-revert
|
||||
|
|
@ -280,11 +289,16 @@ pub async fn lock_update_for_rebuild(name: &str) -> Result<()> {
|
|||
let dir = meta_dir();
|
||||
let input = format!("agent-{name}");
|
||||
nix(&dir, &["flake", "update", &input]).await?;
|
||||
if git_is_clean(&dir).await? {
|
||||
if !paths_dirty(&dir, &["flake.lock"]).await? {
|
||||
return Ok(());
|
||||
}
|
||||
git(&dir, &["add", "flake.lock"]).await?;
|
||||
git_commit(&dir, &format!("rebuild {name}: lock update")).await
|
||||
git_commit_paths(
|
||||
&dir,
|
||||
&format!("rebuild {name}: lock update"),
|
||||
&["flake.lock"],
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Build the `--override-input` value pinning an agent's config repo to
|
||||
|
|
@ -349,7 +363,7 @@ pub async fn lock_update(inputs: &[String]) -> Result<()> {
|
|||
args.push(i.as_str());
|
||||
}
|
||||
nix(&dir, &args).await?;
|
||||
if git_is_clean(&dir).await? {
|
||||
if !paths_dirty(&dir, &["flake.lock"]).await? {
|
||||
return Ok(());
|
||||
}
|
||||
git(&dir, &["add", "flake.lock"]).await?;
|
||||
|
|
@ -360,7 +374,7 @@ pub async fn lock_update(inputs: &[String]) -> Result<()> {
|
|||
} else {
|
||||
format!("lock update: {}", inputs.join(", "))
|
||||
};
|
||||
git_commit(&dir, &msg).await
|
||||
git_commit_paths(&dir, &msg, &["flake.lock"]).await
|
||||
}
|
||||
|
||||
/// One-shot used by the auto-update path: pin the latest hyperhive
|
||||
|
|
@ -370,11 +384,11 @@ pub async fn lock_update_hyperhive() -> Result<()> {
|
|||
let _guard = META_LOCK.lock().await;
|
||||
let dir = meta_dir();
|
||||
nix(&dir, &["flake", "update", "hyperhive"]).await?;
|
||||
if git_is_clean(&dir).await? {
|
||||
if !paths_dirty(&dir, &["flake.lock"]).await? {
|
||||
return Ok(());
|
||||
}
|
||||
git(&dir, &["add", "flake.lock"]).await?;
|
||||
git_commit(&dir, "bump hyperhive").await
|
||||
git_commit_paths(&dir, "bump hyperhive", &["flake.lock"]).await
|
||||
}
|
||||
|
||||
/// Write the tool-groups file for `agent` and commit it atomically
|
||||
|
|
@ -388,8 +402,13 @@ pub async fn commit_tool_groups(agent: &str, groups: &[String]) -> Result<()> {
|
|||
if crate::tool_groups::tool_groups_path().exists() {
|
||||
git(&dir, &["add", "tool-groups.json"]).await?;
|
||||
}
|
||||
if has_staged_changes(&dir).await? {
|
||||
git_commit(&dir, &format!("set tool-groups for {agent}")).await?;
|
||||
if paths_dirty(&dir, &["tool-groups.json"]).await? {
|
||||
git_commit_paths(
|
||||
&dir,
|
||||
&format!("set tool-groups for {agent}"),
|
||||
&["tool-groups.json"],
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -404,8 +423,13 @@ pub async fn commit_capabilities(agent: &str, caps: &[String]) -> Result<()> {
|
|||
if crate::capabilities::capabilities_path().exists() {
|
||||
git(&dir, &["add", "capabilities.json"]).await?;
|
||||
}
|
||||
if has_staged_changes(&dir).await? {
|
||||
git_commit(&dir, &format!("set capabilities for {agent}")).await?;
|
||||
if paths_dirty(&dir, &["capabilities.json"]).await? {
|
||||
git_commit_paths(
|
||||
&dir,
|
||||
&format!("set capabilities for {agent}"),
|
||||
&["capabilities.json"],
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -445,8 +469,14 @@ pub async fn commit_perms(
|
|||
}
|
||||
parts.push("capabilities");
|
||||
}
|
||||
if has_staged_changes(&dir).await? {
|
||||
git_commit(&dir, &format!("set {} for {agent}", parts.join(" + "))).await?;
|
||||
let paths = ["tool-groups.json", "capabilities.json"];
|
||||
if paths_dirty(&dir, &paths).await? {
|
||||
git_commit_paths(
|
||||
&dir,
|
||||
&format!("set {} for {agent}", parts.join(" + ")),
|
||||
&paths,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -467,10 +497,11 @@ pub async fn commit_topology(
|
|||
let dir = meta_dir();
|
||||
let stage = async {
|
||||
git(&dir, &["add", "topology.json"]).await?;
|
||||
if has_staged_changes(&dir).await? {
|
||||
git_commit(
|
||||
if paths_dirty(&dir, &["topology.json"]).await? {
|
||||
git_commit_paths(
|
||||
&dir,
|
||||
&format!("topology: {} → {}", child, new_parent.unwrap_or("<root>")),
|
||||
&["topology.json"],
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
|
@ -541,8 +572,8 @@ pub async fn bulk_commit_topology(
|
|||
};
|
||||
let stage = async {
|
||||
git(&dir, &["add", "topology.json"]).await?;
|
||||
if has_staged_changes(&dir).await? {
|
||||
git_commit(&dir, &commit_msg).await?;
|
||||
if paths_dirty(&dir, &["topology.json"]).await? {
|
||||
git_commit_paths(&dir, &commit_msg, &["topology.json"]).await?;
|
||||
}
|
||||
Ok::<_, anyhow::Error>(())
|
||||
};
|
||||
|
|
@ -1158,16 +1189,6 @@ where
|
|||
out
|
||||
}
|
||||
|
||||
async fn git_is_clean(dir: &Path) -> Result<bool> {
|
||||
let out = lifecycle::git_command()
|
||||
.current_dir(dir)
|
||||
.args(["status", "--porcelain"])
|
||||
.output()
|
||||
.await
|
||||
.with_context(|| format!("git status in {}", dir.display()))?;
|
||||
Ok(out.stdout.iter().all(u8::is_ascii_whitespace))
|
||||
}
|
||||
|
||||
/// Return the list of file names that are currently staged (index differs
|
||||
/// from HEAD). On the initial commit (`HEAD` doesn't exist yet) falls back
|
||||
/// to `git diff --cached --name-only HEAD` failing gracefully by using
|
||||
|
|
@ -1248,6 +1269,43 @@ async fn git_commit(dir: &Path, message: &str) -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Path-limited commit: commits ONLY the given paths, so unrelated
|
||||
/// staged content — above all a `prepare_deploy`-staged `flake.lock`
|
||||
/// — can never be swept into someone else's commit. Every targeted
|
||||
/// meta commit (perm files, topology, lock bumps) goes through this;
|
||||
/// only `sync_agents` uses the bare [`git_commit`], because its
|
||||
/// staged set *is* its intentional commit set.
|
||||
async fn git_commit_paths(dir: &Path, message: &str, paths: &[&str]) -> Result<()> {
|
||||
let name = format!("user.name={GIT_NAME}");
|
||||
let email = format!("user.email={GIT_EMAIL}");
|
||||
let mut args = vec!["-c", &name, "-c", &email, "commit", "-m", message, "--"];
|
||||
args.extend_from_slice(paths);
|
||||
git(dir, &args).await?;
|
||||
if let Err(e) = crate::forge::push_meta(dir).await {
|
||||
tracing::warn!(error = ?e, "forge: meta push after commit failed (non-fatal)");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// True when any of `paths` differs between HEAD and the index or
|
||||
/// working tree — the path-scoped replacement for whole-tree
|
||||
/// `git_is_clean` / `has_staged_changes` guards, which a concurrently
|
||||
/// staged deploy lock would otherwise trip.
|
||||
async fn paths_dirty(dir: &Path, paths: &[&str]) -> Result<bool> {
|
||||
let mut args = vec!["diff", "--quiet", "HEAD", "--"];
|
||||
args.extend_from_slice(paths);
|
||||
let out = lifecycle::git_command()
|
||||
.current_dir(dir)
|
||||
.args(&args)
|
||||
.output()
|
||||
.await
|
||||
.with_context(|| format!("git diff --quiet in {}", dir.display()))?;
|
||||
// Exit 0 = no differences; 1 = differences; anything else (e.g.
|
||||
// no HEAD yet on a fresh repo) → treat as dirty so the commit
|
||||
// path runs and surfaces real errors loudly.
|
||||
Ok(!out.status.success())
|
||||
}
|
||||
|
||||
async fn nix(dir: &Path, args: &[&str]) -> Result<()> {
|
||||
// `--extra-experimental-features` belt-and-suspenders for hosts
|
||||
// that haven't set this in nix.conf. The hyperhive module's
|
||||
|
|
@ -1276,6 +1334,43 @@ async fn nix(dir: &Path, args: &[&str]) -> Result<()> {
|
|||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The regression the deploy-window bug review surfaced: a
|
||||
/// path-limited commit must leave an unrelated staged file (the
|
||||
/// prepare_deploy-staged `flake.lock`) untouched, so a later
|
||||
/// `abort_deploy` still has something to restore.
|
||||
#[tokio::test]
|
||||
async fn path_limited_commit_leaves_unrelated_staged_file_alone() {
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let dir = tmp.path();
|
||||
git(dir, &["init", "--initial-branch=main"])
|
||||
.await
|
||||
.expect("git init");
|
||||
std::fs::write(dir.join("tool-groups.json"), "{}").expect("write");
|
||||
std::fs::write(dir.join("flake.lock"), "v1").expect("write");
|
||||
git(dir, &["add", "-A"]).await.expect("add");
|
||||
git_commit(dir, "seed").await.expect("seed commit");
|
||||
// A deploy stages a new lock (uncommitted)…
|
||||
std::fs::write(dir.join("flake.lock"), "v2-staged-by-deploy").expect("write");
|
||||
git(dir, &["add", "flake.lock"]).await.expect("stage lock");
|
||||
// …and a perm change commits, path-limited.
|
||||
std::fs::write(dir.join("tool-groups.json"), r#"{"alice":[]}"#).expect("write");
|
||||
git(dir, &["add", "tool-groups.json"]).await.expect("add");
|
||||
git_commit_paths(dir, "set tool-groups for alice", &["tool-groups.json"])
|
||||
.await
|
||||
.expect("path-limited commit");
|
||||
// The perm file is committed; the deploy's staged lock is not.
|
||||
assert!(
|
||||
!paths_dirty(dir, &["tool-groups.json"])
|
||||
.await
|
||||
.expect("check"),
|
||||
"perm file must be committed"
|
||||
);
|
||||
assert!(
|
||||
paths_dirty(dir, &["flake.lock"]).await.expect("check"),
|
||||
"staged deploy lock must survive the perm commit"
|
||||
);
|
||||
}
|
||||
|
||||
fn sample_spec(name: &str, is_manager: bool, port: u16) -> AgentSpec {
|
||||
AgentSpec {
|
||||
name: name.to_owned(),
|
||||
|
|
|
|||
Loading…
Reference in a new issue