From 9c1cfafeb5b0c8ad23648299a4e841552f408476 Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 15 Aug 2026 22:34:04 +0200 Subject: [PATCH] refactor(hive-c0re): one producer for the readiness verdict `get_health_ready` computed "degraded iff any warning is crit" inline and wrapped it in a private `ReadyBody`. The swarm status publisher needs the same verdict, and the warnings module's own doc already states why it must not compute its own: two systems independently deciding what counts as unhealthy is how they end up disagreeing. The disagreement would also be silent. Each side would look internally consistent, and the day a second degraded condition is added to one of them, the dashboard and the swarm view would report different things about the same host with nothing to flag it. `warnings::readiness()` is now the single producer and `Readiness` the single type. `ReadyBody` is deleted rather than made public: the endpoint keeps the part that genuinely is its own, the mapping onto an HTTP status code, and serves the shared document as its body. --- hive-c0re/src/dashboard/health.rs | 37 ++++++++++------------ hive-c0re/src/stats/warnings.rs | 52 +++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/hive-c0re/src/dashboard/health.rs b/hive-c0re/src/dashboard/health.rs index 6f5837fc..32a669e5 100644 --- a/hive-c0re/src/dashboard/health.rs +++ b/hive-c0re/src/dashboard/health.rs @@ -26,8 +26,6 @@ use axum::{ use serde::Serialize; use utoipa::ToSchema; -use crate::host_stats::ServerWarning; - #[derive(Serialize, ToSchema)] struct LiveBody { status: &'static str, @@ -44,40 +42,37 @@ pub(super) async fn get_health_live() -> Response { (StatusCode::OK, axum::Json(LiveBody { status: "ok" })).into_response() } -#[derive(Serialize, ToSchema)] -struct ReadyBody { - status: &'static str, - warnings: Vec, -} - /// Readiness. /// /// `200` with `{"status":"ok", "warnings": [...]}` unless a `crit`-level -/// warning is currently set in [`crate::warnings::snapshot`], in which -/// case `503` with `{"status":"degraded", ...}`. `warnings` always -/// carries the full current list (including `warn`-level entries not -/// affecting the status) so a poller gets detail either way. +/// warning is currently set, in which case `503` with +/// `{"status":"degraded", ...}`. `warnings` always carries the full +/// current list (including `warn`-level entries not affecting the status) +/// so a poller gets detail either way. +/// +/// The body is [`crate::warnings::Readiness`] rather than a type of this +/// module's own, and the verdict comes from +/// [`crate::warnings::readiness`] rather than being computed here. The +/// swarm status publisher offers that same document upward, and this +/// endpoint deciding "unhealthy" for itself is precisely how the two +/// would drift apart. What stays here is the only part that *is* this +/// endpoint's: the mapping onto an HTTP status code. #[utoipa::path( get, path = "/health/ready", responses( - (status = 200, description = "no crit-level warning set", body = ReadyBody), - (status = 503, description = "at least one crit-level warning set", body = ReadyBody), + (status = 200, description = "no crit-level warning set", body = crate::warnings::Readiness), + (status = 503, description = "at least one crit-level warning set", body = crate::warnings::Readiness), ), tag = "health" )] pub(super) async fn get_health_ready() -> Response { - let warnings = crate::warnings::snapshot(); - let degraded = warnings.iter().any(|w| w.level == "crit"); - let code = if degraded { + let body = crate::warnings::readiness(); + let code = if body.is_degraded() { StatusCode::SERVICE_UNAVAILABLE } else { StatusCode::OK }; - let body = ReadyBody { - status: if degraded { "degraded" } else { "ok" }, - warnings, - }; (code, axum::Json(body)).into_response() } diff --git a/hive-c0re/src/stats/warnings.rs b/hive-c0re/src/stats/warnings.rs index 9a322176..05ea79e3 100644 --- a/hive-c0re/src/stats/warnings.rs +++ b/hive-c0re/src/stats/warnings.rs @@ -30,6 +30,9 @@ use std::collections::BTreeMap; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Mutex, OnceLock}; +use serde::Serialize; +use utoipa::ToSchema; + use crate::host_stats::ServerWarning; /// Monotonic guard-id source. Each [`set_warning`] mints a fresh id so a @@ -89,6 +92,55 @@ pub fn set_warning( WarningGuard { kind, id } } +/// `status` value for a hive with no `crit`-level warning set. +pub const STATUS_OK: &str = "ok"; +/// `status` value for a hive with at least one `crit`-level warning set. +pub const STATUS_DEGRADED: &str = "degraded"; + +/// What this hive currently says about its own health. +/// +/// One type with one producer ([`readiness`]) because there is more than +/// one consumer: `/health/ready` answers a poller with it, and the swarm +/// status publisher offers the same document upward. **Two consumers each +/// deciding for themselves what counts as unhealthy is how they end up +/// disagreeing** — and the disagreement would be invisible, since each +/// would look internally consistent. The day a second degraded condition +/// is added, it is added here and both consumers get it. +#[derive(Debug, Clone, Serialize, ToSchema)] +pub struct Readiness { + /// [`STATUS_OK`] or [`STATUS_DEGRADED`]. + pub status: &'static str, + /// The full current warning list, `warn`-level entries included even + /// though they do not affect `status` — a consumer gets the detail + /// either way rather than having to ask twice. + pub warnings: Vec, +} + +impl Readiness { + /// Whether anything `crit`-level is set. The predicate lives next to + /// the constants that encode it so a caller never spells the string. + #[must_use] + pub fn is_degraded(&self) -> bool { + self.status == STATUS_DEGRADED + } +} + +/// Derive the readiness verdict from the current registry contents. +/// +/// The rule — degraded iff any warning is `crit` — is stated exactly +/// once, here. `warn` is deliberately not degrading: it is the level for +/// "an operator should look", not "stop sending me work". +#[must_use] +pub fn readiness() -> Readiness { + let warnings = snapshot(); + let status = if warnings.iter().any(|w| w.level == "crit") { + STATUS_DEGRADED + } else { + STATUS_OK + }; + Readiness { status, warnings } +} + /// Snapshot of the currently-active warnings, kind-sorted. Cheap read /// behind the registry mutex — safe to call on every `/api/state`. #[must_use]