diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 76410933e..5a10bc508 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,18 +5,17 @@ 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-startup-service-bootstrap` -- Baseline: `origin/main` at `b49057c8e3e0b1a0ed828912806ee67b49e404c4` +- Branch: `overtrue/arch-startup-subsystems-bootstrap` +- Baseline: `origin/main` at `949939137066435e590c8bd0f029ed1500e0e1cd` - PR type for this branch: `pure-move` -- Runtime behavior changes: no external behavior change expected; binary startup - still treats notification initialization failure as fatal, while embedded - startup still treats audit and notification failures as non-fatal warnings. -- Rust code changes: centralize event notifier, audit startup, and notification - system bootstrap in `startup_services` helpers with caller-owned - logging/error policy, then use those helpers from binary and embedded +- Runtime behavior changes: no external behavior change expected; FTP, FTPS, + WebDAV, and SFTP startup still honors compile features and env-driven + enable/disable behavior, startup failure mapping, and shutdown handle flow. +- Rust code changes: centralize protocol sidecar bootstrap in + `startup_protocols::init_protocol_shutdown_senders` and use it from binary startup. - CI/script changes: none. -- Docs changes: record `R-010` startup service bootstrap wrapper progress and +- Docs changes: record `R-011` startup protocol bootstrap wrapper progress and verification. ## Phase 0 Tasks @@ -578,6 +577,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Verification: focused startup service tests, binary/lib compile checks, formatting, migration guards, Rust risk scan, and pre-commit quality gate. +- [x] `R-011` Centralize startup protocol sidecar bootstrap. + - Do: move FTP, FTPS, WebDAV, and SFTP startup orchestration behind + `startup_protocols::init_protocol_shutdown_senders`. + - Acceptance: feature-gated protocols still return `None` when not compiled + or enabled, started/disabled/failure logging preserves protocol and state + fields, and startup failures still abort binary startup with the same + `Error::other` mapping. + - Must preserve: protocol feature gates, env-driven enable/disable behavior, + startup log event/state/protocol values, shutdown handle ownership, and + existing shutdown ordering. + - Verification: focused startup protocol tests, binary/lib compile checks, + formatting, migration guards, Rust risk scan, and pre-commit quality gate. + ## Next PRs 1. `pure-move`: continue extracting startup boot wrappers in larger slices while @@ -589,37 +601,39 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| Quality/architecture | passed | Pure-move slice centralizes optional startup service bootstrap while keeping caller-specific logging and error policy at the binary and embedded boundaries. | -| Migration preservation | passed | Binary keeps notification init fatal, embedded keeps audit and notification failures warn-and-continue, and event notifier still starts before audit. | -| Testing/verification | passed | Focused startup service tests, compile checks, formatting, migration/layer guards, Rust risk scan, branch freshness check, and full `make pre-commit` passed. | +| Quality/architecture | passed | Pure-move slice centralizes protocol sidecar startup while keeping the binary boundary in charge of shutdown wiring. | +| Migration preservation | passed | Protocol feature gates, env-driven disabled handling, startup log event/state/protocol fields, and `Error::other` failure mapping are preserved. | +| Testing/verification | passed | Focused startup protocol tests, compile checks, formatting, migration/layer guards, Rust risk scan, branch freshness check, and full `make pre-commit` passed. | ## Verification Notes -Passed on `b49057c8e3e0b1a0ed828912806ee67b49e404c4`: +Passed on `949939137066435e590c8bd0f029ed1500e0e1cd`: -- `cargo test -p rustfs startup_services --no-fail-fast`. -- `cargo check -p rustfs --bin rustfs`. - `cargo check -p rustfs --lib`. +- `cargo check -p rustfs --bin rustfs`. +- `cargo test -p rustfs startup_protocols --no-fail-fast`. - `cargo fmt --all --check`. - `git diff --check`. - `./scripts/check_architecture_migration_rules.sh`. - `./scripts/check_layer_dependencies.sh`. - `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, and existing startup imports; added - lines introduced 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 5969 passed and +- Rust risk scan for changed Rust files: the only added-line match is the + `Box` type required by existing protocol init + signatures; full-file matches were existing docs examples and existing binary + startup error output. +- `make pre-commit`: all checks passed, including nextest with 5971 passed and 111 skipped, plus doctests. Notes: -- This slice centralizes startup optional service bootstrap without moving - logging or error-policy ownership out of binary and embedded startup. -- Binary and embedded keep their existing notification failure semantics. +- This slice centralizes startup protocol sidecar bootstrap without changing + startup ordering or shutdown handle ownership. +- Protocol logging keeps the existing event, subsystem, protocol, and state + values. ## Handoff Notes -- R-010 is complete. +- R-011 is complete. - Next startup slices can be larger pure moves, but must keep startup ordering, - fatal/non-fatal boundaries, and readiness ownership explicit in tests. + fatal/non-fatal boundaries, shutdown ownership, and readiness ownership + explicit in tests. diff --git a/rustfs/src/lib.rs b/rustfs/src/lib.rs index b6c1ae9b9..a6e41bef9 100644 --- a/rustfs/src/lib.rs +++ b/rustfs/src/lib.rs @@ -69,6 +69,7 @@ pub mod protocols; pub mod server; pub mod startup_fs_guard; pub mod startup_iam; +pub mod startup_protocols; pub mod startup_services; pub mod storage; pub(crate) mod table_catalog; diff --git a/rustfs/src/main.rs b/rustfs/src/main.rs index 2c8436d26..0b5ca339f 100644 --- a/rustfs/src/main.rs +++ b/rustfs/src/main.rs @@ -17,15 +17,6 @@ use rustfs::init::{ add_bucket_notification_configuration, init_buffer_profile_system, init_kms_system, init_update_check, print_server_info, }; -#[cfg(feature = "ftps")] -use rustfs::init::{init_ftp_system, init_ftps_system}; - -#[cfg(feature = "webdav")] -use rustfs::init::init_webdav_system; - -#[cfg(feature = "sftp")] -use rustfs::init::init_sftp_system; - use futures_util::future::join_all; use rustfs::capacity::capacity_integration::init_capacity_management; use rustfs::license::{init_license, license_status}; @@ -35,6 +26,7 @@ use rustfs::server::{ }; use rustfs::startup_fs_guard::enforce_unsupported_fs_policy; use rustfs::startup_iam::{bootstrap_or_defer_iam_init, publish_ready_for_iam_bootstrap}; +use rustfs::startup_protocols::{ProtocolShutdownSenders, init_protocol_shutdown_senders}; use rustfs_common::{GlobalReadiness, SystemStage, set_global_addr}; use rustfs_credentials::init_global_action_credentials; use rustfs_ecstore::store::init_lock_clients; @@ -606,173 +598,7 @@ async fn run(config: rustfs::config::Config) -> Result<()> { // Initialize KMS system if enabled init_kms_system(&config).await?; - // Initialize FTP system if enabled - #[cfg(feature = "ftps")] - let ftp_shutdown_tx = match init_ftp_system().await { - Ok(Some(tx)) => { - debug!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "ftp", - state = "started", - "Protocol runtime started" - ); - Some(tx) - } - Ok(None) => { - info!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "ftp", - state = "disabled", - "Protocol runtime disabled" - ); - None - } - Err(e) => { - error!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "ftp", - state = "initialization_failed", - error = %e, - "Protocol runtime initialization failed" - ); - return Err(Error::other(e)); - } - }; - - #[cfg(not(feature = "ftps"))] - let ftp_shutdown_tx: Option = None; - - // Initialize FTPS system if enabled - #[cfg(feature = "ftps")] - let ftps_shutdown_tx = match init_ftps_system().await { - Ok(Some(tx)) => { - debug!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "ftps", - state = "started", - "Protocol runtime started" - ); - Some(tx) - } - Ok(None) => { - info!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "ftps", - state = "disabled", - "Protocol runtime disabled" - ); - None - } - Err(e) => { - error!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "ftps", - state = "initialization_failed", - error = %e, - "Protocol runtime initialization failed" - ); - return Err(Error::other(e)); - } - }; - - #[cfg(not(feature = "ftps"))] - let ftps_shutdown_tx: Option = None; - - // Initialize WebDAV system if enabled - #[cfg(feature = "webdav")] - let webdav_shutdown_tx = match init_webdav_system().await { - Ok(Some(tx)) => { - debug!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "webdav", - state = "started", - "Protocol runtime started" - ); - Some(tx) - } - Ok(None) => { - info!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "webdav", - state = "disabled", - "Protocol runtime disabled" - ); - None - } - Err(e) => { - error!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "webdav", - state = "initialization_failed", - error = %e, - "Protocol runtime initialization failed" - ); - return Err(Error::other(e)); - } - }; - - #[cfg(not(feature = "webdav"))] - let webdav_shutdown_tx: Option = None; - - // Initialize SFTP system if enabled - #[cfg(feature = "sftp")] - let sftp_shutdown_tx = match init_sftp_system().await { - Ok(Some(tx)) => { - debug!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "sftp", - state = "started", - "Protocol runtime started" - ); - Some(tx) - } - Ok(None) => { - info!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "sftp", - state = "disabled", - "Protocol runtime disabled" - ); - None - } - Err(e) => { - error!( - event = EVENT_PROTOCOL_SYSTEM_STATE, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_STARTUP, - protocol = "sftp", - state = "initialization_failed", - error = %e, - "Protocol runtime initialization failed" - ); - return Err(Error::other(e)); - } - }; - - #[cfg(not(feature = "sftp"))] - let sftp_shutdown_tx: Option = None; + let protocol_shutdowns = init_protocol_shutdown_senders().await?; // Initialize buffer profiling system init_buffer_profile_system(&config); @@ -979,12 +805,7 @@ async fn run(config: rustfs::config::Config) -> Result<()> { shutdown_signal, s3_shutdown_tx, console_shutdown_tx, - ProtocolShutdownSenders { - ftp: ftp_shutdown_tx, - ftps: ftps_shutdown_tx, - webdav: webdav_shutdown_tx, - sftp: sftp_shutdown_tx, - }, + protocol_shutdowns, ctx.clone(), ) .await; @@ -1027,15 +848,6 @@ fn log_external_prefix_compat_report(report: &ExternalEnvCompatReport) { } } -/// Shutdown channels for every protocol server. None means the protocol was -/// disabled at startup. -struct ProtocolShutdownSenders { - ftp: Option, - ftps: Option, - webdav: Option, - sftp: Option, -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum BackgroundShutdownStep { DataScanner, diff --git a/rustfs/src/startup_protocols.rs b/rustfs/src/startup_protocols.rs new file mode 100644 index 000000000..00d5cb984 --- /dev/null +++ b/rustfs/src/startup_protocols.rs @@ -0,0 +1,153 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[cfg(feature = "sftp")] +use crate::init::init_sftp_system; +#[cfg(feature = "webdav")] +use crate::init::init_webdav_system; +#[cfg(feature = "ftps")] +use crate::init::{init_ftp_system, init_ftps_system}; +use crate::server::ShutdownHandle; +use std::future::Future; +use std::io::{Error, Result}; +use tracing::{debug, error, info}; + +const LOG_COMPONENT_MAIN: &str = "main"; +const LOG_SUBSYSTEM_STARTUP: &str = "startup"; +const EVENT_PROTOCOL_SYSTEM_STATE: &str = "protocol_system_state"; + +type ProtocolInitResult = std::result::Result, Box>; + +/// Shutdown channels for every protocol server. None means the protocol was +/// disabled at startup or not compiled in. +pub struct ProtocolShutdownSenders { + pub ftp: Option, + pub ftps: Option, + pub webdav: Option, + pub sftp: Option, +} + +pub async fn init_protocol_shutdown_senders() -> Result { + Ok(ProtocolShutdownSenders { + ftp: init_ftp_protocol().await?, + ftps: init_ftps_protocol().await?, + webdav: init_webdav_protocol().await?, + sftp: init_sftp_protocol().await?, + }) +} + +#[cfg(feature = "ftps")] +async fn init_ftp_protocol() -> Result> { + init_protocol("ftp", init_ftp_system).await +} + +#[cfg(not(feature = "ftps"))] +async fn init_ftp_protocol() -> Result> { + Ok(None) +} + +#[cfg(feature = "ftps")] +async fn init_ftps_protocol() -> Result> { + init_protocol("ftps", init_ftps_system).await +} + +#[cfg(not(feature = "ftps"))] +async fn init_ftps_protocol() -> Result> { + Ok(None) +} + +#[cfg(feature = "webdav")] +async fn init_webdav_protocol() -> Result> { + init_protocol("webdav", init_webdav_system).await +} + +#[cfg(not(feature = "webdav"))] +async fn init_webdav_protocol() -> Result> { + Ok(None) +} + +#[cfg(feature = "sftp")] +async fn init_sftp_protocol() -> Result> { + init_protocol("sftp", init_sftp_system).await +} + +#[cfg(not(feature = "sftp"))] +async fn init_sftp_protocol() -> Result> { + Ok(None) +} + +async fn init_protocol(protocol: &'static str, init: InitFn) -> Result> +where + InitFn: FnOnce() -> InitFuture, + InitFuture: Future, +{ + match init().await { + Ok(Some(tx)) => { + debug!( + event = EVENT_PROTOCOL_SYSTEM_STATE, + component = LOG_COMPONENT_MAIN, + subsystem = LOG_SUBSYSTEM_STARTUP, + protocol = protocol, + state = "started", + "Protocol runtime started" + ); + Ok(Some(tx)) + } + Ok(None) => { + info!( + event = EVENT_PROTOCOL_SYSTEM_STATE, + component = LOG_COMPONENT_MAIN, + subsystem = LOG_SUBSYSTEM_STARTUP, + protocol = protocol, + state = "disabled", + "Protocol runtime disabled" + ); + Ok(None) + } + Err(err) => { + error!( + event = EVENT_PROTOCOL_SYSTEM_STATE, + component = LOG_COMPONENT_MAIN, + subsystem = LOG_SUBSYSTEM_STARTUP, + protocol = protocol, + state = "initialization_failed", + error = %err, + "Protocol runtime initialization failed" + ); + Err(Error::other(err)) + } + } +} + +#[cfg(test)] +mod tests { + use super::init_protocol; + + #[tokio::test] + async fn init_protocol_returns_none_when_disabled() { + let shutdown = init_protocol("test", || async { Ok(None) }).await; + + assert!(matches!(shutdown, Ok(None))); + } + + #[tokio::test] + async fn init_protocol_maps_startup_error() { + let shutdown = init_protocol("test", || async { + Err(Box::::from("startup failed")) + }) + .await; + + assert!(shutdown.is_err()); + } +}