mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-29 00:17:11 +00:00
fix(health): reflect node readiness in /health response body (#6520)
The /health endpoint (liveness) was returning a hardcoded `ready: true` in its response body regardless of actual node readiness state. This caused a semantic contradiction with /health/ready (readiness), which correctly reported readiness based on storage, IAM, lock quorum, and peer health. This led to confusing behavior in Kubernetes deployments where: - /health returned 200 with `ready: true` (liveness) - /health/ready returned 503 (readiness) - Pods remained Running but were removed from Service endpoints Changes: - readiness_source_for_probe(Liveness) now returns Node readiness source - health_check_state() for Liveness reflects actual readiness in body while keeping HTTP 200 status (process is alive) - build_health_response_parts() for Liveness now includes dependency details and degradedReasons when readiness report is available This ensures the `ready` field in /health body is truthful while maintaining backward compatibility for liveness probe behavior. Refs: rustfs/backlog#2011 Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -113,8 +113,8 @@ mod tests {
|
|||||||
fn test_liveness_state_iam_not_ready() {
|
fn test_liveness_state_iam_not_ready() {
|
||||||
let state = health_check_state(true, false, true, true, HealthProbe::Liveness);
|
let state = health_check_state(true, false, true, true, HealthProbe::Liveness);
|
||||||
assert_eq!(state.status_code, StatusCode::OK);
|
assert_eq!(state.status_code, StatusCode::OK);
|
||||||
assert_eq!(state.status, "ok");
|
assert_eq!(state.status, "degraded");
|
||||||
assert!(state.ready);
|
assert!(!state.ready);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -172,7 +172,7 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_readiness_probe_uses_node_collector_only() {
|
fn test_readiness_probe_uses_node_collector_only() {
|
||||||
assert_eq!(readiness_source_for_probe(HealthProbe::Readiness), Some(HealthReadinessSource::Node));
|
assert_eq!(readiness_source_for_probe(HealthProbe::Readiness), Some(HealthReadinessSource::Node));
|
||||||
assert_eq!(readiness_source_for_probe(HealthProbe::Liveness), None);
|
assert_eq!(readiness_source_for_probe(HealthProbe::Liveness), Some(HealthReadinessSource::Node));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -256,12 +256,15 @@ mod tests {
|
|||||||
None,
|
None,
|
||||||
None,
|
None,
|
||||||
);
|
);
|
||||||
|
// Liveness HTTP status remains 200 (process is alive).
|
||||||
assert_eq!(parts.status_code, StatusCode::OK);
|
assert_eq!(parts.status_code, StatusCode::OK);
|
||||||
let payload = parts.payload.expect("GET should include payload");
|
let payload = parts.payload.expect("GET should include payload");
|
||||||
assert_eq!(payload["status"], "ok");
|
// But `ready` now reflects actual readiness state.
|
||||||
assert_eq!(payload["ready"], true);
|
assert_eq!(payload["status"], "degraded");
|
||||||
assert!(payload.get("details").is_none());
|
assert_eq!(payload["ready"], false);
|
||||||
assert!(payload.get("degradedReasons").is_none());
|
// Dependency details are included when readiness report is present.
|
||||||
|
assert!(payload.get("details").is_some());
|
||||||
|
assert!(payload.get("degradedReasons").is_some());
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -103,8 +103,7 @@ fn apply_object_traffic_snapshot(report: &mut DependencyReadinessReport, snapsho
|
|||||||
|
|
||||||
pub(crate) fn readiness_source_for_probe(probe: HealthProbe) -> Option<HealthReadinessSource> {
|
pub(crate) fn readiness_source_for_probe(probe: HealthProbe) -> Option<HealthReadinessSource> {
|
||||||
match probe {
|
match probe {
|
||||||
HealthProbe::Liveness => None,
|
HealthProbe::Liveness | HealthProbe::Readiness => Some(HealthReadinessSource::Node),
|
||||||
HealthProbe::Readiness => Some(HealthReadinessSource::Node),
|
|
||||||
HealthProbe::ClusterWrite => Some(HealthReadinessSource::ClusterWrite),
|
HealthProbe::ClusterWrite => Some(HealthReadinessSource::ClusterWrite),
|
||||||
HealthProbe::ClusterRead => Some(HealthReadinessSource::ClusterRead),
|
HealthProbe::ClusterRead => Some(HealthReadinessSource::ClusterRead),
|
||||||
}
|
}
|
||||||
@@ -117,15 +116,19 @@ pub(crate) fn health_check_state(
|
|||||||
peer_health_ready: bool,
|
peer_health_ready: bool,
|
||||||
probe: HealthProbe,
|
probe: HealthProbe,
|
||||||
) -> HealthCheckState {
|
) -> HealthCheckState {
|
||||||
|
let ready = storage_ready && iam_ready && peer_health_ready && (!probe.requires_lock_quorum() || lock_quorum_ready);
|
||||||
|
|
||||||
if probe == HealthProbe::Liveness {
|
if probe == HealthProbe::Liveness {
|
||||||
|
// Liveness always returns HTTP 200 (process is alive), but the `ready`
|
||||||
|
// field now reflects actual node readiness so that callers who inspect
|
||||||
|
// the body get a truthful signal instead of a hardcoded `true`.
|
||||||
return HealthCheckState {
|
return HealthCheckState {
|
||||||
status_code: StatusCode::OK,
|
status_code: StatusCode::OK,
|
||||||
status: "ok",
|
status: if ready { "ok" } else { "degraded" },
|
||||||
ready: true,
|
ready,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
let ready = storage_ready && iam_ready && peer_health_ready && (!probe.requires_lock_quorum() || lock_quorum_ready);
|
|
||||||
let status = if ready { "ok" } else { "degraded" };
|
let status = if ready { "ok" } else { "degraded" };
|
||||||
|
|
||||||
let status_code = if ready {
|
let status_code = if ready {
|
||||||
@@ -287,7 +290,7 @@ pub(crate) fn build_health_response_parts(
|
|||||||
) -> HealthResponseParts {
|
) -> HealthResponseParts {
|
||||||
let (storage_ready, iam_ready, lock_quorum_ready, mut health, mut degraded_reasons, include_dependency_details) =
|
let (storage_ready, iam_ready, lock_quorum_ready, mut health, mut degraded_reasons, include_dependency_details) =
|
||||||
match (probe, readiness_report) {
|
match (probe, readiness_report) {
|
||||||
(probe @ (HealthProbe::Readiness | HealthProbe::ClusterWrite | HealthProbe::ClusterRead), Some(readiness_report)) => {
|
(probe, Some(readiness_report)) => {
|
||||||
let storage_ready = readiness_report.readiness.storage_ready;
|
let storage_ready = readiness_report.readiness.storage_ready;
|
||||||
let iam_ready = readiness_report.readiness.iam_ready;
|
let iam_ready = readiness_report.readiness.iam_ready;
|
||||||
let lock_quorum_ready = readiness_report.readiness.lock_quorum_ready;
|
let lock_quorum_ready = readiness_report.readiness.lock_quorum_ready;
|
||||||
@@ -313,7 +316,7 @@ pub(crate) fn build_health_response_parts(
|
|||||||
vec![ReadinessDegradedReason::StorageIamAndLockUnavailable],
|
vec![ReadinessDegradedReason::StorageIamAndLockUnavailable],
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
(HealthProbe::Liveness, _) => (
|
(HealthProbe::Liveness, None) => (
|
||||||
false,
|
false,
|
||||||
false,
|
false,
|
||||||
false,
|
false,
|
||||||
|
|||||||
Reference in New Issue
Block a user