mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-10 23:26:53 +00:00
fix(iam): load IAM bootstrap snapshot without namespace locks (#4363)
* fix(iam): load IAM bootstrap snapshot without namespace locks
IAM bootstrap (init_iam_sys -> load_all) read every config object with
default ObjectOptions (no_lock=false), so each read acquired a
distributed namespace read lock. Lock quorum is counted over cluster
nodes and unreachable peers are hard failures, so during a sequential
restart the very first read failed with
"Quorum not reached: required 2, achieved 0" and IAM could not come up
until enough peers' lock RPC surfaces converged - even when the storage
read quorum was already satisfiable (rustfs#4304).
Extend the startup contract from rustfs#4056 ("startup metadata I/O must
not require namespace locks") to the IAM bootstrap path:
- Introduce LoadMode {Locked, BootstrapNoLock} and plumb it through the
load_all chain (groups, users, policies, mapped policies and their
concurrent variants) down to the storage read options.
- load_all now performs all reads with no_lock=true; on-line
single-object loads via the Store trait keep locked semantics.
- Fail-closed behavior is unchanged: any loader error still aborts the
whole snapshot load.
Safety: config objects are atomic whole-object writes, so a lock-free
read only observes an old or a new value; staleness is bounded by the
existing periodic IAM reload. Listing (walk) never took namespace locks,
and maybe_schedule_lazy_rewrite stays a best-effort background task.
Verification:
- cargo test -p rustfs-iam --lib (153 passed, incl. new LoadMode tests)
- cargo clippy -p rustfs-iam --all-targets
- cargo check -p rustfs
- make pre-commit
Ref: rustfs#4304; tracking rustfs/backlog#884, rustfs/backlog#885
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(iam): sequential-restart regression test for lock-free bootstrap
Add an integration test reproducing the rustfs#4304 failure mode against
a real 4-disk temp-dir ECStore:
- Seed IAM group data in single-node mode, then flip the runtime into
distributed-erasure mode. new_ns_lock now builds a distributed lock
over the set's (empty) lock-client list, so every namespace-locked
read fails exactly like a sequential restart with unreachable peers
(lock quorum unavailable, storage read quorum healthy).
- Assert the locked load_group path fails in that state, while the
bulk snapshot load_all (no_lock plumbing from the previous commit)
succeeds, and the data survives intact once single-node mode is
restored.
Reverse-verified: temporarily switching load_all back to the locked
mode makes the test fail, so it genuinely guards the contract.
Verification:
- cargo test -p rustfs-iam --test iam_bootstrap_no_lock_test
- cargo test -p rustfs-iam --lib
Ref: rustfs#4304; tracking rustfs/backlog#886
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(iam): route test ECStore imports through ecstore_test_compat boundary
The new integration test imported rustfs_ecstore facade paths directly,
tripping three architecture migration rules. Move every ECStore import
behind crates/iam/tests/ecstore_test_compat/mod.rs (the sanctioned
test-compat pattern), and register that module as a reviewed test-only
global-facade boundary in check_architecture_migration_rules.sh: the
sequential-restart regression test needs api::global::update_erasure_type
to flip into distributed-erasure mode for lock-quorum fault injection.
Verification:
- ./scripts/check_architecture_migration_rules.sh
- cargo test -p rustfs-iam --test iam_bootstrap_no_lock_test
- make pre-commit
Co-Authored-By: heihutu <heihutu@gmail.com>
* feat(server): expose readiness blocking reason + rolling-restart runbook
Operators hitting the rustfs#4304 sequential cold start could not tell
from the outside why a node stayed unavailable. Three additions:
- The readiness gate's 503 now names the blocking dependency in both the
body ("Service not ready: waiting for storage_quorum") and a new
x-rustfs-readiness-pending header (storage_quorum | iam |
startup_finalization), derived from the current startup stage.
/health/ready already returned details + degradedReasons; this covers
the plain S3 requests that hit the gate.
- IAM bootstrap retry logs now carry an actionable `hint` field that
classifies the failure (storage read quorum vs lock quorum vs
uninitialized metadata) instead of only echoing the storage error.
- New docs/operations/rolling-restart.md runbook: correct rolling
restart procedure, sequential cold-start expectations (degraded ->
auto-recovery), readiness signal reference, and
RUSTFS_STARTUP_READINESS_MAX_WAIT_SECS guidance.
Verification:
- cargo test -p rustfs --lib -- hint_tests service_not_ready readiness_pending
- make pre-commit
Ref: rustfs#4304; tracking rustfs/backlog#887
Co-Authored-By: heihutu <heihutu@gmail.com>
* upgrade deps version and improve import
* feat(iam): notification-path cache refreshes read without namespace locks (#4368)
P3 step 1 of rustfs/backlog#884 (scoped down from full MinIO readConfig
alignment after review): cross-node notification handlers
(group/policy/policy-mapping/user) refresh the local IAM cache with
single-object reads that previously took distributed namespace read
locks. These refreshes are asynchronous, best-effort, and already
stale-tolerant (the periodic reload converges them), so a node-counted
lock quorum failure or lock RPC hiccup on a peer must not fail them —
the same rationale as the lock-free bootstrap load_all (rustfs#4304).
- Store trait: add load_user_no_lock / load_group_no_lock /
load_policy_doc_no_lock / load_mapped_policy_no_lock with defaults
forwarding to the locked variants, so existing implementations and
test mocks keep their behavior.
- ObjectStore overrides them via the existing LoadMode::BootstrapNoLock
plumbing. Deletions triggered by the handlers keep locked writes.
- manager.rs: the four *_notification_handler paths (8 call sites)
switch to the lock-free variants.
- Integration test: while the lock quorum is unavailable (DistErasure
with empty lockers), load_group_no_lock must succeed exactly where
the locked load_group fails.
Request-path loads (check_key, verify_temp_user_persistence) and admin
write-then-reload paths intentionally stay locked: load_user_identity
embeds expiry deletions, so those need the side-effect extraction
tracked in rustfs/backlog#884 before going lock-free.
Verification:
- cargo test -p rustfs-iam --lib (156 passed)
- cargo test -p rustfs-iam --test iam_bootstrap_no_lock_test
- make pre-commit
Ref: rustfs/backlog#884, rustfs#4304
Co-authored-by: heihutu <heihutu@gmail.com>
* fix(server): drop unused iam_bootstrap_failure_hint import in tests
The hint tests live in their own hint_tests module with a local import;
the stale re-import in mod tests failed clippy's -D warnings on the
Test and Lint CI variants.
Verification:
- cargo clippy -p rustfs --all-targets
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix
---------
Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -194,8 +194,22 @@ fn readiness_gate_blocks_path(path: &str, readiness: &GlobalReadiness) -> bool {
|
||||
type BoxError = Box<dyn std::error::Error + Send + Sync>;
|
||||
type BoxBody = http_body_util::combinators::UnsyncBoxBody<Bytes, BoxError>;
|
||||
|
||||
fn service_not_ready_response() -> Response<BoxBody> {
|
||||
let body: BoxBody = Full::new(Bytes::from_static(b"Service not ready"))
|
||||
/// Header exposing which startup dependency the readiness gate is waiting
|
||||
/// on, so operators can diagnose a 503 without shell access (rustfs#4304).
|
||||
const READINESS_PENDING_HEADER: &str = "x-rustfs-readiness-pending";
|
||||
|
||||
/// Maps the current startup stage to the dependency the gate is waiting on.
|
||||
fn readiness_pending_dependency(stage: rustfs_common::SystemStage) -> &'static str {
|
||||
match stage {
|
||||
rustfs_common::SystemStage::Booting => "storage_quorum",
|
||||
rustfs_common::SystemStage::StorageReady => "iam",
|
||||
rustfs_common::SystemStage::IamReady | rustfs_common::SystemStage::FullReady => "startup_finalization",
|
||||
}
|
||||
}
|
||||
|
||||
fn service_not_ready_response(stage: rustfs_common::SystemStage) -> Response<BoxBody> {
|
||||
let pending = readiness_pending_dependency(stage);
|
||||
let body: BoxBody = Full::new(Bytes::from(format!("Service not ready: waiting for {pending}")))
|
||||
.map_err(|e| -> BoxError { Box::new(e) })
|
||||
.boxed_unsync();
|
||||
|
||||
@@ -211,6 +225,9 @@ fn service_not_ready_response() -> Response<BoxBody> {
|
||||
.headers_mut()
|
||||
.insert(http::header::CACHE_CONTROL, HeaderValue::from_static("no-store"));
|
||||
response
|
||||
.headers_mut()
|
||||
.insert(READINESS_PENDING_HEADER, HeaderValue::from_static(pending));
|
||||
response
|
||||
}
|
||||
|
||||
impl<S, B> Service<HttpRequest<Incoming>> for ReadinessGateService<S>
|
||||
@@ -236,7 +253,7 @@ where
|
||||
let path = req.uri().path();
|
||||
debug!("ReadinessGateService: Received request for path: {}", path);
|
||||
if readiness_gate_blocks_path(path, &readiness) {
|
||||
return Ok(service_not_ready_response());
|
||||
return Ok(service_not_ready_response(readiness.current_stage()));
|
||||
}
|
||||
let resp = inner.call(req).await?;
|
||||
// System is ready, forward to the actual S3/RPC handlers
|
||||
@@ -1205,7 +1222,7 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn service_not_ready_response_preserves_observable_contract() {
|
||||
let response = service_not_ready_response();
|
||||
let response = service_not_ready_response(rustfs_common::SystemStage::Booting);
|
||||
|
||||
assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE);
|
||||
assert_eq!(
|
||||
@@ -1229,11 +1246,27 @@ mod tests {
|
||||
.and_then(|value| value.to_str().ok()),
|
||||
Some("text/plain; charset=utf-8")
|
||||
);
|
||||
assert_eq!(
|
||||
response
|
||||
.headers()
|
||||
.get(READINESS_PENDING_HEADER)
|
||||
.and_then(|value| value.to_str().ok()),
|
||||
Some("storage_quorum")
|
||||
);
|
||||
let body = match response.into_body().collect().await {
|
||||
Ok(body) => body.to_bytes(),
|
||||
Err(err) => panic!("not-ready body should collect: {err}"),
|
||||
};
|
||||
assert_eq!(body, Bytes::from_static(b"Service not ready"));
|
||||
assert_eq!(body, Bytes::from_static(b"Service not ready: waiting for storage_quorum"));
|
||||
}
|
||||
|
||||
/// rustfs#4304: operators must be able to tell from the 503 alone which
|
||||
/// startup dependency the node is blocked on.
|
||||
#[test]
|
||||
fn readiness_pending_dependency_maps_every_stage() {
|
||||
assert_eq!(readiness_pending_dependency(rustfs_common::SystemStage::Booting), "storage_quorum");
|
||||
assert_eq!(readiness_pending_dependency(rustfs_common::SystemStage::StorageReady), "iam");
|
||||
assert_eq!(readiness_pending_dependency(rustfs_common::SystemStage::IamReady), "startup_finalization");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -168,6 +168,7 @@ async fn run_iam_recovery_loop<InitFn, FinalizeFn>(
|
||||
}
|
||||
Err(err) => {
|
||||
let next_interval = compute_backoff_interval(attempts + 1, initial_interval, max_interval);
|
||||
let hint = iam_bootstrap_failure_hint(&err.to_string());
|
||||
if attempts >= IAM_RETRY_ESCALATION_THRESHOLD {
|
||||
error!(
|
||||
event = EVENT_IAM_BOOTSTRAP_RETRY_FAILED,
|
||||
@@ -177,6 +178,7 @@ async fn run_iam_recovery_loop<InitFn, FinalizeFn>(
|
||||
next_retry_secs = next_interval.as_secs(),
|
||||
degraded_duration_secs = degraded_since.elapsed().as_secs(),
|
||||
error = %err,
|
||||
hint,
|
||||
"IAM bootstrap retry failed; service remains degraded"
|
||||
);
|
||||
} else {
|
||||
@@ -187,6 +189,7 @@ async fn run_iam_recovery_loop<InitFn, FinalizeFn>(
|
||||
attempts,
|
||||
next_retry_secs = next_interval.as_secs(),
|
||||
error = %err,
|
||||
hint,
|
||||
"IAM bootstrap retry failed; service remains degraded"
|
||||
);
|
||||
}
|
||||
@@ -235,6 +238,23 @@ async fn run_iam_recovery_loop<InitFn, FinalizeFn>(
|
||||
}
|
||||
}
|
||||
|
||||
/// Classifies an IAM bootstrap failure into an actionable operator hint, so
|
||||
/// the degraded-retry logs explain *what to do* instead of only echoing the
|
||||
/// storage error (rustfs#4304: sequential cold starts left operators
|
||||
/// guessing why the node stayed degraded).
|
||||
fn iam_bootstrap_failure_hint(err_text: &str) -> &'static str {
|
||||
let lower = err_text.to_lowercase();
|
||||
if lower.contains("quorum") && lower.contains("lock") {
|
||||
"distributed lock quorum unavailable; waiting for peer nodes' lock RPC endpoints to come online"
|
||||
} else if lower.contains("read quorum") || lower.contains("erasure") || lower.contains("quorum") {
|
||||
"storage read quorum not met yet; waiting for enough cluster nodes/disks to come online"
|
||||
} else if lower.contains("not ready") || lower.contains("not found") {
|
||||
"storage metadata not initialized yet; retrying automatically"
|
||||
} else {
|
||||
"retrying automatically; check storage and peer connectivity if this persists"
|
||||
}
|
||||
}
|
||||
|
||||
fn initial_retry_interval() -> Duration {
|
||||
// Only honor the test override in debug builds to prevent accidental
|
||||
// production use via environment configuration.
|
||||
@@ -376,12 +396,40 @@ pub(crate) async fn init_iam_runtime(
|
||||
bootstrap_or_defer_iam_init_with_startup_kms(store, readiness, Some(state_manager), Some(ctx)).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod hint_tests {
|
||||
use super::iam_bootstrap_failure_hint;
|
||||
|
||||
/// rustfs#4304: the degraded-retry log must tell the operator what the
|
||||
/// node is waiting on, for each of the observed failure shapes.
|
||||
#[test]
|
||||
fn classifies_observed_bootstrap_failures() {
|
||||
assert!(
|
||||
iam_bootstrap_failure_hint(
|
||||
"load group failed: io error: Failed to acquire read lock: Quorum not reached: required 2, achieved 0"
|
||||
)
|
||||
.contains("lock quorum"),
|
||||
"lock-quorum failures must point at peer lock RPC endpoints"
|
||||
);
|
||||
assert!(
|
||||
iam_bootstrap_failure_hint("load group failed: erasure read quorum").contains("storage read quorum"),
|
||||
"storage-quorum failures must point at missing nodes/disks"
|
||||
);
|
||||
assert!(
|
||||
iam_bootstrap_failure_hint("Storage metadata not ready: probe object not found").contains("not initialized"),
|
||||
"missing-metadata failures must say the store is still initializing"
|
||||
);
|
||||
assert!(
|
||||
iam_bootstrap_failure_hint("some opaque failure").contains("retrying automatically"),
|
||||
"unknown failures must still promise the automatic retry"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{
|
||||
IAM_RETRY_ESCALATION_THRESHOLD, IAM_RETRY_INITIAL_INTERVAL, IAM_RETRY_MAX_INTERVAL, IamBootstrapDisposition,
|
||||
compute_backoff_interval, publish_ready_for_iam_bootstrap_with, run_iam_recovery_loop,
|
||||
};
|
||||
use super::*;
|
||||
use super::{IAM_RETRY_ESCALATION_THRESHOLD, IAM_RETRY_INITIAL_INTERVAL, IAM_RETRY_MAX_INTERVAL, compute_backoff_interval};
|
||||
use rustfs_common::{GlobalReadiness, SystemStage};
|
||||
use std::io::Error;
|
||||
use std::sync::{
|
||||
|
||||
Reference in New Issue
Block a user