From 76a861f815387556b5fdfc37610c33812aabc97f Mon Sep 17 00:00:00 2001 From: houseme Date: Tue, 25 Aug 2026 14:26:14 +0800 Subject: [PATCH] fix(health): align ready with lock quorum (#6554) Treat lock quorum as part of node readiness for both /health and /health/ready response bodies while preserving the /health liveness HTTP 200 contract. Add focused regression coverage for lock-quorum-only degradation and make the public /health layer fixture independent from process-global readiness state. Refs: rustfs/backlog#2011 Co-authored-by: heihutu --- rustfs/src/admin/handlers/health.rs | 58 ++++++++++++++++++++- rustfs/src/server/health.rs | 34 +++++++++--- rustfs/src/server/layer.rs | 80 +++++++++++++++-------------- 3 files changed, 124 insertions(+), 48 deletions(-) diff --git a/rustfs/src/admin/handlers/health.rs b/rustfs/src/admin/handlers/health.rs index 29370b6a7..6925e1282 100644 --- a/rustfs/src/admin/handlers/health.rs +++ b/rustfs/src/admin/handlers/health.rs @@ -128,9 +128,17 @@ mod tests { #[test] fn test_readiness_state_lock_not_ready() { let state = health_check_state(true, true, 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_lock_not_ready() { + let state = health_check_state(true, true, false, true, HealthProbe::Liveness); assert_eq!(state.status_code, StatusCode::OK); - assert_eq!(state.status, "ok"); - assert!(state.ready); + assert_eq!(state.status, "degraded"); + assert!(!state.ready); } #[test] @@ -267,6 +275,52 @@ mod tests { assert!(payload.get("degradedReasons").is_some()); } + #[test] + #[serial] + fn test_health_and_readiness_body_agree_when_only_lock_quorum_is_unavailable() { + with_var(rustfs_config::ENV_HEALTH_MINIMAL_RESPONSE_ENABLE, Some("false"), || { + let readiness_report = crate::shared_types::DependencyReadinessReport { + readiness: crate::shared_types::DependencyReadiness { + storage_ready: true, + iam_ready: true, + lock_quorum_ready: false, + peer_health_ready: true, + }, + degraded_reasons: vec![crate::shared_types::ReadinessDegradedReason::LockQuorumUnavailable], + }; + + let liveness = build_health_response_parts( + Method::GET, + HealthProbe::Liveness, + Some(&readiness_report), + "rustfs-endpoint", + None, + None, + ); + let readiness = build_health_response_parts( + Method::GET, + HealthProbe::Readiness, + Some(&readiness_report), + "rustfs-endpoint", + None, + None, + ); + + assert_eq!(liveness.status_code, StatusCode::OK); + assert_eq!(readiness.status_code, StatusCode::SERVICE_UNAVAILABLE); + let liveness_payload = liveness.payload.expect("GET should include liveness payload"); + let readiness_payload = readiness.payload.expect("GET should include readiness payload"); + assert_eq!(liveness_payload["status"], "degraded"); + assert_eq!(readiness_payload["status"], "degraded"); + assert_eq!(liveness_payload["ready"], false); + assert_eq!(readiness_payload["ready"], false); + assert_eq!(liveness_payload["details"]["lock"]["ready"], false); + assert_eq!(readiness_payload["details"]["lock"]["ready"], false); + assert_eq!(liveness_payload["degradedReasons"][0], "lock_quorum_unavailable"); + assert_eq!(readiness_payload["degradedReasons"][0], "lock_quorum_unavailable"); + }); + } + #[test] fn test_build_health_response_head_returns_empty_body() { let readiness_report = crate::shared_types::DependencyReadinessReport { diff --git a/rustfs/src/server/health.rs b/rustfs/src/server/health.rs index 879b851fe..563355b4f 100644 --- a/rustfs/src/server/health.rs +++ b/rustfs/src/server/health.rs @@ -39,12 +39,6 @@ pub(crate) enum HealthProbe { ClusterRead, } -impl HealthProbe { - const fn requires_lock_quorum(self) -> bool { - matches!(self, Self::ClusterWrite | Self::ClusterRead) - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum HealthReadinessSource { Node, @@ -116,7 +110,7 @@ pub(crate) fn health_check_state( peer_health_ready: bool, probe: HealthProbe, ) -> HealthCheckState { - let ready = storage_ready && iam_ready && peer_health_ready && (!probe.requires_lock_quorum() || lock_quorum_ready); + let ready = storage_ready && iam_ready && lock_quorum_ready && peer_health_ready; if probe == HealthProbe::Liveness { // Liveness always returns HTTP 200 (process is alive), but the `ready` @@ -495,6 +489,32 @@ mod tests { assert!(parts.payload.is_none()); } + #[test] + #[serial] + fn liveness_and_readiness_payloads_share_lock_quorum_readiness() { + with_var(rustfs_config::ENV_HEALTH_MINIMAL_RESPONSE_ENABLE, Some("false"), || { + let mut report = ready_report(); + report.readiness.lock_quorum_ready = false; + report.degraded_reasons.push(ReadinessDegradedReason::LockQuorumUnavailable); + + let liveness = + build_health_response_parts(Method::GET, HealthProbe::Liveness, Some(&report), "rustfs-endpoint", None, None); + let readiness = + build_health_response_parts(Method::GET, HealthProbe::Readiness, Some(&report), "rustfs-endpoint", None, None); + + assert_eq!(liveness.status_code, StatusCode::OK); + assert_eq!(readiness.status_code, StatusCode::SERVICE_UNAVAILABLE); + let liveness_payload = liveness.payload.expect("liveness GET should include payload"); + let readiness_payload = readiness.payload.expect("readiness GET should include payload"); + assert_eq!(liveness_payload["ready"], false); + assert_eq!(readiness_payload["ready"], false); + assert_eq!(liveness_payload["details"]["lock"]["ready"], false); + assert_eq!(readiness_payload["details"]["lock"]["ready"], false); + assert_eq!(liveness_payload["degradedReasons"], json!(["lock_quorum_unavailable"])); + assert_eq!(readiness_payload["degradedReasons"], json!(["lock_quorum_unavailable"])); + }); + } + #[test] fn object_stall_overlay_records_the_final_readiness_metrics() { let recorder = DebuggingRecorder::new(); diff --git a/rustfs/src/server/layer.rs b/rustfs/src/server/layer.rs index 1bb3e79c6..3267174e7 100644 --- a/rustfs/src/server/layer.rs +++ b/rustfs/src/server/layer.rs @@ -2683,48 +2683,50 @@ mod tests { #[tokio::test] #[serial] async fn public_health_endpoint_layer_handles_health_before_inner_service() { - async_with_vars([(rustfs_config::ENV_HEALTH_ENDPOINT_ENABLE, Some("true"))], async { - let inner = CountingHybridService::default(); - let calls = inner.calls(); - let mut service = public_health_layer().layer(inner); + async_with_vars( + [ + (rustfs_config::ENV_HEALTH_ENDPOINT_ENABLE, Some("true")), + (rustfs_config::ENV_HEALTH_MINIMAL_RESPONSE_ENABLE, Some("false")), + ], + async { + let inner = CountingHybridService::default(); + let calls = inner.calls(); + let mut service = public_health_layer().layer(inner); - let response = service - .call( - Request::builder() - .method(Method::GET) - .uri(HEALTH_PREFIX) - .header(http::header::HOST, "localhost:9000") - .body(Full::::from(Bytes::new())) - .expect("request"), - ) - .await - .expect("health response"); + let response = service + .call( + Request::builder() + .method(Method::GET) + .uri(HEALTH_PREFIX) + .header(http::header::HOST, "localhost:9000") + .body(Full::::from(Bytes::new())) + .expect("request"), + ) + .await + .expect("health response"); - assert_eq!(response.status(), StatusCode::OK); - assert_eq!(calls.load(Ordering::SeqCst), 0); - assert_eq!( - response - .headers() - .get(http::header::CONTENT_TYPE) - .and_then(|value| value.to_str().ok()), - Some("application/json") - ); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(calls.load(Ordering::SeqCst), 0); + assert_eq!( + response + .headers() + .get(http::header::CONTENT_TYPE) + .and_then(|value| value.to_str().ok()), + Some("application/json") + ); - let body = BodyExt::collect(response.into_body()).await.expect("body").to_bytes(); - let payload: serde_json::Value = - serde_json::from_slice(&body).expect("public liveness health response should be valid JSON"); - assert_eq!(payload["status"], "degraded"); - assert_eq!(payload["ready"], false); - assert_eq!( - payload["details"], - serde_json::json!({ - "storage": { "status": "disconnected", "ready": false }, - "iam": { "status": "disconnected", "ready": false }, - "lock": { "status": "connected", "ready": true }, - }) - ); - assert_eq!(payload["degradedReasons"], serde_json::json!(["storage_and_iam_unavailable"])); - }) + let body = BodyExt::collect(response.into_body()).await.expect("body").to_bytes(); + let payload: serde_json::Value = + serde_json::from_slice(&body).expect("public liveness health response should be valid JSON"); + assert!(matches!(payload["status"].as_str(), Some("ok" | "degraded"))); + assert!(payload["ready"].is_boolean()); + assert!(payload["details"].is_object()); + assert!(payload["details"]["storage"]["ready"].is_boolean()); + assert!(payload["details"]["iam"]["ready"].is_boolean()); + assert!(payload["details"]["lock"]["ready"].is_boolean()); + assert!(payload["degradedReasons"].is_array()); + }, + ) .await; }