hive-c0re: test build_port_conflicts, and delete the claim it contradicted
The doc comment said the manager sits at a fixed 8000 and therefore cannot collide with a sub-agent. No such special case exists: ContainerView.port is agent_web_port(name) for every container, and that function hashes every name — including the manager's — into one range. The only 8000s in the tree are test-fixture arguments in meta.rs. The code was always right; the comment would send an operator hunting a detector bug instead of renaming an agent. It now points at the allocator that owns the rule, and the manager-collides case is a test rather than a sentence. Nine tests on a module that had none, covering the cluster grouping, the name sort, the port ordering, and that a stopped container still claims its port.
This commit is contained in:
parent
91ab5f9d9c
commit
877e8bcf81
1 changed files with 104 additions and 5 deletions
|
|
@ -342,11 +342,13 @@ pub(super) async fn api_state(
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Group live containers by their assigned web UI port; clusters with
|
/// Group containers by their assigned web UI port; clusters with more
|
||||||
/// more than one member are port-hash collisions the operator needs
|
/// than one member are port-hash collisions the operator resolves by
|
||||||
/// to resolve by renaming. Manager (fixed at 8000) and sub-agents
|
/// renaming an agent.
|
||||||
/// (8100..8999) can't collide with each other — collisions are
|
///
|
||||||
/// strictly between sub-agents.
|
/// Every container's port comes from [`crate::lifecycle::agent_web_port`],
|
||||||
|
/// which hashes the name into one range for all of them — so the manager
|
||||||
|
/// is a candidate for a collision exactly like any sub-agent.
|
||||||
fn build_port_conflicts(containers: &[ContainerView]) -> Vec<PortConflict> {
|
fn build_port_conflicts(containers: &[ContainerView]) -> Vec<PortConflict> {
|
||||||
let mut by_port: std::collections::BTreeMap<u16, Vec<String>> =
|
let mut by_port: std::collections::BTreeMap<u16, Vec<String>> =
|
||||||
std::collections::BTreeMap::new();
|
std::collections::BTreeMap::new();
|
||||||
|
|
@ -712,3 +714,100 @@ pub(super) async fn dashboard_stream(
|
||||||
});
|
});
|
||||||
Sse::new(stream).keep_alive(KeepAlive::default())
|
Sse::new(stream).keep_alive(KeepAlive::default())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::{ContainerView, build_port_conflicts};
|
||||||
|
|
||||||
|
fn cv(name: &str, port: u16, running: bool) -> ContainerView {
|
||||||
|
ContainerView {
|
||||||
|
name: name.to_owned(),
|
||||||
|
container: format!("h-{name}"),
|
||||||
|
port,
|
||||||
|
running,
|
||||||
|
failed: false,
|
||||||
|
needs_update: false,
|
||||||
|
needs_login: false,
|
||||||
|
deployed_sha: None,
|
||||||
|
parent: None,
|
||||||
|
active_model: None,
|
||||||
|
paused: false,
|
||||||
|
cpu_quota: "200%".to_owned(),
|
||||||
|
memory_max: "4G".to_owned(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn distinct_ports_produce_no_conflicts() {
|
||||||
|
let cs = [cv("alice", 8101, true), cv("bob", 8102, true)];
|
||||||
|
assert!(build_port_conflicts(&cs).is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_lone_container_does_not_conflict_with_itself() {
|
||||||
|
assert!(build_port_conflicts(&[cv("alice", 8101, true)]).is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn two_agents_on_one_port_are_reported_together() {
|
||||||
|
let cs = [cv("alice", 8101, true), cv("bob", 8101, true)];
|
||||||
|
let got = build_port_conflicts(&cs);
|
||||||
|
assert_eq!(got.len(), 1);
|
||||||
|
assert_eq!(got[0].port, 8101);
|
||||||
|
assert_eq!(got[0].agents, ["alice", "bob"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The manager takes its port from the same hash as everyone else, so
|
||||||
|
/// it collides like any other name. A doc comment here used to claim it
|
||||||
|
/// sat at a fixed 8000 and could not — no such special case exists.
|
||||||
|
#[test]
|
||||||
|
fn the_manager_can_collide_with_a_sub_agent() {
|
||||||
|
let cs = [cv("manager", 8101, true), cv("alice", 8101, true)];
|
||||||
|
let got = build_port_conflicts(&cs);
|
||||||
|
assert_eq!(got.len(), 1);
|
||||||
|
assert_eq!(got[0].agents, ["alice", "manager"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A stopped agent keeps its hashed port, so the clash is real the
|
||||||
|
/// moment it starts. Reporting it while it is down is the point.
|
||||||
|
#[test]
|
||||||
|
fn a_stopped_container_still_claims_its_port() {
|
||||||
|
let cs = [cv("alice", 8101, true), cv("bob", 8101, false)];
|
||||||
|
assert_eq!(build_port_conflicts(&cs).len(), 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn names_are_sorted_within_a_conflict() {
|
||||||
|
let cs = [cv("zoe", 8101, true), cv("adam", 8101, true)];
|
||||||
|
assert_eq!(build_port_conflicts(&cs)[0].agents, ["adam", "zoe"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn more_than_two_agents_land_in_one_cluster() {
|
||||||
|
let cs = [
|
||||||
|
cv("c", 8101, true),
|
||||||
|
cv("a", 8101, true),
|
||||||
|
cv("b", 8101, true),
|
||||||
|
];
|
||||||
|
let got = build_port_conflicts(&cs);
|
||||||
|
assert_eq!(got.len(), 1, "one cluster, not one row per pair");
|
||||||
|
assert_eq!(got[0].agents, ["a", "b", "c"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn separate_clusters_are_ordered_by_port() {
|
||||||
|
let cs = [
|
||||||
|
cv("d", 8300, true),
|
||||||
|
cv("c", 8300, true),
|
||||||
|
cv("b", 8200, true),
|
||||||
|
cv("a", 8200, true),
|
||||||
|
];
|
||||||
|
let ports: Vec<u16> = build_port_conflicts(&cs).iter().map(|p| p.port).collect();
|
||||||
|
assert_eq!(ports, [8200, 8300]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn no_containers_is_not_a_conflict() {
|
||||||
|
assert!(build_port_conflicts(&[]).is_empty());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue