comment once on a merged config pr when its swarm build fails
This commit is contained in:
parent
c28ca7aab5
commit
0bc5a7e475
7 changed files with 460 additions and 22 deletions
|
|
@ -225,6 +225,16 @@ is the same case: the old builder has no subscriber for the new subject, so the
|
|||
controller gets no responders at once, and the deploys go out after the builder
|
||||
updates. Each round adds a build and deploy node per failing agent.
|
||||
|
||||
When the build belongs to a merged config PR and `nix` exited with an error or hit
|
||||
its deadline, the controller also comments once on that PR, with its forge
|
||||
account: the agent, the `internal/meta` commit, `nix`'s last lines (at most 4000
|
||||
bytes), and that the deploy waits and the swarm builds it again every round. A comment
|
||||
carries a marker line with the agent and the commit, and the controller posts
|
||||
only if the PR holds no comment with that marker, so the retry rounds stay quiet
|
||||
until `internal/meta` moves. A `no verdict` failure, and a build that isn't a
|
||||
merged PR's (a new agent, the pin fan-out), leave no comment. When the forge
|
||||
refuses the comment, the controller logs a warning and the node ends as before.
|
||||
|
||||
The controller exports the gauge `swarm.deploys.held`: the agents whose latest
|
||||
build failed, with the label `reason` set to `failed` or `unavailable`. The Grafana
|
||||
alert `hh-deploys-held` fires when the sum stays above zero for 30 minutes, so
|
||||
|
|
|
|||
|
|
@ -73,11 +73,13 @@ struct WebhookBranch {
|
|||
sha: Option<String>,
|
||||
}
|
||||
|
||||
/// A config PR that was just merged: whose config, and the commit to deploy.
|
||||
/// A config PR that was just merged: whose config, the commit to deploy, and
|
||||
/// the PR.
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub struct MergedConfigPr {
|
||||
pub agent: String,
|
||||
pub rev: String,
|
||||
pub pr: u64,
|
||||
}
|
||||
|
||||
/// The merge a verified `ConfigPr` delivery reports, if it reports one.
|
||||
|
|
@ -108,6 +110,7 @@ pub fn merged(body: &[u8]) -> Option<MergedConfigPr> {
|
|||
Some(MergedConfigPr {
|
||||
agent: payload.repository.name,
|
||||
rev,
|
||||
pr: payload.pull_request.number,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -373,6 +376,7 @@ mod tests {
|
|||
Some(super::MergedConfigPr {
|
||||
agent: "damocles".to_owned(),
|
||||
rev: "abc123".to_owned(),
|
||||
pr: 5,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -55,6 +55,7 @@ pub mod legacy_tokens;
|
|||
mod meta_repo;
|
||||
pub mod objects;
|
||||
pub mod operator_profiles;
|
||||
pub(crate) mod pr_comment;
|
||||
pub mod pull_diff;
|
||||
pub mod review_request;
|
||||
pub mod runner_token;
|
||||
|
|
|
|||
150
swarm-controller/src/forge/pr_comment.rs
Normal file
150
swarm-controller/src/forge/pr_comment.rs
Normal file
|
|
@ -0,0 +1,150 @@
|
|||
//! Comments on a pull request, posted once per marker.
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
use forgejo_api::structs::{CreateIssueCommentOption, IssueGetCommentsQuery};
|
||||
|
||||
use super::Client;
|
||||
|
||||
impl Client {
|
||||
/// Comment `body` on pull request `pr` of `owner/repo` unless a comment
|
||||
/// there already contains `marker`. Whether a comment was posted.
|
||||
///
|
||||
/// The marker is what the comment carries, so the check holds across a
|
||||
/// restart and across every caller. It reads the comments first, so two
|
||||
/// concurrent calls with one marker can both post.
|
||||
pub(crate) async fn comment_once(
|
||||
&self,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
pr: u64,
|
||||
marker: &str,
|
||||
body: &str,
|
||||
) -> Result<bool> {
|
||||
let index = i64::try_from(pr).context("PR number out of range")?;
|
||||
let existing = self
|
||||
.api
|
||||
.call(|api| {
|
||||
Box::pin(
|
||||
api.issue_get_comments(
|
||||
owner,
|
||||
repo,
|
||||
index,
|
||||
IssueGetCommentsQuery {
|
||||
since: None,
|
||||
before: None,
|
||||
},
|
||||
)
|
||||
.all(),
|
||||
)
|
||||
})
|
||||
.await
|
||||
.with_context(|| format!("listing the comments on {owner}/{repo}#{pr}"))?;
|
||||
if existing
|
||||
.iter()
|
||||
.any(|c| c.body.as_deref().is_some_and(|b| b.contains(marker)))
|
||||
{
|
||||
return Ok(false);
|
||||
}
|
||||
let option = CreateIssueCommentOption {
|
||||
body: body.to_owned(),
|
||||
updated_at: None,
|
||||
};
|
||||
self.api
|
||||
.call(|api| {
|
||||
api.issue_create_comment(owner, repo, index, option.clone())
|
||||
.into_future()
|
||||
})
|
||||
.await
|
||||
.with_context(|| format!("commenting on {owner}/{repo}#{pr}"))?;
|
||||
Ok(true)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) mod tests {
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use axum::http::{Method, StatusCode, Uri};
|
||||
|
||||
use super::*;
|
||||
|
||||
/// The comments a stand-in forge holds, as the issue path they were
|
||||
/// posted to and their body.
|
||||
pub(crate) type Posted = Arc<Mutex<Vec<(String, String)>>>;
|
||||
|
||||
/// A stand-in forge that stores the comments posted to any issue path and
|
||||
/// lists them back.
|
||||
pub(crate) async fn stand_in() -> (Client, Posted) {
|
||||
let held: Posted = Arc::default();
|
||||
let state = Arc::clone(&held);
|
||||
let app = axum::Router::new().fallback(
|
||||
move |method: Method, uri: Uri, body: axum::body::Bytes| {
|
||||
let state = Arc::clone(&state);
|
||||
async move {
|
||||
let path = uri.path().to_owned();
|
||||
let mut held = state.lock().unwrap();
|
||||
match method.as_str() {
|
||||
"GET" => {
|
||||
let list: Vec<_> = held
|
||||
.iter()
|
||||
.filter(|(p, _)| *p == path)
|
||||
.enumerate()
|
||||
.map(
|
||||
|(id, (_, body))| serde_json::json!({ "id": id, "body": body }),
|
||||
)
|
||||
.collect();
|
||||
let count = list.len().to_string();
|
||||
(
|
||||
StatusCode::OK,
|
||||
[("x-total-count", count)],
|
||||
serde_json::Value::from(list).to_string(),
|
||||
)
|
||||
}
|
||||
"POST" => {
|
||||
let sent: serde_json::Value = serde_json::from_slice(&body).unwrap();
|
||||
let text = sent["body"].as_str().unwrap().to_owned();
|
||||
held.push((path, text.clone()));
|
||||
(
|
||||
StatusCode::CREATED,
|
||||
[("x-total-count", String::new())],
|
||||
serde_json::json!({ "id": held.len(), "body": text }).to_string(),
|
||||
)
|
||||
}
|
||||
_ => (
|
||||
StatusCode::NOT_FOUND,
|
||||
[("x-total-count", String::new())],
|
||||
r#"{"message":"no"}"#.to_owned(),
|
||||
),
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||
let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap();
|
||||
tokio::spawn(async move { axum::serve(listener, app).await });
|
||||
(Client::for_test_forge(url), held)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_marker_is_commented_once_and_a_new_marker_again() {
|
||||
let (client, held) = stand_in().await;
|
||||
let once = |marker: &'static str| {
|
||||
let client = &client;
|
||||
async move {
|
||||
client
|
||||
.comment_once("o", "r", 7, marker, &format!("{marker} text"))
|
||||
.await
|
||||
.unwrap()
|
||||
}
|
||||
};
|
||||
assert!(once("<!-- a -->").await, "first comment is posted");
|
||||
assert!(!once("<!-- a -->").await, "same marker is not posted again");
|
||||
assert!(once("<!-- b -->").await, "control: another marker posts");
|
||||
let held = held.lock().unwrap();
|
||||
assert_eq!(held.len(), 2, "{held:?}");
|
||||
assert_eq!(
|
||||
held[0].0,
|
||||
format!("/api/v1/repos/o/r/{}/7/comments", "issues")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -96,6 +96,7 @@ mod tests {
|
|||
.append(
|
||||
SwarmNodeKind::BuildMeta {
|
||||
agent: (*agent).to_owned(),
|
||||
pr: None,
|
||||
},
|
||||
Vec::new(),
|
||||
None,
|
||||
|
|
|
|||
|
|
@ -116,7 +116,10 @@ pub(crate) enum SwarmNodeKind {
|
|||
///
|
||||
/// Carries no rev: it reads the head when it runs, which in most DAGs is
|
||||
/// after the `UpdateMeta` that commits it.
|
||||
BuildMeta { agent: String },
|
||||
///
|
||||
/// `pr` is the merged config PR the build belongs to, if any: a red `nix`
|
||||
/// build is reported on it once.
|
||||
BuildMeta { agent: String, pr: Option<u64> },
|
||||
/// Re-mint `agent`'s queue secret if it is still stored and old enough.
|
||||
/// See `agent_renewal`.
|
||||
///
|
||||
|
|
@ -202,10 +205,12 @@ impl hive_jobq_wire::WireNode for SwarmNodeKind {
|
|||
| SwarmNodeKind::MintAgentIdentity { agent }
|
||||
| SwarmNodeKind::MintAgentForgeToken { agent }
|
||||
| SwarmNodeKind::MintAgentMatrixAccount { agent }
|
||||
| SwarmNodeKind::BuildMeta { agent }
|
||||
| SwarmNodeKind::RenewAgentQueueCredential { agent } => {
|
||||
serde_json::json!({ "agent": agent })
|
||||
}
|
||||
SwarmNodeKind::BuildMeta { agent, pr } => {
|
||||
serde_json::json!({ "agent": agent, "pr": pr })
|
||||
}
|
||||
SwarmNodeKind::MintHiveSenderToken { hive }
|
||||
| SwarmNodeKind::MintHiveForgeToken { hive } => serde_json::json!({ "hive": hive }),
|
||||
SwarmNodeKind::MintClassifierForgeToken
|
||||
|
|
@ -414,7 +419,7 @@ async fn run_node_work(
|
|||
SwarmNodeKind::BuildAgent { agent, rev, pr } => {
|
||||
build_agent(&deps, forge, agent, rev, pr).await
|
||||
}
|
||||
SwarmNodeKind::BuildMeta { agent } => build_meta(&deps, forge, agent).await,
|
||||
SwarmNodeKind::BuildMeta { agent, pr } => build_meta(&deps, forge, agent, pr).await,
|
||||
SwarmNodeKind::RenewAgentQueueCredential { agent } => renew_queue_credential(&agent).await,
|
||||
SwarmNodeKind::SetAgentWanted { hive, agent } => {
|
||||
set_agent_wanted(deps.wanted.as_deref(), &hive, &agent).await
|
||||
|
|
@ -602,8 +607,9 @@ async fn build_meta(
|
|||
deps: &WorkerDeps,
|
||||
forge: Option<Arc<forge::Client>>,
|
||||
agent: String,
|
||||
pr: Option<u64>,
|
||||
) -> hive_jobq::scheduler::Outcome {
|
||||
meta_build::run(forge.as_deref(), deps.queue.as_ref(), agent).await
|
||||
meta_build::run(forge.as_deref(), deps.queue.as_ref(), agent, pr).await
|
||||
}
|
||||
|
||||
/// What an `UpdateMeta` pass decided: the deploys to send, and the placements
|
||||
|
|
@ -746,11 +752,13 @@ fn gated_deploy<'a>(
|
|||
hive: &str,
|
||||
agent: &str,
|
||||
rev: Option<String>,
|
||||
pr: Option<u64>,
|
||||
update: Option<hive_jobq::builder::NodeGuid>,
|
||||
) -> hive_jobq::builder::NodeRef<'a, SwarmNodeKind, SwarmResourceKind> {
|
||||
let mut build = b
|
||||
.node(SwarmNodeKind::BuildMeta {
|
||||
agent: agent.to_owned(),
|
||||
pr,
|
||||
})
|
||||
.needs(SwarmResourceKind::Builder);
|
||||
if let Some(update) = update {
|
||||
|
|
@ -785,7 +793,7 @@ fn declare_fan_out(
|
|||
}
|
||||
let deploys: Vec<hive_jobq::builder::NodeGuid> = placements
|
||||
.iter()
|
||||
.map(|(hive, agent)| gated_deploy(grown, hive, agent, None, None).guid())
|
||||
.map(|(hive, agent)| gated_deploy(grown, hive, agent, None, None, None).guid())
|
||||
.collect();
|
||||
if let FanOut::Record { pin } = fan_out {
|
||||
let mut record = grown.node(SwarmNodeKind::RecordFanout { pin: pin.clone() });
|
||||
|
|
@ -1173,7 +1181,7 @@ pub(crate) async fn queue_merged_config_deploy(
|
|||
state: &AppState,
|
||||
merged: config_pr::MergedConfigPr,
|
||||
) {
|
||||
let config_pr::MergedConfigPr { agent, rev } = merged;
|
||||
let config_pr::MergedConfigPr { agent, rev, pr } = merged;
|
||||
let declared = match read_declarations(state, state.hives.iter()).await {
|
||||
Ok(declared) => declared,
|
||||
Err((hive, e)) => {
|
||||
|
|
@ -1197,7 +1205,7 @@ pub(crate) async fn queue_merged_config_deploy(
|
|||
.jobq
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.insert_job(None, |b| merged_deploy_job(b, &hive, &agent, &rev));
|
||||
.insert_job(None, |b| merged_deploy_job(b, &hive, &agent, &rev, pr));
|
||||
match inserted {
|
||||
Ok(_) => tracing::info!(%agent, %rev, %hive, "config-pr merge: deploy queued"),
|
||||
Err(e) => {
|
||||
|
|
@ -1218,12 +1226,13 @@ fn merged_deploy_job(
|
|||
hive: &str,
|
||||
agent: &str,
|
||||
rev: &str,
|
||||
pr: u64,
|
||||
) -> Vec<hive_jobq::builder::NodeGuid> {
|
||||
let update_meta = b
|
||||
.node(SwarmNodeKind::UpdateMeta)
|
||||
.needs(SwarmResourceKind::Meta);
|
||||
let root = update_meta.guid();
|
||||
let _deploy = gated_deploy(b, hive, agent, Some(rev.to_owned()), Some(root));
|
||||
let _deploy = gated_deploy(b, hive, agent, Some(rev.to_owned()), Some(pr), Some(root));
|
||||
vec![root]
|
||||
}
|
||||
|
||||
|
|
@ -1347,7 +1356,7 @@ pub(crate) fn declare_agent_job(
|
|||
// queue at all fails this node — but `TriggerDeploy` has nothing to
|
||||
// publish to on such a host either, so the strict edge cancels a node
|
||||
// that could not have succeeded anyway.
|
||||
let trigger_deploy = gated_deploy(b, hive, agent, None, Some(update_meta.guid()))
|
||||
let trigger_deploy = gated_deploy(b, hive, agent, None, None, Some(update_meta.guid()))
|
||||
.after_any(mint_identity)
|
||||
.after_any(mint_forge_token)
|
||||
// `after_any` for the same reason: the container reads this token
|
||||
|
|
@ -1561,6 +1570,9 @@ struct SkippedDeploy {
|
|||
hive: String,
|
||||
agent: String,
|
||||
rev: Option<String>,
|
||||
/// The merged config PR the failed build belonged to, so the build
|
||||
/// declared again reports on it too.
|
||||
pr: Option<u64>,
|
||||
cause: SkipCause,
|
||||
}
|
||||
|
||||
|
|
@ -1592,14 +1604,15 @@ fn skipped_deploys(
|
|||
let SwarmNodeKind::TriggerDeploy { hive, agent, rev } = &n.payload else {
|
||||
return None;
|
||||
};
|
||||
let failed = |is: fn(&SwarmNodeKind) -> bool| {
|
||||
n.deps.iter().any(|d| match d {
|
||||
hive_jobq::Dep::Node { id, .. } => graph.node(*id).is_some_and(|dep| {
|
||||
is(&dep.payload) && dep.state == hive_jobq::State::Failed
|
||||
}),
|
||||
hive_jobq::Dep::Resource { .. } => false,
|
||||
let dep_nodes = || {
|
||||
n.deps.iter().filter_map(|d| match d {
|
||||
hive_jobq::Dep::Node { id, .. } => graph.node(*id),
|
||||
hive_jobq::Dep::Resource { .. } => None,
|
||||
})
|
||||
};
|
||||
let failed = |is: fn(&SwarmNodeKind) -> bool| {
|
||||
dep_nodes().any(|dep| is(&dep.payload) && dep.state == hive_jobq::State::Failed)
|
||||
};
|
||||
let cause = if failed(|p| matches!(p, SwarmNodeKind::UpdateMeta)) {
|
||||
SkipCause::FailedUpdate
|
||||
} else if failed(|p| matches!(p, SwarmNodeKind::BuildMeta { .. })) {
|
||||
|
|
@ -1607,10 +1620,15 @@ fn skipped_deploys(
|
|||
} else {
|
||||
return None;
|
||||
};
|
||||
let pr = dep_nodes().find_map(|dep| match &dep.payload {
|
||||
SwarmNodeKind::BuildMeta { pr, .. } => *pr,
|
||||
_ => None,
|
||||
});
|
||||
Some(SkippedDeploy {
|
||||
hive: hive.clone(),
|
||||
agent: agent.clone(),
|
||||
rev: rev.clone(),
|
||||
pr,
|
||||
cause,
|
||||
})
|
||||
})
|
||||
|
|
@ -1666,7 +1684,7 @@ fn queue_meta_update(
|
|||
});
|
||||
let deploys: Vec<_> = skipped
|
||||
.into_iter()
|
||||
.map(|s| gated_deploy(b, &s.hive, &s.agent, s.rev, update).guid())
|
||||
.map(|s| gated_deploy(b, &s.hive, &s.agent, s.rev, s.pr, update).guid())
|
||||
.collect();
|
||||
update.map_or(deploys, |update| vec![update])
|
||||
})
|
||||
|
|
@ -2294,7 +2312,7 @@ mod tests {
|
|||
}
|
||||
|
||||
fn merged_dag(b: &TestBuilder) {
|
||||
let _ = super::merged_deploy_job(b, "pr1ma", "atlas", "abc");
|
||||
let _ = super::merged_deploy_job(b, "pr1ma", "atlas", "abc", 7);
|
||||
}
|
||||
|
||||
/// A deploy needs the meta update to succeed: its edge refuses a failed
|
||||
|
|
@ -2890,7 +2908,7 @@ mod tests {
|
|||
.filter(|n| n.payload.label() == label)
|
||||
.filter(|n| n.state == State::Pending)
|
||||
.map(|n| match &n.payload {
|
||||
SwarmNodeKind::BuildMeta { agent }
|
||||
SwarmNodeKind::BuildMeta { agent, .. }
|
||||
| SwarmNodeKind::TriggerDeploy { agent, .. } => agent.clone(),
|
||||
other => panic!("not a build or a deploy: {other:?}"),
|
||||
})
|
||||
|
|
@ -3811,12 +3829,27 @@ mod tests {
|
|||
nodes.into_iter().map(|(_, rev)| rev).collect()
|
||||
}
|
||||
|
||||
/// The `pr` of `agent`'s builds, oldest first.
|
||||
fn build_prs(sched: &SharedSched, agent: &str) -> Vec<Option<u64>> {
|
||||
let guard = sched.lock().unwrap();
|
||||
let mut nodes: Vec<_> = guard
|
||||
.graph()
|
||||
.nodes()
|
||||
.filter_map(|n| match &n.payload {
|
||||
SwarmNodeKind::BuildMeta { agent: a, pr } if a == agent => Some((n.id, *pr)),
|
||||
_ => None,
|
||||
})
|
||||
.collect();
|
||||
nodes.sort_by_key(|(id, _)| *id);
|
||||
nodes.into_iter().map(|(_, pr)| pr).collect()
|
||||
}
|
||||
|
||||
fn merged_atlas() -> SharedSched {
|
||||
let sched = fresh_sched();
|
||||
sched
|
||||
.lock()
|
||||
.unwrap()
|
||||
.insert_job(None, |b| merged_deploy_job(b, "pr1ma", "atlas", "abc"))
|
||||
.insert_job(None, |b| merged_deploy_job(b, "pr1ma", "atlas", "abc", 7))
|
||||
.expect("the graph inserts");
|
||||
sched
|
||||
}
|
||||
|
|
@ -3865,11 +3898,27 @@ mod tests {
|
|||
.iter()
|
||||
.all(|rev| rev.as_deref() == Some("abc"))
|
||||
);
|
||||
// So does the PR it belongs to: every retried build reports on it too.
|
||||
assert_eq!(build_prs(&sched, "atlas"), [Some(7); 4]);
|
||||
assert!(
|
||||
!queue_meta_update(&sched).expect("tick"),
|
||||
"the success ends the retries"
|
||||
);
|
||||
}
|
||||
|
||||
/// Only a merged config PR's build carries a PR: the creation DAG's does
|
||||
/// not, so its failure has nobody to comment on.
|
||||
#[test]
|
||||
fn only_a_merged_prs_build_carries_its_pr() {
|
||||
let sched = merged_atlas();
|
||||
sched
|
||||
.lock()
|
||||
.unwrap()
|
||||
.insert_job(None, |b| declare_agent_job(b, "iris", "pr1ma", false))
|
||||
.expect("the graph inserts");
|
||||
assert_eq!(build_prs(&sched, "atlas"), [Some(7)]);
|
||||
assert_eq!(build_prs(&sched, "iris"), [None]);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
|
|
@ -19,6 +19,11 @@
|
|||
//!
|
||||
//! Both are built again on every round of the meta timer
|
||||
//! (`jobq::queue_meta_update`) until a build succeeds.
|
||||
//!
|
||||
//! A `nix` failure of a merged config PR's build (`pr` set) also leaves one
|
||||
//! comment on that PR, deduped by a marker in the comment per agent and
|
||||
//! `internal/meta` rev, so the retry rounds stay quiet. A no-verdict failure
|
||||
//! says nothing about the config and leaves none.
|
||||
|
||||
use std::fmt::Display;
|
||||
use std::future::Future;
|
||||
|
|
@ -51,6 +56,15 @@ pub(crate) trait MetaRepo {
|
|||
fn head(&self) -> impl Future<Output = Result<Option<String>>> + Send;
|
||||
/// Make sure the builder's forge account can read the repo.
|
||||
fn grant_builder_read(&self) -> impl Future<Output = Result<()>> + Send;
|
||||
/// Comment `body` on `pr` of `agent`'s config repo unless a comment there
|
||||
/// already contains `marker`. Whether a comment was posted.
|
||||
fn comment_once(
|
||||
&self,
|
||||
agent: &str,
|
||||
pr: u64,
|
||||
marker: &str,
|
||||
body: &str,
|
||||
) -> impl Future<Output = Result<bool>> + Send;
|
||||
}
|
||||
|
||||
impl MetaRepo for forge::Client {
|
||||
|
|
@ -67,6 +81,14 @@ impl MetaRepo for forge::Client {
|
|||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn comment_once(&self, agent: &str, pr: u64, marker: &str, body: &str) -> Result<bool> {
|
||||
crate::jobq::forge_node(
|
||||
"build_meta",
|
||||
forge::Client::comment_once(self, forge::CONFIG_ORG, agent, pr, marker, body),
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
/// Where a build is sent and its closure recorded: the queue in production, a
|
||||
|
|
@ -119,11 +141,55 @@ impl MetaBuilds for async_nats::Client {
|
|||
}
|
||||
}
|
||||
|
||||
/// The most of `nix`'s tail a PR comment carries, in bytes: the builder's own cap
|
||||
/// on the tail it sends.
|
||||
const COMMENT_TAIL_BYTES: usize = 4000;
|
||||
|
||||
/// The comment a red build leaves on its merged config PR, first line the
|
||||
/// marker the dedupe looks for. One marker per agent and `internal/meta` rev.
|
||||
fn failure_comment(agent: &str, meta_rev: &str, error_tail: &str) -> (String, String) {
|
||||
let marker = format!("<!-- swarm-build-failed agent={agent} meta={meta_rev} -->");
|
||||
let mut start = error_tail.len().saturating_sub(COMMENT_TAIL_BYTES);
|
||||
while !error_tail.is_char_boundary(start) {
|
||||
start += 1;
|
||||
}
|
||||
let tail = error_tail[start..].replace("```", "'''");
|
||||
let body = format!(
|
||||
"{marker}\nThe swarm could not build `{agent}` from `internal/meta` at `{meta_rev}` \
|
||||
after this PR merged. The deploy is held and built again on every round until a \
|
||||
build succeeds.\n\nLast lines of `nix`:\n\n```\n{}\n```",
|
||||
tail.trim_end()
|
||||
);
|
||||
(marker, body)
|
||||
}
|
||||
|
||||
/// Tell `pr` that its merge's build failed, once per agent and `meta_rev`. A
|
||||
/// comment that cannot be posted is logged and changes nothing about the node.
|
||||
async fn report_failure(
|
||||
head: &impl MetaRepo,
|
||||
pr: u64,
|
||||
agent: &str,
|
||||
meta_rev: &str,
|
||||
error_tail: &str,
|
||||
) {
|
||||
let (marker, body) = failure_comment(agent, meta_rev, error_tail);
|
||||
match head.comment_once(agent, pr, &marker, &body).await {
|
||||
Ok(posted) => {
|
||||
tracing::info!(%agent, pr, %meta_rev, posted, "build_meta: reported the failed build on the PR");
|
||||
}
|
||||
Err(e) => tracing::warn!(
|
||||
%agent, pr, %meta_rev, error = %format!("{e:#}"),
|
||||
"build_meta: could not comment on the PR"
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Build `agent` from `internal/meta`'s head and record the closure.
|
||||
pub(crate) async fn run(
|
||||
head: Option<&impl MetaRepo>,
|
||||
queue: Option<&impl MetaBuilds>,
|
||||
agent: String,
|
||||
pr: Option<u64>,
|
||||
) -> Outcome {
|
||||
let Some(head) = head else {
|
||||
return no_verdict(
|
||||
|
|
@ -155,7 +221,12 @@ pub(crate) async fn run(
|
|||
match queue.build(request).await {
|
||||
Err(why) => no_verdict(why),
|
||||
Ok(FromMetaReply::Unavailable { reason }) => no_verdict(reason),
|
||||
Ok(FromMetaReply::Failed { error_tail }) => Outcome::Failed(error_tail),
|
||||
Ok(FromMetaReply::Failed { error_tail }) => {
|
||||
if let Some(pr) = pr {
|
||||
report_failure(head, pr, &agent, &meta_rev, &error_tail).await;
|
||||
}
|
||||
Outcome::Failed(error_tail)
|
||||
}
|
||||
Ok(FromMetaReply::Built { out_path, system }) => {
|
||||
let closure = AgentClosure {
|
||||
meta_rev,
|
||||
|
|
@ -184,6 +255,8 @@ mod tests {
|
|||
struct FakeHead {
|
||||
head: Result<Option<&'static str>, &'static str>,
|
||||
grant_fails: bool,
|
||||
/// The forge a PR comment goes to; `None` panics on a comment.
|
||||
comments: Option<std::sync::Arc<forge::Client>>,
|
||||
}
|
||||
|
||||
impl FakeHead {
|
||||
|
|
@ -191,6 +264,7 @@ mod tests {
|
|||
Self {
|
||||
head,
|
||||
grant_fails: false,
|
||||
comments: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -207,6 +281,19 @@ mod tests {
|
|||
anyhow::ensure!(!self.grant_fails, "the forge refused the collaborator");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn comment_once(
|
||||
&self,
|
||||
agent: &str,
|
||||
pr: u64,
|
||||
marker: &str,
|
||||
body: &str,
|
||||
) -> Result<bool> {
|
||||
let client = self.comments.as_ref().expect("no comment expected");
|
||||
client
|
||||
.comment_once(forge::CONFIG_ORG, agent, pr, marker, body)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
/// A queue that answers every build with `reply`, and keeps what it was
|
||||
|
|
@ -256,7 +343,11 @@ mod tests {
|
|||
}
|
||||
|
||||
async fn run_with(head: &FakeHead, queue: &FakeQueue) -> Outcome {
|
||||
run(Some(head), Some(queue), "atlas".to_owned()).await
|
||||
run(Some(head), Some(queue), "atlas".to_owned(), None).await
|
||||
}
|
||||
|
||||
async fn run_for_pr(head: &FakeHead, queue: &FakeQueue, pr: u64) -> Outcome {
|
||||
run(Some(head), Some(queue), "atlas".to_owned(), Some(pr)).await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
@ -374,6 +465,7 @@ mod tests {
|
|||
head.as_ref(),
|
||||
with_queue.then_some(&queue),
|
||||
"atlas".to_owned(),
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
let Outcome::Failed(error) = outcome else {
|
||||
|
|
@ -384,6 +476,137 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
const REV2: &str = "fedcba9876543210fedcba9876543210fedcba98";
|
||||
|
||||
fn nix_failed(tail: &str) -> FakeQueue {
|
||||
FakeQueue::answering(Ok(FromMetaReply::Failed {
|
||||
error_tail: tail.to_owned(),
|
||||
}))
|
||||
}
|
||||
|
||||
async fn head_with_comments(
|
||||
rev: &'static str,
|
||||
) -> (FakeHead, crate::forge::pr_comment::tests::Posted) {
|
||||
let (client, posted) = crate::forge::pr_comment::tests::stand_in().await;
|
||||
let head = FakeHead {
|
||||
comments: Some(std::sync::Arc::new(client)),
|
||||
..FakeHead::reading(Ok(Some(rev)))
|
||||
};
|
||||
(head, posted)
|
||||
}
|
||||
|
||||
/// A `nix` failure of a merged config PR's build is one comment on that PR
|
||||
/// with the agent, the `internal/meta` rev, `nix`'s tail and the retry.
|
||||
#[tokio::test]
|
||||
async fn a_red_build_of_a_merged_pr_comments_once_on_it() {
|
||||
let (head, posted) = head_with_comments(REV).await;
|
||||
let outcome = run_for_pr(&head, &nix_failed("error: builder for foo failed"), 7).await;
|
||||
assert_eq!(
|
||||
outcome,
|
||||
Outcome::Failed("error: builder for foo failed".to_owned())
|
||||
);
|
||||
let posted = posted.lock().unwrap();
|
||||
let [(path, body)] = posted.as_slice() else {
|
||||
panic!("one comment: {posted:?}")
|
||||
};
|
||||
assert_eq!(
|
||||
path,
|
||||
&format!("/api/v1/repos/agent-configs/atlas/{}/7/comments", "issues")
|
||||
);
|
||||
assert!(body.contains("`atlas`"), "{body}");
|
||||
assert!(body.contains(REV), "{body}");
|
||||
assert!(body.contains("error: builder for foo failed"), "{body}");
|
||||
assert!(body.contains("held"), "{body}");
|
||||
assert!(body.contains("every round"), "{body}");
|
||||
}
|
||||
|
||||
/// The timer builds a failed merge again on every round: the same agent and
|
||||
/// `internal/meta` rev is not commented twice. A new rev is a new failure,
|
||||
/// so it is.
|
||||
#[tokio::test]
|
||||
async fn the_retry_rounds_do_not_comment_again_but_a_new_meta_rev_does() {
|
||||
let (head, posted) = head_with_comments(REV).await;
|
||||
for _ in 0..3 {
|
||||
run_for_pr(&head, &nix_failed("error: boom"), 7).await;
|
||||
}
|
||||
let count = posted.lock().unwrap().len();
|
||||
assert_eq!(count, 1, "quiet retries");
|
||||
|
||||
let moved = FakeHead {
|
||||
comments: head.comments.clone(),
|
||||
..FakeHead::reading(Ok(Some(REV2)))
|
||||
};
|
||||
run_for_pr(&moved, &nix_failed("error: boom"), 7).await;
|
||||
let posted = posted.lock().unwrap();
|
||||
assert_eq!(posted.len(), 2, "{posted:?}");
|
||||
assert!(posted[1].1.contains(REV2), "{:?}", posted[1]);
|
||||
}
|
||||
|
||||
/// A build that got no verdict says nothing about the PR's config, so it
|
||||
/// leaves no comment. The `nix` failure arm is the control.
|
||||
#[tokio::test]
|
||||
async fn a_no_verdict_failure_does_not_comment() {
|
||||
for (name, reply) in [
|
||||
("no responders", Err("no swarm builder".to_owned())),
|
||||
(
|
||||
"unavailable",
|
||||
Ok(FromMetaReply::Unavailable {
|
||||
reason: "busy".to_owned(),
|
||||
}),
|
||||
),
|
||||
] {
|
||||
let (head, posted) = head_with_comments(REV).await;
|
||||
let outcome = run_for_pr(&head, &FakeQueue::answering(reply), 7).await;
|
||||
assert!(
|
||||
matches!(outcome, Outcome::Failed(e) if is_no_verdict(&e)),
|
||||
"{name}"
|
||||
);
|
||||
assert!(posted.lock().unwrap().is_empty(), "{name}");
|
||||
}
|
||||
let (head, posted) = head_with_comments(REV).await;
|
||||
run_for_pr(&head, &nix_failed("error: boom"), 7).await;
|
||||
assert_eq!(
|
||||
posted.lock().unwrap().len(),
|
||||
1,
|
||||
"control: a nix failure comments"
|
||||
);
|
||||
}
|
||||
|
||||
/// A build that is not a merged PR's (the pin fan-out, a created agent)
|
||||
/// has nobody to comment on.
|
||||
#[tokio::test]
|
||||
async fn a_failed_build_that_is_not_from_a_pr_does_not_comment() {
|
||||
let (head, posted) = head_with_comments(REV).await;
|
||||
let outcome = run_with(&head, &nix_failed("error: boom")).await;
|
||||
assert_eq!(outcome, Outcome::Failed("error: boom".to_owned()));
|
||||
assert!(posted.lock().unwrap().is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn the_comment_carries_the_end_of_a_long_tail_only() {
|
||||
let tail = format!("START{}END", "x".repeat(10_000));
|
||||
let (head, posted) = head_with_comments(REV).await;
|
||||
run_for_pr(&head, &nix_failed(&tail), 7).await;
|
||||
let posted = posted.lock().unwrap();
|
||||
let body = &posted[0].1;
|
||||
assert!(body.contains("END"), "the end of the tail is kept");
|
||||
assert!(!body.contains("START"), "the start is cut");
|
||||
assert!(body.len() < COMMENT_TAIL_BYTES + 1000, "{}", body.len());
|
||||
}
|
||||
|
||||
/// A forge that does not take the comment leaves the node failed as it was.
|
||||
#[tokio::test]
|
||||
async fn a_comment_that_cannot_be_posted_does_not_change_the_outcome() {
|
||||
let head = FakeHead {
|
||||
comments: Some(std::sync::Arc::new(crate::forge::Client::for_test_forge(
|
||||
url::Url::parse("http://127.0.0.1:1/").unwrap(),
|
||||
))),
|
||||
..FakeHead::reading(Ok(Some(REV)))
|
||||
};
|
||||
let outcome = run_for_pr(&head, &nix_failed("error: boom"), 7).await;
|
||||
assert_eq!(outcome, Outcome::Failed("error: boom".to_owned()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_repo_path_the_builder_fetches_is_the_one_the_controller_writes() {
|
||||
assert_eq!(
|
||||
|
|
|
|||
Loading…
Reference in a new issue