Skip to content

Commit d9787b3

Browse files
committed
feat(gateway): add readiness probe metrics and test-only store close
Emit Prometheus readiness metrics for database probes (healthy gauge and outcome-labeled latency histogram) with coverage in health HTTP tests. Restrict Store::close behind test support cfg to prevent accidental runtime pool shutdown under live traffic. Signed-off-by: Adrien Langou <alangou@nvidia.com>
1 parent d255cdd commit d9787b3

21 files changed

Lines changed: 805 additions & 37 deletions

architecture/gateway.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ health, metrics, or tunnel routes. The plaintext service router also rejects
3333
browser requests whose Fetch Metadata, Origin, or Referer headers indicate a
3434
cross-origin or sibling-subdomain request.
3535

36+
Dedicated health listeners expose `/healthz` (process liveness only) and
37+
`/readyz` (dependency-aware readiness). Readiness reflects the latest result
38+
of an in-process background task that pings the persistence layer on a
39+
fixed cadence; the handler reads a cached state, so responses are
40+
sub-millisecond and never race the kubelet probe timeout.
41+
3642
Supported auth modes:
3743

3844
| Mode | Use |

crates/openshell-server/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ x509-parser = "0.16"
9393

9494
[features]
9595
dev-settings = ["openshell-core/dev-settings"]
96+
test-support = []
9697

9798
[dev-dependencies]
9899
hyper-rustls = { version = "0.27", default-features = false, features = ["native-tokio", "http1", "tls12", "logging", "ring", "webpki-tokio"] }

crates/openshell-server/src/http.rs

Lines changed: 271 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,19 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
//! HTTP health endpoints using Axum.
5+
//!
6+
//! Three endpoints with distinct semantics:
7+
//! - `/healthz` — Kubernetes liveness probe. Returns `200 OK` whenever the
8+
//! process is responsive. Intentionally does NOT depend on the database
9+
//! so a transient outage does not cascade into a `CrashLoopBackOff`.
10+
//! - `/readyz` — Kubernetes readiness probe. Reads the cached state
11+
//! published by [`crate::readiness::DatabaseHealthMonitor`] and returns
12+
//! `503 Service Unavailable` when the latest background check failed.
13+
//! Handler latency is sub-millisecond: the database is never pinged from
14+
//! inside the request path, so the response cannot race the kubelet's
15+
//! probe timeout.
16+
//! - `/health` — Alias of `/readyz` for external monitors
17+
//! that conventionally probe `/health`.
518
619
use axum::{
720
Json, Router,
@@ -14,43 +27,147 @@ use axum::{
1427
use metrics_exporter_prometheus::PrometheusHandle;
1528
use serde::Serialize;
1629
use std::sync::Arc;
30+
use tokio::sync::watch;
1731

18-
/// Health check response.
32+
use crate::persistence::Store;
33+
use crate::readiness::{DatabaseHealthMonitor, HealthError, HealthState};
34+
35+
const STATUS_HEALTHY: &str = "healthy";
36+
const STATUS_UNHEALTHY: &str = "unhealthy";
37+
const DATABASE_INITIALIZING_ERROR: &str = "readiness monitor still initializing";
38+
const DATABASE_UNAVAILABLE_ERROR: &str = "database unavailable";
39+
const DATABASE_TIMEOUT_ERROR: &str = "database health check timed out";
40+
41+
#[derive(Clone)]
42+
struct HealthRouterState {
43+
health: watch::Receiver<HealthState>,
44+
}
45+
46+
/// Per-dependency check entry exposed under `checks` in the JSON payload.
47+
#[derive(Debug, Serialize)]
48+
pub struct DependencyCheck {
49+
/// `"healthy"` or `"unhealthy"`.
50+
pub status: &'static str,
51+
52+
/// Wall-clock time of the latest background ping, when measurable.
53+
#[serde(skip_serializing_if = "Option::is_none")]
54+
pub latency_ms: Option<u64>,
55+
56+
/// Failure detail. Absent on success.
57+
#[serde(skip_serializing_if = "Option::is_none")]
58+
pub error: Option<String>,
59+
}
60+
61+
/// Aggregated dependency results.
62+
#[derive(Debug, Serialize)]
63+
pub struct HealthChecks {
64+
pub database: DependencyCheck,
65+
}
66+
67+
/// Readiness response payload.
1968
#[derive(Debug, Serialize)]
2069
pub struct HealthResponse {
21-
/// Service status.
70+
/// Overall status: `"healthy"` if every dependency is healthy.
2271
pub status: &'static str,
2372

2473
/// Service version.
2574
pub version: &'static str,
26-
}
2775

28-
/// Simple health check - returns 200 OK.
29-
async fn health() -> impl IntoResponse {
30-
StatusCode::OK
76+
/// Per-dependency breakdown.
77+
pub checks: HealthChecks,
3178
}
3279

33-
/// Kubernetes liveness probe.
80+
/// Kubernetes liveness probe — process responsiveness only.
3481
async fn healthz() -> impl IntoResponse {
3582
StatusCode::OK
3683
}
3784

38-
/// Kubernetes readiness probe with detailed status.
39-
async fn readyz() -> impl IntoResponse {
85+
/// Kubernetes readiness probe — reflects the cached background DB state.
86+
async fn readyz(State(state): State<Arc<HealthRouterState>>) -> impl IntoResponse {
87+
render_response(&state.health.borrow())
88+
}
89+
90+
/// Convenience alias of [`readyz`] for monitors that probe `/health`.
91+
async fn health(State(state): State<Arc<HealthRouterState>>) -> impl IntoResponse {
92+
render_response(&state.health.borrow())
93+
}
94+
95+
fn render_response(state: &HealthState) -> (StatusCode, Json<HealthResponse>) {
96+
let database = render_database(state);
97+
let healthy = state.is_healthy();
4098
let response = HealthResponse {
41-
status: "healthy",
99+
status: if healthy {
100+
STATUS_HEALTHY
101+
} else {
102+
STATUS_UNHEALTHY
103+
},
42104
version: openshell_core::VERSION,
105+
checks: HealthChecks { database },
43106
};
107+
let code = if healthy {
108+
StatusCode::OK
109+
} else {
110+
StatusCode::SERVICE_UNAVAILABLE
111+
};
112+
(code, Json(response))
113+
}
114+
115+
fn render_database(state: &HealthState) -> DependencyCheck {
116+
match state {
117+
HealthState::Initializing => DependencyCheck {
118+
status: STATUS_UNHEALTHY,
119+
latency_ms: None,
120+
error: Some(DATABASE_INITIALIZING_ERROR.to_string()),
121+
},
122+
HealthState::Healthy { latency_ms } => DependencyCheck {
123+
status: STATUS_HEALTHY,
124+
latency_ms: Some(*latency_ms),
125+
error: None,
126+
},
127+
HealthState::Unhealthy {
128+
reason: HealthError::Unavailable,
129+
latency_ms,
130+
} => DependencyCheck {
131+
status: STATUS_UNHEALTHY,
132+
latency_ms: *latency_ms,
133+
error: Some(DATABASE_UNAVAILABLE_ERROR.to_string()),
134+
},
135+
HealthState::Unhealthy {
136+
reason: HealthError::Timeout,
137+
..
138+
} => DependencyCheck {
139+
status: STATUS_UNHEALTHY,
140+
latency_ms: None,
141+
error: Some(DATABASE_TIMEOUT_ERROR.to_string()),
142+
},
143+
}
144+
}
44145

45-
(StatusCode::OK, Json(response))
146+
/// Build the health router by spawning a background [`DatabaseHealthMonitor`]
147+
/// for `store` and wiring its receiver into the handlers.
148+
///
149+
/// Awaits the monitor's first poll before returning so callers (tests and
150+
/// the server runtime) get a router whose `/readyz` already reflects the
151+
/// real database state by the time it accepts traffic. The background task
152+
/// continues running detached for the remainder of the runtime.
153+
pub async fn health_router(store: Arc<Store>) -> Router {
154+
let mut monitor = DatabaseHealthMonitor::spawn(store);
155+
monitor.wait_until_polled().await;
156+
health_router_from_receiver(monitor.subscribe())
46157
}
47158

48-
/// Create the health router.
49-
pub fn health_router() -> Router {
159+
/// Build the health router from an existing monitor receiver.
160+
///
161+
/// Crate-internal: used by [`health_router`] and by tests that drive the
162+
/// `HealthState` directly without spinning up the polling task.
163+
pub fn health_router_from_receiver(receiver: watch::Receiver<HealthState>) -> Router {
164+
let state = Arc::new(HealthRouterState { health: receiver });
165+
50166
Router::new()
51167
.route("/health", get(health))
52168
.route("/healthz", get(healthz))
53169
.route("/readyz", get(readyz))
170+
.with_state(state)
54171
}
55172

56173
/// Create the metrics router for the dedicated metrics port.
@@ -64,7 +181,7 @@ async fn render_metrics(State(handle): State<PrometheusHandle>) -> impl IntoResp
64181
handle.render()
65182
}
66183

67-
/// Create the HTTP router.
184+
/// Create the HTTP router served on the multiplexed gateway port.
68185
pub fn http_router(state: Arc<crate::ServerState>) -> Router {
69186
crate::ws_tunnel::router(state.clone())
70187
.merge(crate::auth::router(state.clone()))
@@ -305,3 +422,143 @@ mod tests {
305422
assert!(!browser_context_allows_plaintext_service_request(&req));
306423
}
307424
}
425+
426+
#[cfg(test)]
427+
mod readiness_tests {
428+
use super::*;
429+
use axum::body::Body;
430+
use http::Request;
431+
use http_body_util::BodyExt;
432+
use tower::ServiceExt;
433+
434+
async fn in_memory_store() -> Arc<Store> {
435+
Arc::new(
436+
Store::connect("sqlite::memory:")
437+
.await
438+
.expect("connect in-memory sqlite store"),
439+
)
440+
}
441+
442+
async fn get(router: Router, path: &str) -> (StatusCode, serde_json::Value) {
443+
let response = router
444+
.oneshot(Request::get(path).body(Body::empty()).unwrap())
445+
.await
446+
.expect("router responds");
447+
let status = response.status();
448+
let bytes = response
449+
.into_body()
450+
.collect()
451+
.await
452+
.expect("collect body")
453+
.to_bytes();
454+
let body = if bytes.is_empty() {
455+
serde_json::Value::Null
456+
} else {
457+
serde_json::from_slice(&bytes).expect("response is valid JSON")
458+
};
459+
(status, body)
460+
}
461+
462+
/// Build a router whose state is driven by a `HealthState` we control,
463+
/// so each handler-shape test can pin the exact mapping under test.
464+
fn router_with_state(state: HealthState) -> Router {
465+
let (_tx, rx) = watch::channel(state);
466+
health_router_from_receiver(rx)
467+
}
468+
469+
#[tokio::test]
470+
async fn healthz_is_minimal_and_does_not_touch_the_database() {
471+
// Liveness must succeed even when the database is unreachable —
472+
// otherwise a transient outage would CrashLoopBackOff the gateway.
473+
let store = in_memory_store().await;
474+
store.close().await;
475+
let (status, body) = get(health_router(store).await, "/healthz").await;
476+
assert_eq!(status, StatusCode::OK);
477+
assert!(body.is_null(), "healthz must return an empty body");
478+
}
479+
480+
#[tokio::test]
481+
async fn readyz_returns_200_with_healthy_payload_when_db_is_reachable() {
482+
let store = in_memory_store().await;
483+
let (status, body) = get(health_router(store).await, "/readyz").await;
484+
assert_eq!(status, StatusCode::OK);
485+
assert_eq!(body["status"], "healthy");
486+
assert_eq!(body["checks"]["database"]["status"], "healthy");
487+
assert!(
488+
body["checks"]["database"]["latency_ms"].is_number(),
489+
"expected latency_ms in healthy payload"
490+
);
491+
assert!(
492+
body["checks"]["database"]["error"].is_null(),
493+
"healthy payload must omit the error field"
494+
);
495+
}
496+
497+
#[tokio::test]
498+
async fn health_alias_mirrors_readyz_when_db_is_reachable() {
499+
let store = in_memory_store().await;
500+
let (status, body) = get(health_router(store).await, "/health").await;
501+
assert_eq!(status, StatusCode::OK);
502+
assert_eq!(body["status"], "healthy");
503+
assert_eq!(body["checks"]["database"]["status"], "healthy");
504+
}
505+
506+
#[tokio::test]
507+
async fn readyz_returns_503_with_unhealthy_payload_when_db_is_unreachable() {
508+
let store = in_memory_store().await;
509+
store.close().await;
510+
let (status, body) = get(health_router(store).await, "/readyz").await;
511+
assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE);
512+
assert_eq!(body["status"], "unhealthy");
513+
assert_eq!(body["checks"]["database"]["status"], "unhealthy");
514+
assert_eq!(
515+
body["checks"]["database"]["error"],
516+
DATABASE_UNAVAILABLE_ERROR
517+
);
518+
}
519+
520+
#[tokio::test]
521+
async fn health_alias_returns_503_when_db_is_unreachable() {
522+
let store = in_memory_store().await;
523+
store.close().await;
524+
let (status, body) = get(health_router(store).await, "/health").await;
525+
assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE);
526+
assert_eq!(body["status"], "unhealthy");
527+
assert_eq!(body["checks"]["database"]["status"], "unhealthy");
528+
}
529+
530+
#[tokio::test]
531+
async fn readyz_reports_initializing_state_as_unhealthy_with_explicit_reason() {
532+
let (status, body) = get(router_with_state(HealthState::Initializing), "/readyz").await;
533+
assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE);
534+
assert_eq!(body["status"], "unhealthy");
535+
assert_eq!(body["checks"]["database"]["status"], "unhealthy");
536+
assert_eq!(
537+
body["checks"]["database"]["error"],
538+
DATABASE_INITIALIZING_ERROR
539+
);
540+
assert!(
541+
body["checks"]["database"]["latency_ms"].is_null(),
542+
"initializing state has no latency to report yet"
543+
);
544+
}
545+
546+
#[tokio::test]
547+
async fn readyz_renders_timeout_state_with_dedicated_error_string() {
548+
let (status, body) = get(
549+
router_with_state(HealthState::Unhealthy {
550+
reason: HealthError::Timeout,
551+
latency_ms: None,
552+
}),
553+
"/readyz",
554+
)
555+
.await;
556+
assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE);
557+
assert_eq!(body["status"], "unhealthy");
558+
assert_eq!(body["checks"]["database"]["error"], DATABASE_TIMEOUT_ERROR);
559+
assert!(
560+
body["checks"]["database"]["latency_ms"].is_null(),
561+
"timeout state has no completed-call latency"
562+
);
563+
}
564+
}

crates/openshell-server/src/lib.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ mod multiplex;
3232
mod persistence;
3333
pub(crate) mod policy_store;
3434
mod provider_refresh;
35+
mod readiness;
3536
mod sandbox_index;
3637
mod sandbox_watch;
3738
mod service_routing;
@@ -57,7 +58,7 @@ pub use grpc::OpenShellService;
5758
pub use http::{health_router, http_router, metrics_router, service_http_router};
5859
pub use multiplex::{MultiplexService, MultiplexedService};
5960
use openshell_driver_kubernetes::KubernetesComputeConfig;
60-
use persistence::Store;
61+
pub use persistence::Store;
6162
use sandbox_index::SandboxIndex;
6263
use sandbox_watch::SandboxWatchBus;
6364
pub use tls::TlsAcceptor;
@@ -171,6 +172,7 @@ pub async fn run_server(
171172
if database_url.is_empty() {
172173
return Err(Error::config("database_url is required"));
173174
}
175+
174176
let store = Arc::new(Store::connect(database_url).await?);
175177

176178
let oidc_cache = if let Some(ref oidc) = config.oidc {
@@ -255,9 +257,11 @@ pub async fn run_server(
255257
))
256258
})?;
257259
info!(address = %health_bind_address, "Health server listening");
260+
// `health_router` awaits the readiness monitor's first poll so the
261+
// listener never serves a stale-by-default state on its first probe.
262+
let router = health_router(store.clone()).await;
258263
tokio::spawn(async move {
259-
if let Err(e) = axum::serve(health_listener, health_router().into_make_service()).await
260-
{
264+
if let Err(e) = axum::serve(health_listener, router.into_make_service()).await {
261265
error!("Health server error: {e}");
262266
}
263267
});

0 commit comments

Comments
 (0)