fix(#2971): thread the queue node id into nix_logged
`nix_logged` wrote its `build_logs` row with `node_id = None`, so a deploy's eval/relock log was reachable from the agent+kind+time listing but not from the queue node that ran it. The comment justifying the `None` said both callers are "reached from outside the queue as well as from inside it". That is no longer true: `verify_commit` and `prepare_deploy` have exactly one caller each, and both sit under the `MergeVerify` / `DeployApply` arms of `exec.rs`'s node dispatch, where the `NodeId` is already in scope. Threads `Option<u64>` from the dispatch down, mirroring `prebuild_toplevel`'s existing `Some(id.get())` at exec.rs:241. Kept as `Option` rather than a bare `u64` because `meta::prepare_deploy` / `meta::verify_commit` are public API and a future non-queue caller has no node to name; the comment now says that instead of the stale claim. No behaviour change beyond the log row gaining its node link.
This commit is contained in:
parent
772529b7b5
commit
6ce71556e2
3 changed files with 51 additions and 22 deletions
|
|
@ -213,7 +213,11 @@ fn deploy_ctx(coord: &Coordinator, approval_id: i64) -> Result<DeployCtx> {
|
||||||
/// fails, if the reviewed head does not descend from `applied/main`, or if the
|
/// fails, if the reviewed head does not descend from `applied/main`, or if the
|
||||||
/// eval-verify of the reviewed commit fails. Every one of these leaves the forge
|
/// eval-verify of the reviewed commit fails. Every one of these leaves the forge
|
||||||
/// and `main` untouched, so the node is safely retryable.
|
/// and `main` untouched, so the node is safely retryable.
|
||||||
pub async fn run_deploy_merge_verify(coord: &Arc<Coordinator>, approval_id: i64) -> Result<()> {
|
pub async fn run_deploy_merge_verify(
|
||||||
|
coord: &Arc<Coordinator>,
|
||||||
|
approval_id: i64,
|
||||||
|
node_id: Option<u64>,
|
||||||
|
) -> Result<()> {
|
||||||
let ctx = deploy_ctx(coord, approval_id)?;
|
let ctx = deploy_ctx(coord, approval_id)?;
|
||||||
let pr = ctx.pr;
|
let pr = ctx.pr;
|
||||||
let reviewed = ctx.reviewed.as_str();
|
let reviewed = ctx.reviewed.as_str();
|
||||||
|
|
@ -250,9 +254,14 @@ pub async fn run_deploy_merge_verify(coord: &Arc<Coordinator>, approval_id: i64)
|
||||||
}
|
}
|
||||||
|
|
||||||
// 4. Eval-verify BEFORE the irreversible merge (bad nix fails fast here).
|
// 4. Eval-verify BEFORE the irreversible merge (bad nix fails fast here).
|
||||||
crate::meta::verify_commit(ctx.approval.agent.as_str(), &ctx.applied_dir, reviewed)
|
crate::meta::verify_commit(
|
||||||
.await
|
ctx.approval.agent.as_str(),
|
||||||
.map_err(|e| anyhow::anyhow!("verify merge head {reviewed}: {e:#}"))?;
|
&ctx.applied_dir,
|
||||||
|
reviewed,
|
||||||
|
node_id,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map_err(|e| anyhow::anyhow!("verify merge head {reviewed}: {e:#}"))?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -275,7 +284,11 @@ pub async fn run_deploy_merge_verify(coord: &Arc<Coordinator>, approval_id: i64)
|
||||||
/// fast-forwarding `applied/main` / `meta::prepare_deploy` fails. From the merge
|
/// fast-forwarding `applied/main` / `meta::prepare_deploy` fails. From the merge
|
||||||
/// onward a failure is *not* retryable on its own — [`run_deploy_tail`] runs
|
/// onward a failure is *not* retryable on its own — [`run_deploy_tail`] runs
|
||||||
/// `AfterAny` to compensate.
|
/// `AfterAny` to compensate.
|
||||||
pub async fn run_deploy_apply(coord: &Arc<Coordinator>, approval_id: i64) -> Result<()> {
|
pub async fn run_deploy_apply(
|
||||||
|
coord: &Arc<Coordinator>,
|
||||||
|
approval_id: i64,
|
||||||
|
node_id: Option<u64>,
|
||||||
|
) -> Result<()> {
|
||||||
let ctx = deploy_ctx(coord, approval_id)?;
|
let ctx = deploy_ctx(coord, approval_id)?;
|
||||||
let agent = ctx.approval.agent.as_str();
|
let agent = ctx.approval.agent.as_str();
|
||||||
let pr = ctx.pr;
|
let pr = ctx.pr;
|
||||||
|
|
@ -300,7 +313,7 @@ pub async fn run_deploy_apply(coord: &Arc<Coordinator>, approval_id: i64) -> Res
|
||||||
Err(e) => bail!("ff-merge PR #{pr}: {e}"),
|
Err(e) => bail!("ff-merge PR #{pr}: {e}"),
|
||||||
}
|
}
|
||||||
|
|
||||||
prepare_applied_target(agent, &ctx.applied_dir, &ctx.reviewed, &prev_main).await
|
prepare_applied_target(agent, &ctx.applied_dir, &ctx.reviewed, &prev_main, node_id).await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// `DeployTail` node body — compensation + bookkeeping, `AfterAny` the apply
|
/// `DeployTail` node body — compensation + bookkeeping, `AfterAny` the apply
|
||||||
|
|
@ -817,6 +830,7 @@ async fn prepare_applied_target(
|
||||||
applied_dir: &std::path::Path,
|
applied_dir: &std::path::Path,
|
||||||
target: &str,
|
target: &str,
|
||||||
expected_main: &str,
|
expected_main: &str,
|
||||||
|
node_id: Option<u64>,
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
// Fast-forward applied/main to target + sync the working tree. Meta input
|
// Fast-forward applied/main to target + sync the working tree. Meta input
|
||||||
// pins `?ref=main`, so this is what makes nix re-lock to the target commit
|
// pins `?ref=main`, so this is what makes nix re-lock to the target commit
|
||||||
|
|
@ -840,7 +854,7 @@ async fn prepare_applied_target(
|
||||||
// Phase 1 of the meta two-phase deploy: relock without committing. The
|
// Phase 1 of the meta two-phase deploy: relock without committing. The
|
||||||
// staged lock then stays uncommitted across the whole appended rebuild —
|
// staged lock then stays uncommitted across the whole appended rebuild —
|
||||||
// which is why the `MetaWindow` is held by the deploy root, not by a node.
|
// which is why the `MetaWindow` is held by the deploy root, not by a node.
|
||||||
crate::meta::prepare_deploy(agent)
|
crate::meta::prepare_deploy(agent, node_id)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| anyhow::anyhow!("meta prepare_deploy: {e:#}"))
|
.map_err(|e| anyhow::anyhow!("meta prepare_deploy: {e:#}"))
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -103,9 +103,11 @@ pub(super) async fn run_node(
|
||||||
// that could never fire.
|
// that could never fire.
|
||||||
NodeKind::WritePermFile { payload, .. } => run_write_perm_file(coord, agent, payload).await,
|
NodeKind::WritePermFile { payload, .. } => run_write_perm_file(coord, agent, payload).await,
|
||||||
NodeKind::Reparent { moves } => run_reparent(coord, moves).await,
|
NodeKind::Reparent { moves } => run_reparent(coord, moves).await,
|
||||||
NodeKind::MergeVerify { approval_id, .. } => run_merge_verify(coord, *approval_id).await,
|
NodeKind::MergeVerify { approval_id, .. } => {
|
||||||
|
run_merge_verify(coord, *approval_id, id).await
|
||||||
|
}
|
||||||
NodeKind::DeployApply { approval_id, .. } => {
|
NodeKind::DeployApply { approval_id, .. } => {
|
||||||
run_deploy_apply(coord, *approval_id).await.map(|()| {
|
run_deploy_apply(coord, *approval_id, id).await.map(|()| {
|
||||||
super::templates::deploy_rebuild_nodes(&builder, agent, *approval_id);
|
super::templates::deploy_rebuild_nodes(&builder, agent, *approval_id);
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
@ -574,8 +576,8 @@ async fn run_reparent(
|
||||||
/// Deploy phase 1 — drift gate, fetch, eval-verify. Mutates nothing, so a
|
/// Deploy phase 1 — drift gate, fetch, eval-verify. Mutates nothing, so a
|
||||||
/// failure here cancel-cascades the rest of the subtree with the forge and the
|
/// failure here cancel-cascades the rest of the subtree with the forge and the
|
||||||
/// applied repo exactly as they were.
|
/// applied repo exactly as they were.
|
||||||
async fn run_merge_verify(coord: &Arc<Coordinator>, approval_id: i64) -> Result<()> {
|
async fn run_merge_verify(coord: &Arc<Coordinator>, approval_id: i64, id: NodeId) -> Result<()> {
|
||||||
crate::actions::run_deploy_merge_verify(coord, approval_id).await
|
crate::actions::run_deploy_merge_verify(coord, approval_id, Some(id.get())).await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Deploy phase 2 — the irreversible half: ff-merge, then phase 1 of the
|
/// Deploy phase 2 — the irreversible half: ff-merge, then phase 1 of the
|
||||||
|
|
@ -587,8 +589,8 @@ async fn run_merge_verify(coord: &Arc<Coordinator>, approval_id: i64) -> Result<
|
||||||
/// their `MetaSync` declares is re-entered rather than deadlocked against the
|
/// their `MetaSync` declares is re-entered rather than deadlocked against the
|
||||||
/// ancestor already holding it. On failure nothing is appended and the tail
|
/// ancestor already holding it. On failure nothing is appended and the tail
|
||||||
/// compensates, exactly as before.
|
/// compensates, exactly as before.
|
||||||
async fn run_deploy_apply(coord: &Arc<Coordinator>, approval_id: i64) -> Result<()> {
|
async fn run_deploy_apply(coord: &Arc<Coordinator>, approval_id: i64, id: NodeId) -> Result<()> {
|
||||||
crate::actions::run_deploy_apply(coord, approval_id).await
|
crate::actions::run_deploy_apply(coord, approval_id, Some(id.get())).await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Deploy phase 3 — close the staged-lock window once the appended rebuild has
|
/// Deploy phase 3 — close the staged-lock window once the appended rebuild has
|
||||||
|
|
|
||||||
|
|
@ -243,7 +243,7 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> {
|
||||||
/// the index. Doesn't commit — `finalize_deploy` commits on build
|
/// the index. Doesn't commit — `finalize_deploy` commits on build
|
||||||
/// success, `abort_deploy` drops the staged change on failure so
|
/// success, `abort_deploy` drops the staged change on failure so
|
||||||
/// meta history only carries successful deploys.
|
/// meta history only carries successful deploys.
|
||||||
pub async fn prepare_deploy(name: &str) -> Result<()> {
|
pub async fn prepare_deploy(name: &str, node_id: Option<u64>) -> Result<()> {
|
||||||
let _guard = META_LOCK.lock().await;
|
let _guard = META_LOCK.lock().await;
|
||||||
let dir = crate::paths::meta_root();
|
let dir = crate::paths::meta_root();
|
||||||
let input = format!("agent-{name}");
|
let input = format!("agent-{name}");
|
||||||
|
|
@ -268,6 +268,7 @@ pub async fn prepare_deploy(name: &str) -> Result<()> {
|
||||||
],
|
],
|
||||||
name,
|
name,
|
||||||
"prepare-deploy",
|
"prepare-deploy",
|
||||||
|
node_id,
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
// Stage the new lock — git+file://'s dirty-tree fetcher reads
|
// Stage the new lock — git+file://'s dirty-tree fetcher reads
|
||||||
|
|
@ -375,7 +376,12 @@ fn applied_override_url(applied_dir: &Path) -> String {
|
||||||
/// resolution at the same cost profile as the legacy pre-merge check;
|
/// resolution at the same cost profile as the legacy pre-merge check;
|
||||||
/// the real container build still runs (and can still roll back) on the
|
/// the real container build still runs (and can still roll back) on the
|
||||||
/// actual apply, so this is the right "would this apply" gate.
|
/// actual apply, so this is the right "would this apply" gate.
|
||||||
pub async fn verify_commit(name: &str, applied_dir: &Path, sha: &str) -> Result<()> {
|
pub async fn verify_commit(
|
||||||
|
name: &str,
|
||||||
|
applied_dir: &Path,
|
||||||
|
sha: &str,
|
||||||
|
node_id: Option<u64>,
|
||||||
|
) -> Result<()> {
|
||||||
let _guard = META_LOCK.lock().await;
|
let _guard = META_LOCK.lock().await;
|
||||||
let dir = crate::paths::meta_root();
|
let dir = crate::paths::meta_root();
|
||||||
let input = format!("agent-{name}");
|
let input = format!("agent-{name}");
|
||||||
|
|
@ -393,6 +399,7 @@ pub async fn verify_commit(name: &str, applied_dir: &Path, sha: &str) -> Result<
|
||||||
],
|
],
|
||||||
name,
|
name,
|
||||||
"verify",
|
"verify",
|
||||||
|
node_id,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
@ -1586,16 +1593,22 @@ async fn nix(dir: &Path, args: &[&str]) -> Result<()> {
|
||||||
/// zero build logs to look at. Best-effort logging: a missing global
|
/// zero build logs to look at. Best-effort logging: a missing global
|
||||||
/// handle or a failed `start()` just skips the row — the command still
|
/// handle or a failed `start()` just skips the row — the command still
|
||||||
/// runs and its exit status is still enforced.
|
/// runs and its exit status is still enforced.
|
||||||
async fn nix_logged(dir: &Path, args: &[&str], agent: &str, kind: &str) -> Result<()> {
|
async fn nix_logged(
|
||||||
|
dir: &Path,
|
||||||
|
args: &[&str],
|
||||||
|
agent: &str,
|
||||||
|
kind: &str,
|
||||||
|
node_id: Option<u64>,
|
||||||
|
) -> Result<()> {
|
||||||
let cmdline = format!("nix {}", nix_argv(args).join(" "));
|
let cmdline = format!("nix {}", nix_argv(args).join(" "));
|
||||||
let logs = crate::build_logs::global();
|
let logs = crate::build_logs::global();
|
||||||
let log_id = logs.as_ref().and_then(|h| {
|
let log_id = logs.as_ref().and_then(|h| {
|
||||||
// No node id: `nix_logged`'s two callers are meta-flake operations
|
// `node_id` links the row to the queue node that ran it, so the
|
||||||
// reached from outside the queue as well as from inside it, and the
|
// dashboard can reach this log from the node instead of only from the
|
||||||
// agent+kind+time listing is how they're surfaced today. Linking them
|
// agent+kind+time listing. Both call paths are queue-only today and
|
||||||
// to a node would mean threading the id through `meta`'s public API
|
// pass `Some`; it stays an `Option` because these are `meta`'s public
|
||||||
// for no current reader — worth doing when something wants it.
|
// API and a future non-queue caller has no node to name.
|
||||||
h.start(agent, kind, &cmdline, None)
|
h.start(agent, kind, &cmdline, node_id)
|
||||||
.map_err(|e| {
|
.map_err(|e| {
|
||||||
tracing::warn!(error = ?e, %kind, "build_logs: start failed (meta log dropped)");
|
tracing::warn!(error = ?e, %kind, "build_logs: start failed (meta log dropped)");
|
||||||
})
|
})
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue