refactor(#3434): the job nodes carry the agent, not the repo too
Every repo in this graph is `agents/<agent>` -- the node payloads were carrying the same string under two names, and `create_agent` opened with a `let repo = agent.clone()` that said so out loud. All four node kinds now carry `agent` alone, and `forge::agent_repo` is the single home for the naming convention. The identity it returns is the point: a caller holding an agent name never writes a repo name itself, so changing the convention later is one edit rather than a search. `forge::Client`'s methods keep taking a repo, because they are a general forge client and `add_repo_member(repo, user)` is a real signature -- the derivation belongs at the call site that knows the two are the same here, not baked into an API that has no reason to assume it. `data()`'s four arms are now identical and merged into one or-pattern. Left as an explicit list rather than a catch-all so a fifth variant fails to compile here instead of silently rendering as an agent name.
This commit is contained in:
parent
f2790ab360
commit
4d526b8492
2 changed files with 45 additions and 29 deletions
|
|
@ -33,6 +33,18 @@ use crate::webhook::DeliveryKind;
|
||||||
/// the same forge instance, not a separate one, so the same org.
|
/// the same forge instance, not a separate one, so the same org.
|
||||||
pub const AGENTS_ORG: &str = "agents";
|
pub const AGENTS_ORG: &str = "agents";
|
||||||
|
|
||||||
|
/// The repo an agent's config lives in — one repo per agent inside
|
||||||
|
/// [`AGENTS_ORG`], named after the agent.
|
||||||
|
///
|
||||||
|
/// The identity here is the point, not an accident to be inlined: this is
|
||||||
|
/// the single home for the naming convention, so a caller holding an agent
|
||||||
|
/// name never writes the repo name itself. Callers used to pass both, which
|
||||||
|
/// meant every node payload carried the same string twice and any future
|
||||||
|
/// change to the convention would have been a search rather than an edit.
|
||||||
|
pub fn agent_repo(agent: &str) -> &str {
|
||||||
|
agent
|
||||||
|
}
|
||||||
|
|
||||||
/// The `operators` team, whitelisted for the merge gate on every repo
|
/// The `operators` team, whitelisted for the merge gate on every repo
|
||||||
/// this client protects — provisioned by `hive-c0re::forge::repos`
|
/// this client protects — provisioned by `hive-c0re::forge::repos`
|
||||||
/// already (`ensure_operators_team`), not re-provisioned here. If that
|
/// already (`ensure_operators_team`), not re-provisioned here. If that
|
||||||
|
|
|
||||||
|
|
@ -56,13 +56,13 @@ mod webhook;
|
||||||
enum SwarmNodeKind {
|
enum SwarmNodeKind {
|
||||||
/// Ensure `agent` exists as an authelia subject at the swarm level.
|
/// Ensure `agent` exists as an authelia subject at the swarm level.
|
||||||
CreateIdentity { agent: String },
|
CreateIdentity { agent: String },
|
||||||
/// Create `repo` in `forge::AGENTS_ORG` with the operator merge gate
|
/// Create the agent's repo in `forge::AGENTS_ORG` with the operator
|
||||||
/// on its default branch. See `forge::Client::create_repo`.
|
/// merge gate on its default branch. See `forge::Client::create_repo`.
|
||||||
CreateRepo { repo: String },
|
CreateRepo { agent: String },
|
||||||
/// Add `agent` as a write collaborator on `repo`. See
|
/// Add `agent` as a write collaborator on its own repo. See
|
||||||
/// `forge::Client::add_repo_member`.
|
/// `forge::Client::add_repo_member`.
|
||||||
AddRepoMember { repo: String, agent: String },
|
AddRepoMember { agent: String },
|
||||||
/// Seed `repo` with `agent.nix` + `flake.nix`. See
|
/// Seed the agent's repo with `agent.nix` + `flake.nix`. See
|
||||||
/// `forge::Client::seed_agent_config`.
|
/// `forge::Client::seed_agent_config`.
|
||||||
///
|
///
|
||||||
/// Deliberately carries no hive: seeding a config repo is the same
|
/// Deliberately carries no hive: seeding a config repo is the same
|
||||||
|
|
@ -70,7 +70,7 @@ enum SwarmNodeKind {
|
||||||
/// states nothing about where it runs. The hive is an address the
|
/// states nothing about where it runs. The hive is an address the
|
||||||
/// swarm routes a deploy message to — it belongs on the node that
|
/// swarm routes a deploy message to — it belongs on the node that
|
||||||
/// sends that message, not on this one.
|
/// sends that message, not on this one.
|
||||||
InitAgentConfigRepo { repo: String, agent: String },
|
InitAgentConfigRepo { agent: String },
|
||||||
}
|
}
|
||||||
|
|
||||||
impl hive_jobq_wire::WireNode for SwarmNodeKind {
|
impl hive_jobq_wire::WireNode for SwarmNodeKind {
|
||||||
|
|
@ -84,20 +84,18 @@ impl hive_jobq_wire::WireNode for SwarmNodeKind {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn data(&self, _id: hive_jobq_wire::WireId) -> serde_json::Value {
|
fn data(&self, _id: hive_jobq_wire::WireId) -> serde_json::Value {
|
||||||
|
// Every node in this graph is about exactly one agent, so they all
|
||||||
|
// render the same and `label()` is what distinguishes them. Spelled
|
||||||
|
// out as an or-pattern rather than a catch-all on purpose: a fifth
|
||||||
|
// variant then fails to compile here instead of silently rendering
|
||||||
|
// as an agent name.
|
||||||
match self {
|
match self {
|
||||||
SwarmNodeKind::CreateIdentity { agent } => {
|
SwarmNodeKind::CreateIdentity { agent }
|
||||||
|
| SwarmNodeKind::CreateRepo { agent }
|
||||||
|
| SwarmNodeKind::AddRepoMember { agent }
|
||||||
|
| SwarmNodeKind::InitAgentConfigRepo { agent } => {
|
||||||
serde_json::json!({ "agent": agent })
|
serde_json::json!({ "agent": agent })
|
||||||
}
|
}
|
||||||
SwarmNodeKind::CreateRepo { repo } => {
|
|
||||||
serde_json::json!({ "repo": repo })
|
|
||||||
}
|
|
||||||
// Same rendering, and that is not a coincidence to be split
|
|
||||||
// apart later: both nodes act on one repo for one agent, and
|
|
||||||
// `label()` is what tells a viewer which of the two it is.
|
|
||||||
SwarmNodeKind::AddRepoMember { repo, agent }
|
|
||||||
| SwarmNodeKind::InitAgentConfigRepo { repo, agent } => {
|
|
||||||
serde_json::json!({ "repo": repo, "agent": agent })
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -151,13 +149,13 @@ async fn run_swarm_node(
|
||||||
Err(e) => Outcome::Failed(format!("{e:#}")),
|
Err(e) => Outcome::Failed(format!("{e:#}")),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
SwarmNodeKind::CreateRepo { repo } => match deps.forge {
|
SwarmNodeKind::CreateRepo { agent } => match deps.forge {
|
||||||
None => Outcome::Failed(
|
None => Outcome::Failed(
|
||||||
"no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \
|
"no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \
|
||||||
SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)"
|
SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)"
|
||||||
.to_owned(),
|
.to_owned(),
|
||||||
),
|
),
|
||||||
Some(client) => match client.create_repo(&repo).await {
|
Some(client) => match client.create_repo(forge::agent_repo(&agent)).await {
|
||||||
Ok(full_name) => {
|
Ok(full_name) => {
|
||||||
tracing::info!(%full_name, "swarm jobq: create_repo done");
|
tracing::info!(%full_name, "swarm jobq: create_repo done");
|
||||||
Outcome::Done
|
Outcome::Done
|
||||||
|
|
@ -165,24 +163,30 @@ async fn run_swarm_node(
|
||||||
Err(e) => Outcome::Failed(format!("{e:#}")),
|
Err(e) => Outcome::Failed(format!("{e:#}")),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
SwarmNodeKind::AddRepoMember { repo, agent } => match deps.forge {
|
SwarmNodeKind::AddRepoMember { agent } => match deps.forge {
|
||||||
None => Outcome::Failed(
|
None => Outcome::Failed(
|
||||||
"no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \
|
"no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \
|
||||||
SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)"
|
SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)"
|
||||||
.to_owned(),
|
.to_owned(),
|
||||||
),
|
),
|
||||||
Some(client) => match client.add_repo_member(&repo, &agent).await {
|
Some(client) => match client
|
||||||
|
.add_repo_member(forge::agent_repo(&agent), &agent)
|
||||||
|
.await
|
||||||
|
{
|
||||||
Ok(()) => Outcome::Done,
|
Ok(()) => Outcome::Done,
|
||||||
Err(e) => Outcome::Failed(format!("{e:#}")),
|
Err(e) => Outcome::Failed(format!("{e:#}")),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
SwarmNodeKind::InitAgentConfigRepo { repo, agent } => match deps.forge {
|
SwarmNodeKind::InitAgentConfigRepo { agent } => match deps.forge {
|
||||||
None => Outcome::Failed(
|
None => Outcome::Failed(
|
||||||
"no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \
|
"no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \
|
||||||
SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)"
|
SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)"
|
||||||
.to_owned(),
|
.to_owned(),
|
||||||
),
|
),
|
||||||
Some(client) => match client.seed_agent_config(&repo, &agent).await {
|
Some(client) => match client
|
||||||
|
.seed_agent_config(forge::agent_repo(&agent), &agent)
|
||||||
|
.await
|
||||||
|
{
|
||||||
Ok(()) => Outcome::Done,
|
Ok(()) => Outcome::Done,
|
||||||
Err(e) => Outcome::Failed(format!("{e:#}")),
|
Err(e) => Outcome::Failed(format!("{e:#}")),
|
||||||
},
|
},
|
||||||
|
|
@ -639,7 +643,6 @@ async fn create_agent(
|
||||||
let agent = hive_types::Ident::parse(&req.name)
|
let agent = hive_types::Ident::parse(&req.name)
|
||||||
.map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))?
|
.map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))?
|
||||||
.into_string();
|
.into_string();
|
||||||
let repo = agent.clone();
|
|
||||||
|
|
||||||
// Two checks, and the second is the one that makes the field worth
|
// Two checks, and the second is the one that makes the field worth
|
||||||
// having: `Ident::parse` says the string is *shaped* like a hive name,
|
// having: `Ident::parse` says the string is *shaped* like a hive name,
|
||||||
|
|
@ -678,19 +681,20 @@ async fn create_agent(
|
||||||
agent: agent.clone(),
|
agent: agent.clone(),
|
||||||
});
|
});
|
||||||
let create_repo = b
|
let create_repo = b
|
||||||
.node(SwarmNodeKind::CreateRepo { repo: repo.clone() })
|
.node(SwarmNodeKind::CreateRepo {
|
||||||
|
agent: agent.clone(),
|
||||||
|
})
|
||||||
.after_ok(create_identity);
|
.after_ok(create_identity);
|
||||||
// Both fan out from `create_repo` directly — independent
|
// Both fan out from `create_repo` directly — independent
|
||||||
// operations on the same repo, no ordering requirement on
|
// operations on the same repo, no ordering requirement on
|
||||||
// each other (see the doc comment above).
|
// each other (see the doc comment above).
|
||||||
let _add_repo_member = b
|
let _add_repo_member = b
|
||||||
.node(SwarmNodeKind::AddRepoMember {
|
.node(SwarmNodeKind::AddRepoMember {
|
||||||
repo: repo.clone(),
|
|
||||||
agent: agent.clone(),
|
agent: agent.clone(),
|
||||||
})
|
})
|
||||||
.after_ok(create_repo);
|
.after_ok(create_repo);
|
||||||
let _init_config = b
|
let _init_config = b
|
||||||
.node(SwarmNodeKind::InitAgentConfigRepo { repo, agent })
|
.node(SwarmNodeKind::InitAgentConfigRepo { agent })
|
||||||
.after_ok(create_repo);
|
.after_ok(create_repo);
|
||||||
vec![create_identity.guid()]
|
vec![create_identity.guid()]
|
||||||
})
|
})
|
||||||
|
|
@ -1144,7 +1148,7 @@ mod tests {
|
||||||
let id = sched
|
let id = sched
|
||||||
.append(
|
.append(
|
||||||
SwarmNodeKind::CreateRepo {
|
SwarmNodeKind::CreateRepo {
|
||||||
repo: "atlas".to_owned(),
|
agent: "atlas".to_owned(),
|
||||||
},
|
},
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
None,
|
None,
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue