From 715cf33b892dca0b915fc6f4ed85fee728b46bd8 Mon Sep 17 00:00:00 2001 From: realyashnegi <54710562+yash27-lab@users.noreply.github.com> Date: Sat, 14 Feb 2026 21:26:47 -0500 Subject: [PATCH] fix(admin): return 503 when health deps are not ready (#1824) --- helm/rustfs/templates/deployment.yaml | 2 +- helm/rustfs/values.yaml | 2 +- rustfs/src/admin/console.rs | 44 ++--- rustfs/src/admin/handlers/health.rs | 197 +++++++++++++++++--- rustfs/src/admin/route_registration_test.rs | 4 +- rustfs/src/admin/router.rs | 10 +- rustfs/src/server/layer.rs | 2 +- rustfs/src/server/prefix.rs | 4 + rustfs/src/server/readiness.rs | 1 + 9 files changed, 210 insertions(+), 56 deletions(-) diff --git a/helm/rustfs/templates/deployment.yaml b/helm/rustfs/templates/deployment.yaml index 533975448..f67695cb5 100644 --- a/helm/rustfs/templates/deployment.yaml +++ b/helm/rustfs/templates/deployment.yaml @@ -111,7 +111,7 @@ spec: failureThreshold: 3 readinessProbe: httpGet: - path: /health + path: /health/ready port: 9000 initialDelaySeconds: 30 periodSeconds: 5 diff --git a/helm/rustfs/values.yaml b/helm/rustfs/values.yaml index 8e6341ca2..bdfcecb4a 100644 --- a/helm/rustfs/values.yaml +++ b/helm/rustfs/values.yaml @@ -190,7 +190,7 @@ livenessProbe: readinessProbe: httpGet: - path: /health + path: /health/ready port: endpoint initialDelaySeconds: 30 periodSeconds: 5 diff --git a/rustfs/src/admin/console.rs b/rustfs/src/admin/console.rs index e1a3ffe3d..8aa936273 100644 --- a/rustfs/src/admin/console.rs +++ b/rustfs/src/admin/console.rs @@ -12,9 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::admin::handlers::health::{HealthProbe, build_component_details, collect_dependency_readiness, health_check_state}; use crate::config::build; use crate::license::get_license; -use crate::server::{CONSOLE_PREFIX, FAVICON_PATH, HEALTH_PREFIX, RUSTFS_ADMIN_PREFIX}; +use crate::server::{CONSOLE_PREFIX, FAVICON_PATH, HEALTH_PREFIX, HEALTH_READY_PATH, RUSTFS_ADMIN_PREFIX}; use axum::{ Router, body::Body, @@ -410,6 +411,7 @@ fn setup_console_middleware_stack( .route(&format!("{CONSOLE_PREFIX}/license"), get(license_handler)) .route(&format!("{CONSOLE_PREFIX}/version"), get(version_handler)) .route(&format!("{CONSOLE_PREFIX}{HEALTH_PREFIX}"), get(health_check).head(health_check)) + .route(&format!("{CONSOLE_PREFIX}{HEALTH_READY_PATH}"), get(health_check).head(health_check)) .nest(CONSOLE_PREFIX, Router::new().fallback_service(get(static_handler))) .fallback_service(get(static_handler)); @@ -448,41 +450,29 @@ fn setup_console_middleware_stack( /// # Returns: /// - A `Response` containing the health check result. #[instrument] -async fn health_check(method: Method) -> Response { +async fn health_check(method: Method, uri: Uri) -> Response { + let probe = if uri.path().strip_prefix(CONSOLE_PREFIX) == Some(HEALTH_READY_PATH) { + HealthProbe::Readiness + } else { + HealthProbe::Liveness + }; + let (storage_ready, iam_ready) = collect_dependency_readiness(); + let health = health_check_state(storage_ready, iam_ready, probe); + let builder = Response::builder() - .status(StatusCode::OK) + .status(health.status_code) .header("content-type", "application/json"); + match method { // GET: Returns complete JSON Method::GET => { - let mut health_status = "ok"; - let mut details = json!({}); - - // Check storage backend health - if let Some(_store) = rustfs_ecstore::new_object_layer_fn() { - details["storage"] = json!({"status": "connected"}); - } else { - health_status = "degraded"; - details["storage"] = json!({"status": "disconnected"}); - } - - // Check IAM system health - match rustfs_iam::get() { - Ok(_) => { - details["iam"] = json!({"status": "connected"}); - } - Err(_) => { - health_status = "degraded"; - details["iam"] = json!({"status": "disconnected"}); - } - } - let body_json = json!({ - "status": health_status, + "status": health.status, + "ready": health.ready, "service": "rustfs-console", "timestamp": jiff::Zoned::now().to_string(), "version": env!("CARGO_PKG_VERSION"), - "details": details, + "details": build_component_details(storage_ready, iam_ready), "uptime": std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() diff --git a/rustfs/src/admin/handlers/health.rs b/rustfs/src/admin/handlers/health.rs index f8830afa1..8741a5e5b 100644 --- a/rustfs/src/admin/handlers/health.rs +++ b/rustfs/src/admin/handlers/health.rs @@ -14,17 +14,20 @@ use super::profile::{TriggerProfileCPU, TriggerProfileMemory}; use crate::admin::router::{AdminOperation, Operation, S3Router}; -use crate::server::{HEALTH_PREFIX, PROFILE_CPU_PATH, PROFILE_MEMORY_PATH}; +use crate::server::{HEALTH_PREFIX, HEALTH_READY_PATH, PROFILE_CPU_PATH, PROFILE_MEMORY_PATH}; use http::{HeaderMap, HeaderValue}; use hyper::{Method, StatusCode}; use matchit::Params; use s3s::header::CONTENT_TYPE; use s3s::{Body, S3Request, S3Response, S3Result}; +use serde_json::{Value, json}; pub fn register_health_route(r: &mut S3Router) -> std::io::Result<()> { // Health check endpoint for monitoring and orchestration r.insert(Method::GET, HEALTH_PREFIX, AdminOperation(&HealthCheckHandler {}))?; r.insert(Method::HEAD, HEALTH_PREFIX, AdminOperation(&HealthCheckHandler {}))?; + r.insert(Method::GET, HEALTH_READY_PATH, AdminOperation(&HealthCheckHandler {}))?; + r.insert(Method::HEAD, HEALTH_READY_PATH, AdminOperation(&HealthCheckHandler {}))?; r.insert(Method::GET, PROFILE_CPU_PATH, AdminOperation(&TriggerProfileCPU {}))?; r.insert(Method::GET, PROFILE_MEMORY_PATH, AdminOperation(&TriggerProfileMemory {}))?; @@ -34,11 +37,95 @@ pub fn register_health_route(r: &mut S3Router) -> std::io::Resul /// Health check handler for endpoint monitoring pub struct HealthCheckHandler {} +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct HealthCheckState { + pub(crate) status_code: StatusCode, + pub(crate) status: &'static str, + pub(crate) ready: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum HealthProbe { + Liveness, + Readiness, +} + +pub(crate) fn collect_dependency_readiness() -> (bool, bool) { + let storage_ready = rustfs_ecstore::new_object_layer_fn().is_some(); + let iam_ready = rustfs_iam::get().is_ok(); + (storage_ready, iam_ready) +} + +pub(crate) fn health_check_state(storage_ready: bool, iam_ready: bool, probe: HealthProbe) -> HealthCheckState { + let ready = storage_ready && iam_ready; + let status = if ready { "ok" } else { "degraded" }; + + let status_code = match probe { + HealthProbe::Liveness => StatusCode::OK, + HealthProbe::Readiness if ready => StatusCode::OK, + HealthProbe::Readiness => StatusCode::SERVICE_UNAVAILABLE, + }; + + HealthCheckState { + status_code, + status, + ready, + } +} + +pub(crate) fn build_component_details(storage_ready: bool, iam_ready: bool) -> Value { + json!({ + "storage": { + "status": if storage_ready { "connected" } else { "disconnected" }, + "ready": storage_ready, + }, + "iam": { + "status": if iam_ready { "connected" } else { "disconnected" }, + "ready": iam_ready, + } + }) +} + +pub(crate) fn probe_from_path(path: &str) -> HealthProbe { + if path == HEALTH_READY_PATH { + HealthProbe::Readiness + } else { + HealthProbe::Liveness + } +} + +pub(crate) fn build_health_response( + method: Method, + probe: HealthProbe, + storage_ready: bool, + iam_ready: bool, +) -> S3Response<(StatusCode, Body)> { + let health = health_check_state(storage_ready, iam_ready, probe); + let health_info = json!({ + "status": health.status, + "ready": health.ready, + "service": "rustfs-endpoint", + "timestamp": jiff::Zoned::now().to_string(), + "version": env!("CARGO_PKG_VERSION"), + "details": build_component_details(storage_ready, iam_ready) + }); + + let mut headers = HeaderMap::new(); + headers.insert(CONTENT_TYPE, HeaderValue::from_static("application/json")); + + if method == Method::HEAD { + return S3Response::with_headers((health.status_code, Body::empty()), headers); + } + + let body_str = serde_json::to_string(&health_info).unwrap_or_else(|_| "{}".to_string()); + let body = Body::from(body_str); + + S3Response::with_headers((health.status_code, body), headers) +} + #[async_trait::async_trait] impl Operation for HealthCheckHandler { async fn call(&self, req: S3Request, _params: Params<'_, '_>) -> S3Result> { - use serde_json::json; - // Extract the original HTTP Method (encapsulated by s3s into S3Request) let method = req.method; @@ -53,25 +140,91 @@ impl Operation for HealthCheckHandler { )); } - let health_info = json!({ - "status": "ok", - "service": "rustfs-endpoint", - "timestamp": jiff::Zoned::now().to_string(), - "version": env!("CARGO_PKG_VERSION") - }); + let probe = probe_from_path(req.uri.path()); + let (storage_ready, iam_ready) = collect_dependency_readiness(); - let mut headers = HeaderMap::new(); - headers.insert(CONTENT_TYPE, HeaderValue::from_static("application/json")); - - if method == http::Method::HEAD { - // HEAD: only returns the header and status code, not the body - return Ok(S3Response::with_headers((StatusCode::OK, Body::empty()), headers)); - } - - // GET: Return JSON body normally - let body_str = serde_json::to_string(&health_info).unwrap_or_else(|_| "{}".to_string()); - let body = Body::from(body_str); - - Ok(S3Response::with_headers((StatusCode::OK, body), headers)) + Ok(build_health_response(method, probe, storage_ready, iam_ready)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_readiness_state_ready() { + let state = health_check_state(true, true, HealthProbe::Readiness); + assert_eq!(state.status_code, StatusCode::OK); + assert_eq!(state.status, "ok"); + assert!(state.ready); + } + + #[test] + fn test_readiness_state_storage_not_ready() { + let state = health_check_state(false, true, HealthProbe::Readiness); + assert_eq!(state.status_code, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(state.status, "degraded"); + assert!(!state.ready); + } + + #[test] + fn test_liveness_state_iam_not_ready() { + let state = health_check_state(true, false, HealthProbe::Liveness); + assert_eq!(state.status_code, StatusCode::OK); + assert_eq!(state.status, "degraded"); + assert!(!state.ready); + } + + #[test] + fn test_readiness_state_iam_not_ready() { + let state = health_check_state(true, false, HealthProbe::Readiness); + assert_eq!(state.status_code, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(state.status, "degraded"); + assert!(!state.ready); + } + + #[test] + fn test_health_check_component_details() { + let details = build_component_details(true, false); + + assert_eq!(details["storage"]["status"], "connected"); + assert_eq!(details["storage"]["ready"], true); + assert_eq!(details["iam"]["status"], "disconnected"); + assert_eq!(details["iam"]["ready"], false); + } + + #[test] + fn test_probe_from_path_readiness() { + assert_eq!(probe_from_path(HEALTH_READY_PATH), HealthProbe::Readiness); + } + + #[test] + fn test_probe_from_path_liveness() { + assert_eq!(probe_from_path(HEALTH_PREFIX), HealthProbe::Liveness); + assert_eq!(probe_from_path("/random"), HealthProbe::Liveness); + } + + #[test] + fn test_build_health_response_readiness_returns_503_when_deps_not_ready() { + let resp = build_health_response(Method::GET, HealthProbe::Readiness, false, true); + assert_eq!(resp.output.0, StatusCode::SERVICE_UNAVAILABLE); + } + + #[test] + fn test_build_health_response_readiness_returns_200_when_deps_ready() { + let resp = build_health_response(Method::GET, HealthProbe::Readiness, true, true); + assert_eq!(resp.output.0, StatusCode::OK); + } + + #[test] + fn test_build_health_response_liveness_returns_200_when_deps_not_ready() { + let resp = build_health_response(Method::GET, HealthProbe::Liveness, false, false); + assert_eq!(resp.output.0, StatusCode::OK); + } + + #[test] + fn test_build_health_response_head_returns_empty_body() { + let resp = build_health_response(Method::HEAD, HealthProbe::Readiness, false, false); + assert_eq!(resp.output.0, StatusCode::SERVICE_UNAVAILABLE); } } diff --git a/rustfs/src/admin/route_registration_test.rs b/rustfs/src/admin/route_registration_test.rs index 25f661fe0..59ece9a94 100644 --- a/rustfs/src/admin/route_registration_test.rs +++ b/rustfs/src/admin/route_registration_test.rs @@ -16,7 +16,7 @@ use crate::admin::{ handlers::{bucket_meta, heal, health, kms, pools, profile_admin, quota, rebalance, replication, sts, system, tier, user}, router::{AdminOperation, S3Router}, }; -use crate::server::{ADMIN_PREFIX, HEALTH_PREFIX, PROFILE_CPU_PATH, PROFILE_MEMORY_PATH}; +use crate::server::{ADMIN_PREFIX, HEALTH_PREFIX, HEALTH_READY_PATH, PROFILE_CPU_PATH, PROFILE_MEMORY_PATH}; use hyper::Method; fn admin_path(path: &str) -> String { @@ -52,6 +52,8 @@ fn test_register_routes_cover_representative_admin_paths() { assert_route(&router, Method::GET, HEALTH_PREFIX); assert_route(&router, Method::HEAD, HEALTH_PREFIX); + assert_route(&router, Method::GET, HEALTH_READY_PATH); + assert_route(&router, Method::HEAD, HEALTH_READY_PATH); assert_route(&router, Method::GET, PROFILE_CPU_PATH); assert_route(&router, Method::GET, PROFILE_MEMORY_PATH); diff --git a/rustfs/src/admin/router.rs b/rustfs/src/admin/router.rs index 697b90c63..8c32c46ee 100644 --- a/rustfs/src/admin/router.rs +++ b/rustfs/src/admin/router.rs @@ -14,7 +14,7 @@ use crate::admin::console::is_console_path; use crate::admin::console::make_console_server; -use crate::server::{ADMIN_PREFIX, HEALTH_PREFIX, PROFILE_CPU_PATH, PROFILE_MEMORY_PATH, RPC_PREFIX}; +use crate::server::{ADMIN_PREFIX, HEALTH_PREFIX, HEALTH_READY_PATH, PROFILE_CPU_PATH, PROFILE_MEMORY_PATH, RPC_PREFIX}; use hyper::HeaderMap; use hyper::Method; use hyper::StatusCode; @@ -39,6 +39,10 @@ pub struct S3Router { console_router: Option>, } +fn is_public_health_path(path: &str) -> bool { + path == HEALTH_PREFIX || path == HEALTH_READY_PATH +} + impl S3Router { pub fn new(console_enabled: bool) -> Self { let router = Router::new(); @@ -99,7 +103,7 @@ where } // Health check - if (method == Method::HEAD || method == Method::GET) && path == HEALTH_PREFIX { + if (method == Method::HEAD || method == Method::GET) && is_public_health_path(path) { return true; } @@ -130,7 +134,7 @@ where } // Health check - if (req.method == Method::HEAD || req.method == Method::GET) && path == HEALTH_PREFIX { + if (req.method == Method::HEAD || req.method == Method::GET) && is_public_health_path(path) { return Ok(()); } diff --git a/rustfs/src/server/layer.rs b/rustfs/src/server/layer.rs index ccc6d3d9f..b74bc1712 100644 --- a/rustfs/src/server/layer.rs +++ b/rustfs/src/server/layer.rs @@ -109,7 +109,7 @@ impl ConditionalCorsLayer { } /// Exact paths that should be excluded from being treated as S3 paths. - const EXCLUDED_EXACT_PATHS: &'static [&'static str] = &["/health", "/profile/cpu", "/profile/memory"]; + const EXCLUDED_EXACT_PATHS: &'static [&'static str] = &["/health", "/health/ready", "/profile/cpu", "/profile/memory"]; fn is_s3_path(path: &str) -> bool { // Exclude Admin, Console, RPC, and configured special paths diff --git a/rustfs/src/server/prefix.rs b/rustfs/src/server/prefix.rs index 2dd920e7e..ab48d54b9 100644 --- a/rustfs/src/server/prefix.rs +++ b/rustfs/src/server/prefix.rs @@ -27,6 +27,10 @@ pub(crate) const FAVICON_PATH: &str = "/favicon.ico"; /// This path is used to check the health status of the server. pub(crate) const HEALTH_PREFIX: &str = "/health"; +/// Predefined readiness check path for RustFS server. +/// This path is used to check dependency readiness and may return 503. +pub(crate) const HEALTH_READY_PATH: &str = "/health/ready"; + /// Predefined administrative prefix for RustFS server routes. /// This prefix is used for endpoints that handle administrative tasks /// such as configuration, monitoring, and management. diff --git a/rustfs/src/server/readiness.rs b/rustfs/src/server/readiness.rs index 1050fe826..5f5e949b5 100644 --- a/rustfs/src/server/readiness.rs +++ b/rustfs/src/server/readiness.rs @@ -96,6 +96,7 @@ where crate::server::PROFILE_MEMORY_PATH | crate::server::PROFILE_CPU_PATH | crate::server::HEALTH_PREFIX + | crate::server::HEALTH_READY_PATH | crate::server::FAVICON_PATH );