From b49057c8e3e0b1a0ed828912806ee67b49e404c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Sun, 14 Jun 2026 21:24:07 +0800 Subject: [PATCH] refactor: centralize startup readiness bootstrap (#3446) --- docs/architecture/migration-progress.md | 84 +++++++++++++++---------- rustfs/src/embedded.rs | 16 +++-- rustfs/src/main.rs | 6 +- rustfs/src/startup_iam.rs | 76 +++++++++++++++++++++- 4 files changed, 134 insertions(+), 48 deletions(-) diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 8b98aa8ec..210477ad7 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,17 +5,18 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Current Context - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) -- Branch: `overtrue/arch-app-usecase-object-store-fallback-cleanup` -- Baseline: `origin/main` at `fc894c9b569229101f859a6c4097eb64d3f86f5c` -- PR type for this branch: `consumer-migration` -- Runtime behavior changes: no external behavior change expected; app usecase - object-store lookups share the same explicit-context resolver and keep the - existing legacy global object-layer fallback when no usecase context exists. -- Rust code changes: add an explicit AppContext object-store resolver helper, - migrate admin, bucket, multipart, and object usecases to it, and remove a - stale ECStore tier comment that referenced the old direct accessor. +- Branch: `overtrue/arch-startup-readiness-bootstrap` +- Baseline: `origin/main` at `8d23ce06c6dba11f50f656af4c30b63036cef92f` +- PR type for this branch: `pure-move` +- Runtime behavior changes: no external behavior change expected; inline IAM + bootstrap still publishes runtime readiness after runtime dependencies are + ready, and deferred IAM bootstrap still leaves readiness publication to the + recovery loop. +- Rust code changes: centralize the IAM bootstrap readiness publication decision + in `startup_iam`, use it from binary and embedded startup, and add + wrapper-level coverage for inline, deferred, and failure paths. - CI/script changes: none. -- Docs changes: record `CTX-011` compatibility fallback cleanup scope and +- Docs changes: record `R-009` startup readiness bootstrap wrapper progress and verification. ## Phase 0 Tasks @@ -548,48 +549,65 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Verification: focused config reload and shutdown tests, compile checks, formatting, diff hygiene, and Rust risk scan. +## Phase 9 Startup Bootstrap Tasks + +- [x] `R-009` Centralize startup IAM readiness publication bootstrap. + - Do: move the ReadyInline/Deferred readiness publication decision behind + `startup_iam::publish_ready_for_iam_bootstrap` and use it from binary and + embedded startup. + - Acceptance: inline IAM bootstrap still waits for runtime readiness and + updates service state, deferred IAM bootstrap does not publish readiness + from main or embedded startup, and embedded runtime readiness failures still + trigger embedded shutdown error mapping. + - Must preserve: startup ordering, IAM degraded recovery ownership, + `IamReady`/`FullReady` publication semantics, and embedded shutdown + behavior. + - Verification: focused startup IAM tests, binary/lib compile checks, + formatting, migration guards, Rust risk scan, and pre-commit quality gate. + ## Next PRs -1. `consumer-migration`: remove the final old global object-layer accessor - compatibility path once downstream/public API cleanup is accepted. -2. `pure-move`: start `R-009` boot wrapper with the IAM degraded readiness - contract covered. +1. `pure-move`: continue extracting startup boot wrappers in larger slices while + preserving startup order and readiness ownership. +2. `ci-gate`: finish `G-006` public re-export and storage trait coverage checks + before the remaining cleanup slices. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | passed | Single consumer-migration slice; app usecases delegate object-store fallback to the AppContext compatibility helper instead of duplicating direct global accessor calls. | -| Migration preservation | passed | Admin, bucket, multipart, and object usecases keep injected context precedence and explicit no-context legacy global fallback behavior. | -| Testing/verification | passed | Formatting, compile checks, migration/layer guards, Rust risk scan, branch freshness check, and full `make pre-commit` passed. | +| Quality/architecture | passed | Pure-move slice centralizes IAM bootstrap readiness publication without moving runtime readiness collection or startup dependency checks. | +| Migration preservation | passed | Main and embedded keep ReadyInline publication behavior, Deferred remains recovery-loop owned, and embedded readiness errors still map through shutdown. | +| Testing/verification | passed | Focused startup IAM tests, compile checks, formatting, migration/layer guards, Rust risk scan, branch freshness check, and full `make pre-commit` passed. | ## Verification Notes -Passed on `fc894c9b569229101f859a6c4097eb64d3f86f5c`: +Passed on `8d23ce06c6dba11f50f656af4c30b63036cef92f`: - `cargo fmt --all --check`. -- `cargo check -p rustfs-ecstore`. +- `cargo test -p rustfs startup_iam --no-fail-fast`. +- `cargo check -p rustfs --bin rustfs`. - `cargo check -p rustfs --lib`. - `git diff --check`. - `./scripts/check_architecture_migration_rules.sh`. - `./scripts/check_layer_dependencies.sh`. -- `git rev-list --left-right --count HEAD...origin/main` returned `1 0` - after rebase. -- Rust risk scan for changed Rust files: full-file matches were existing tests, - existing numeric casts, existing string error signatures, and existing relaxed - counters; added-line scan returned no unwrap/expect, numeric cast, string - error, boxed error, print macro, or relaxed-ordering match. -- `make pre-commit`: all checks passed, including nextest with 5961 passed - and 111 skipped, plus doctests. +- `git rev-list --left-right --count HEAD...origin/main` returned `0 0`. +- Rust risk scan for changed Rust files: full-file matches were existing docs + examples, existing startup error output, existing test expectations, and the + existing boxed recovery future; added-line scan returned no unwrap/expect, + numeric cast, string error, boxed error, print macro, relaxed-ordering, or + unsafe match. +- `make pre-commit`: all checks passed, including nextest with 5966 passed and + 111 skipped, plus doctests. Notes: -- This slice consolidates app usecase object-store fallback without changing - request behavior. -- The old global accessor remains as the resolver fallback and public - compatibility re-export for a later cleanup slice. +- This slice centralizes startup IAM readiness publication without changing the + runtime readiness checks themselves. +- Deferred IAM bootstrap readiness remains owned by the recovery loop. ## Handoff Notes -- CTX-011 is complete. -- The global fallback definition and re-export remain for a later cleanup slice. +- R-009 is complete. +- Next startup slices can be larger pure moves, but must keep startup ordering + and readiness ownership explicit in tests. diff --git a/rustfs/src/embedded.rs b/rustfs/src/embedded.rs index dcce405ec..c47b24823 100644 --- a/rustfs/src/embedded.rs +++ b/rustfs/src/embedded.rs @@ -52,7 +52,7 @@ use crate::server::{ ShutdownHandle, init_event_notifier, shutdown_event_notifier, start_audit_system, start_http_server, stop_audit_system, }; use crate::startup_fs_guard::enforce_unsupported_fs_policy; -use crate::startup_iam::{IamBootstrapDisposition, bootstrap_or_defer_iam_init}; +use crate::startup_iam::{bootstrap_or_defer_iam_init, publish_ready_for_iam_bootstrap}; use rustfs_common::{GlobalReadiness, SystemStage, set_global_addr}; use rustfs_credentials::init_global_action_credentials; use rustfs_ecstore::store::init_lock_clients; @@ -494,14 +494,12 @@ impl RustFSServerBuilder { ); } - if iam_bootstrap == IamBootstrapDisposition::ReadyInline { - crate::server::publish_ready_when_runtime_ready(readiness.as_ref(), None) - .await - .map_err(|e| { - shutdown_embedded_server(); - ServerError::Init(format!("runtime readiness: {e}")) - })?; - } + publish_ready_for_iam_bootstrap(iam_bootstrap, readiness.as_ref(), None) + .await + .map_err(|e| { + shutdown_embedded_server(); + ServerError::Init(format!("runtime readiness: {e}")) + })?; rustfs_common::set_global_init_time_now().await; diff --git a/rustfs/src/main.rs b/rustfs/src/main.rs index 9bd640871..90cbc2d93 100644 --- a/rustfs/src/main.rs +++ b/rustfs/src/main.rs @@ -34,7 +34,7 @@ use rustfs::server::{ start_audit_system, start_http_server, stop_audit_system, wait_for_shutdown, }; use rustfs::startup_fs_guard::enforce_unsupported_fs_policy; -use rustfs::startup_iam::{IamBootstrapDisposition, bootstrap_or_defer_iam_init}; +use rustfs::startup_iam::{bootstrap_or_defer_iam_init, publish_ready_for_iam_bootstrap}; use rustfs_common::{GlobalReadiness, SystemStage, set_global_addr}; use rustfs_credentials::init_global_action_credentials; use rustfs_ecstore::store::init_lock_clients; @@ -968,9 +968,7 @@ async fn run(config: rustfs::config::Config) -> Result<()> { iam_bootstrap = ?iam_bootstrap, "RustFS server ready" ); - if iam_bootstrap == IamBootstrapDisposition::ReadyInline { - rustfs::server::publish_ready_when_runtime_ready(readiness.as_ref(), Some(state_manager.as_ref())).await?; - } + publish_ready_for_iam_bootstrap(iam_bootstrap, readiness.as_ref(), Some(state_manager.as_ref())).await?; // Set the global RustFS initialization time to now rustfs_common::set_global_init_time_now().await; diff --git a/rustfs/src/startup_iam.rs b/rustfs/src/startup_iam.rs index f46e5d631..6b1c83e79 100644 --- a/rustfs/src/startup_iam.rs +++ b/rustfs/src/startup_iam.rs @@ -44,6 +44,33 @@ pub enum IamBootstrapDisposition { Deferred, } +pub async fn publish_ready_for_iam_bootstrap( + disposition: IamBootstrapDisposition, + readiness: &GlobalReadiness, + state_manager: Option<&ServiceStateManager>, +) -> Result { + publish_ready_for_iam_bootstrap_with(disposition, || async move { + publish_ready_when_runtime_ready(readiness, state_manager).await + }) + .await +} + +async fn publish_ready_for_iam_bootstrap_with( + disposition: IamBootstrapDisposition, + publish_ready: PublishFn, +) -> Result +where + PublishFn: FnOnce() -> PublishFuture, + PublishFuture: Future>, +{ + if disposition == IamBootstrapDisposition::ReadyInline { + publish_ready().await?; + return Ok(true); + } + + Ok(false) +} + fn init_app_context_if_needed(store: Arc, kms_interface: Arc) -> bool { if get_global_app_context().is_some() { return false; @@ -341,8 +368,8 @@ pub async fn bootstrap_or_defer_iam_init( #[cfg(test)] mod tests { use super::{ - IAM_RETRY_ESCALATION_THRESHOLD, IAM_RETRY_INITIAL_INTERVAL, IAM_RETRY_MAX_INTERVAL, compute_backoff_interval, - run_iam_recovery_loop, + 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 rustfs_common::{GlobalReadiness, SystemStage}; use std::io::Error; @@ -377,6 +404,51 @@ mod tests { assert_eq!(compute_backoff_interval(100, initial, max), Duration::from_secs(30)); } + #[tokio::test] + async fn ready_inline_bootstrap_publishes_runtime_readiness() { + let publish_calls = Arc::new(AtomicUsize::new(0)); + let publish_calls_for_assert = publish_calls.clone(); + + let published = publish_ready_for_iam_bootstrap_with(IamBootstrapDisposition::ReadyInline, move || async move { + publish_calls.fetch_add(1, Ordering::SeqCst); + Ok(()) + }) + .await; + + assert!(published.is_ok(), "ready inline publication should succeed"); + let published = published.unwrap_or(false); + assert!(published); + assert_eq!(publish_calls_for_assert.load(Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn deferred_bootstrap_skips_runtime_readiness_publication() { + let publish_calls = Arc::new(AtomicUsize::new(0)); + let publish_calls_for_assert = publish_calls.clone(); + + let published = publish_ready_for_iam_bootstrap_with(IamBootstrapDisposition::Deferred, move || async move { + publish_calls.fetch_add(1, Ordering::SeqCst); + Ok(()) + }) + .await; + + assert!(published.is_ok(), "deferred publication should be a no-op"); + let published = published.unwrap_or(true); + assert!(!published); + assert_eq!(publish_calls_for_assert.load(Ordering::SeqCst), 0); + } + + #[tokio::test] + async fn ready_inline_bootstrap_propagates_runtime_readiness_failure() { + let err = publish_ready_for_iam_bootstrap_with(IamBootstrapDisposition::ReadyInline, || async { + Err(Error::other("runtime readiness failed")) + }) + .await + .expect_err("ready inline publication failure should be returned"); + + assert_eq!(err.to_string(), "runtime readiness failed"); + } + #[tokio::test(start_paused = true)] async fn recovery_loop_retries_finalize_until_success() { let init_calls = Arc::new(AtomicUsize::new(0));