test: preserve background config reload assumptions (#3409)

docs/architecture/migration-progress.md

@@ -5,16 +5,16 @@ 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-metrics-runtime-controller-status`
+- Branch: `overtrue/arch-config-reload-preservation`
-- Baseline: `origin/main` at `76b375c478a8c2f5c79e62e848156a55915ed769`
+- Baseline: `origin/main` at `6b1e114b39a51166a872388ad64f86ee3d910864`
-- PR type for this branch: `behavior-change`
+- PR type for this branch: `test-only`
-- Runtime behavior changes: none; the metrics runtime controller/status surface
+- Runtime behavior changes: none; config reload and shutdown behavior are
-  only returns read-only snapshot/reconcile data and never starts, stops,
+  preserved while their no-worker-mutation and shutdown-order assumptions are
-  resizes, or wakes collectors.
+  made testable.
-- Rust code changes: add metrics runtime status/controller snapshots and
+- Rust code changes: add module-local config reload and background shutdown
-  reconcile plans with explicit no-op worker mutation.
+  plans plus focused tests for `TEST-BGC-002`.
 - CI/script changes: none.
-- Docs changes: record `BGC-006` metrics runtime controller/status scope.
+- Docs changes: record `TEST-BGC-002` config reload preservation coverage.
 
 ## Phase 0 Tasks
 
@@ -425,48 +425,57 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
     cancellation, and startup call shape.
   - Verification: focused metrics runtime tests, compile checks, formatting,
     migration guards, Rust risk scan, and pre-commit quality gate.
+- [x] `TEST-BGC-002` Preserve config reload and shutdown assumptions.
+  - Acceptance: dynamic server-config reload reports no worker mutation for
+    scanner/heal runtime config, bucket lifecycle/replication config files are
+    not dynamic server-config reload targets, and background shutdown keeps
+    scanner before AHM while preserving the scanner-implies-AHM dependency.
+  - Must preserve: no scanner, heal, lifecycle, replication, audit, storage
+    class, peer-signal, readiness, or worker lifecycle behavior change.
+  - Verification: focused config reload and shutdown tests, compile checks,
+    formatting, diff hygiene, and Rust risk scan.
 
 ## Next PRs
 
-1. `test-only`: add config-reload preservation coverage for scanner/heal/
+1. `behavior-change`: add another low-risk read-only status surface now that
-   lifecycle/replication in `TEST-BGC-002`.
+   config-reload and shutdown assumptions have dedicated preservation tests.
-2. `behavior-change`: add another low-risk read-only status surface only after
+2. `test-only`: extend preservation coverage only if the next controller touches
-   preserving config-reload and shutdown assumptions.
+   additional startup/shutdown or config reload assumptions.
 
 ## Pre-Push Review Log
 
 | Expert | Status | Notes |
 |---|---|---|
-| Quality/architecture | pass | Confirmed metrics runtime follows the established typed desired/status/reconcile shape without adding a generic scheduler, registry, or admin route. |
+| Quality/architecture | pass | Confirmed the new plans are module-local test seams only and do not add a generic scheduler, registry, route, or public API. |
-| Migration preservation | pass | Confirmed reconcile reports `worker_mutation: none`, preserving collector grouping, interval parsing, tombstone cycles, cancellation, and metrics emission behavior. |
+| Migration preservation | pass | Confirmed reload plans report `worker_mutation: none`, bucket lifecycle/replication remain outside dynamic server-config reload, and shutdown keeps scanner before AHM. |
-| Testing/verification | pass | Focused tests, compile checks, formatting, diff hygiene, migration guards, Rust risk scan, and `make pre-commit` passed. |
+| Testing/verification | pass | Focused tests, formatting, diff hygiene, migration guards, Rust risk scan, and `make pre-commit` passed. |
 
 ## Verification Notes
 
-Passed on `76b375c478a8c2f5c79e62e848156a55915ed769`:
+Passed on `6b1e114b39a51166a872388ad64f86ee3d910864`:
-- `cargo test -p rustfs-obs metrics_runtime --lib`; 4 passed.
+- `cargo test -p rustfs --bin rustfs background_shutdown_plan_keeps_scanner_before_ahm`; 1 passed.
-- `cargo check -p rustfs-obs`.
+- `cargo test -p rustfs background_config_reload_plan_never_mutates_workers`; 1 passed.
-- `cargo check -p rustfs --lib`.
 - `cargo fmt --all`.
 - `cargo fmt --all --check`.
 - `git diff --check`.
 - `./scripts/check_architecture_migration_rules.sh`.
 - `./scripts/check_layer_dependencies.sh`.
-- `./scripts/check_metrics_migration_refs.sh`.
+- Rust risk scan for changed Rust files; only pre-existing test `expect`,
-- Rust risk scan for changed Rust files; only an existing doc-comment
+  startup `expect`, import alias `as`, and startup `eprintln!` matches remain;
-  `println!` example and an existing bounded scheduler numeric cast matched.
+  added-line risky-pattern scan found no matches.
-- `make pre-commit`; all checks passed, including nextest 5880 passed and 111
+- `make pre-commit`; all checks passed, including nextest 5882 passed and 111
   skipped.
 
 Notes:
-- This slice adds reconcile/status data only. It does not apply reconcile output
+- This slice does not make dynamic config reload start, stop, restart, resize,
-  to the running metrics collectors.
+  or wake scanner/heal/lifecycle/replication workers.
-- There is no admin/API exposure in this PR; future controller harness work
+- The shutdown plan preserves the existing scanner before AHM order and keeps
-  should stay isolated from scanner, heal, lifecycle, and replication.
+  scanner shutdown implying AHM shutdown.
 
 ## Handoff Notes
 
-- Next migration slice should start `TEST-BGC-002` config-reload preservation
+- Next migration slice can add another low-risk read-only background status
-  coverage before broader controller surfaces.
+  surface while keeping worker lifecycle mutations out of scope.
-- Keep scanner, heal, lifecycle, replication, disk health, and config reload out
+- Keep scanner, heal, lifecycle, replication, disk health, and config reload
-  of broad controller movement until dedicated preservation tests exist.
+  behavior changes out of broad controller movement until each has dedicated
+  status snapshots.

rustfs/src/admin/service/config.rs

@@ -42,6 +42,22 @@ pub fn is_dynamic_config_subsystem(sub_system: &str) -> bool {
     )
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum DynamicConfigWorkerMutation {
+    None,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+struct DynamicConfigReloadPlan {
+    worker_mutation: DynamicConfigWorkerMutation,
+}
+
+fn dynamic_config_reload_plan(sub_system: &str) -> Option<DynamicConfigReloadPlan> {
+    is_dynamic_config_subsystem(sub_system).then_some(DynamicConfigReloadPlan {
+        worker_mutation: DynamicConfigWorkerMutation::None,
+    })
+}
+
 fn internal_error(message: impl Into<String>) -> S3Error {
     S3Error::with_message(S3ErrorCode::InternalError, message.into())
 }
@@ -257,7 +273,7 @@ pub async fn validate_server_config(config: &ServerConfig, sub_system: Option<&s
 }
 
 pub async fn apply_dynamic_config_for_subsystem(config: &ServerConfig, sub_system: &str) -> S3Result<bool> {
-    if !is_dynamic_config_subsystem(sub_system) {
+    if dynamic_config_reload_plan(sub_system).is_none() {
         return Ok(false);
     }
 
@@ -359,6 +375,10 @@ mod tests {
     use rustfs_config::oidc::{OIDC_CLIENT_ID, OIDC_CONFIG_URL, OIDC_SCOPES};
     use rustfs_config::{HEAL_SUB_SYS, SCANNER_SUB_SYS};
     use rustfs_config::{MQTT_BROKER, MQTT_QUEUE_DIR, MQTT_TOPIC, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_DIR};
+    use rustfs_ecstore::bucket::metadata::{BUCKET_LIFECYCLE_CONFIG, BUCKET_REPLICATION_CONFIG};
+
+    const LIFECYCLE_RELOAD_LABEL: &str = "lifecycle";
+    const REPLICATION_RELOAD_LABEL: &str = "replication";
 
     #[test]
     fn dynamic_config_subsystems_match_runtime_apply_support() {
@@ -371,6 +391,31 @@ mod tests {
         assert!(!is_dynamic_config_subsystem("notify_webhook"));
     }
 
+    #[test]
+    fn background_config_reload_plan_never_mutates_workers() {
+        for sub_system in [
+            STORAGE_CLASS_SUB_SYS,
+            AUDIT_WEBHOOK_SUB_SYS,
+            AUDIT_MQTT_SUB_SYS,
+            SCANNER_SUB_SYS,
+            HEAL_SUB_SYS,
+        ] {
+            assert_eq!(
+                dynamic_config_reload_plan(sub_system).map(|plan| plan.worker_mutation),
+                Some(DynamicConfigWorkerMutation::None)
+            );
+        }
+
+        for bucket_config in [
+            BUCKET_LIFECYCLE_CONFIG,
+            BUCKET_REPLICATION_CONFIG,
+            LIFECYCLE_RELOAD_LABEL,
+            REPLICATION_RELOAD_LABEL,
+        ] {
+            assert_eq!(dynamic_config_reload_plan(bucket_config), None);
+        }
+    }
+
     #[test]
     fn validate_storage_class_kvs_rejects_invalid_parity() {
         let mut kvs = KVS::new();

rustfs/src/main.rs

@@ -1011,6 +1011,23 @@ struct ProtocolShutdownSenders {
     sftp: Option<ShutdownHandle>,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum BackgroundShutdownStep {
+    DataScanner,
+    Ahm,
+}
+
+fn background_shutdown_steps(enable_scanner: bool, enable_heal: bool) -> Vec<BackgroundShutdownStep> {
+    let mut steps = Vec::with_capacity(2);
+    if enable_scanner {
+        steps.push(BackgroundShutdownStep::DataScanner);
+    }
+    if enable_heal || enable_scanner {
+        steps.push(BackgroundShutdownStep::Ahm);
+    }
+    steps
+}
+
 /// Handles the shutdown process of the server
 async fn handle_shutdown(
     state_manager: &ServiceStateManager,
@@ -1044,33 +1061,37 @@ async fn handle_shutdown(
     let enable_heal = get_env_bool_with_aliases(ENV_HEAL_ENABLED, &[ENV_HEAL_ENABLED_DEPRECATED], true);
 
     // Stop background services based on what was enabled.
-    if enable_scanner {
+    let background_steps = background_shutdown_steps(enable_scanner, enable_heal);
-        info!(
+    for step in &background_steps {
-            target: "rustfs::main::handle_shutdown",
+        match step {
-            event = EVENT_BACKGROUND_SERVICE_SHUTDOWN,
+            BackgroundShutdownStep::DataScanner => {
-            component = LOG_COMPONENT_MAIN,
+                info!(
-            subsystem = LOG_SUBSYSTEM_STARTUP,
+                    target: "rustfs::main::handle_shutdown",
-            service = "data_scanner",
+                    event = EVENT_BACKGROUND_SERVICE_SHUTDOWN,
-            state = "stopping",
+                    component = LOG_COMPONENT_MAIN,
-            "Background service shutdown started"
+                    subsystem = LOG_SUBSYSTEM_STARTUP,
-        );
+                    service = "data_scanner",
-        shutdown_background_services();
+                    state = "stopping",
+                    "Background service shutdown started"
+                );
+                shutdown_background_services();
+            }
+            BackgroundShutdownStep::Ahm => {
+                info!(
+                    target: "rustfs::main::handle_shutdown",
+                    event = EVENT_BACKGROUND_SERVICE_SHUTDOWN,
+                    component = LOG_COMPONENT_MAIN,
+                    subsystem = LOG_SUBSYSTEM_STARTUP,
+                    service = "ahm",
+                    state = "stopping",
+                    "Background service shutdown started"
+                );
+                shutdown_ahm_services();
+            }
+        }
     }
 
-    if enable_heal || enable_scanner {
+    if background_steps.is_empty() {
-        info!(
-            target: "rustfs::main::handle_shutdown",
-            event = EVENT_BACKGROUND_SERVICE_SHUTDOWN,
-            component = LOG_COMPONENT_MAIN,
-            subsystem = LOG_SUBSYSTEM_STARTUP,
-            service = "ahm",
-            state = "stopping",
-            "Background service shutdown started"
-        );
-        shutdown_ahm_services();
-    }
-
-    if !enable_scanner && !enable_heal {
         info!(
             target: "rustfs::main::handle_shutdown",
             event = EVENT_BACKGROUND_SERVICE_SHUTDOWN,
@@ -1268,4 +1289,18 @@ mod tests {
         assert!(!DEFAULT_CREDENTIALS_WARNING_MESSAGE.contains(rustfs_credentials::DEFAULT_ACCESS_KEY));
         assert!(!DEFAULT_CREDENTIALS_WARNING_MESSAGE.contains(rustfs_credentials::DEFAULT_SECRET_KEY));
     }
+
+    #[test]
+    fn background_shutdown_plan_keeps_scanner_before_ahm() {
+        assert_eq!(
+            background_shutdown_steps(true, true),
+            vec![BackgroundShutdownStep::DataScanner, BackgroundShutdownStep::Ahm]
+        );
+        assert_eq!(
+            background_shutdown_steps(true, false),
+            vec![BackgroundShutdownStep::DataScanner, BackgroundShutdownStep::Ahm]
+        );
+        assert_eq!(background_shutdown_steps(false, true), vec![BackgroundShutdownStep::Ahm]);
+        assert!(background_shutdown_steps(false, false).is_empty());
+    }
 }