diff --git a/crates/ecstore/src/rpc/client.rs b/crates/ecstore/src/rpc/client.rs index 16f4999bb..521a784a6 100644 --- a/crates/ecstore/src/rpc/client.rs +++ b/crates/ecstore/src/rpc/client.rs @@ -143,7 +143,7 @@ mod tests { use tracing_subscriber::{Registry, layer::SubscriberExt}; fn ensure_test_rpc_secret() { - let _ = rustfs_credentials::GLOBAL_RUSTFS_RPC_SECRET.set("test-rpc-secret".to_string()); + runtime_sources::ensure_test_rpc_secret(); } fn with_trace_parent(trace_id_hex: &str, f: F) diff --git a/crates/ecstore/src/rpc/http_auth.rs b/crates/ecstore/src/rpc/http_auth.rs index a62bf029c..9beb270eb 100644 --- a/crates/ecstore/src/rpc/http_auth.rs +++ b/crates/ecstore/src/rpc/http_auth.rs @@ -164,6 +164,7 @@ pub fn verify_rpc_signature(url: &str, method: &Method, headers: &HeaderMap) -> mod tests { use super::*; use crate::rpc::context_propagation::REQUEST_ID_HEADER; + use crate::runtime_sources; use http::{HeaderMap, Method}; use std::io::{self, Write}; use std::sync::{Arc, Mutex}; @@ -215,7 +216,7 @@ mod tests { } fn ensure_test_rpc_secret() { - let _ = rustfs_credentials::GLOBAL_RUSTFS_RPC_SECRET.set("test-rpc-secret".to_string()); + runtime_sources::ensure_test_rpc_secret(); } #[test] diff --git a/crates/ecstore/src/rpc/remote_disk.rs b/crates/ecstore/src/rpc/remote_disk.rs index ee3675bfc..6a9a38020 100644 --- a/crates/ecstore/src/rpc/remote_disk.rs +++ b/crates/ecstore/src/rpc/remote_disk.rs @@ -2220,7 +2220,7 @@ impl DiskAPI for RemoteDisk { mod tests { use super::*; use crate::rpc::internode_data_transport::{InternodeDataTransportCapabilities, TcpHttpInternodeDataTransport}; - use rustfs_common::GLOBAL_CONN_MAP; + use crate::runtime_sources; use serde_json::Value; use std::io::{self as std_io, Write}; use std::pin::Pin; @@ -2709,7 +2709,7 @@ mod tests { #[tokio::test] async fn test_remote_disk_recovery_requires_disk_rpc_readiness() { init_tracing(Level::ERROR); - let _ = rustfs_credentials::GLOBAL_RUSTFS_RPC_SECRET.set("test-rpc-secret".to_string()); + runtime_sources::ensure_test_rpc_secret(); let listener = match TcpListener::bind("127.0.0.1:0").await { Ok(listener) => listener, @@ -2737,8 +2737,8 @@ mod tests { health.mark_failure(&endpoint, "test_failure"); assert_eq!(health.runtime_state(), RuntimeDriveHealthState::Offline); let channel = TonicEndpoint::from_shared(base_addr.clone()).unwrap().connect_lazy(); - GLOBAL_CONN_MAP.write().await.insert(base_addr.clone(), channel); - assert!(GLOBAL_CONN_MAP.read().await.contains_key(&base_addr)); + runtime_sources::cache_test_node_channel(base_addr.clone(), channel).await; + assert!(runtime_sources::test_node_channel_is_cached(&base_addr).await); temp_env::async_with_vars( [ @@ -2764,7 +2764,7 @@ mod tests { "a plain TCP listener without disk_info RPC readiness must not restore the remote disk online" ); assert!( - !GLOBAL_CONN_MAP.read().await.contains_key(&base_addr), + !runtime_sources::test_node_channel_is_cached(&base_addr).await, "failed recovery probes should evict stale cached gRPC channels" ); }, @@ -3336,8 +3336,8 @@ mod tests { .unwrap(); let channel = TonicEndpoint::from_shared(addr.clone()).unwrap().connect_lazy(); - GLOBAL_CONN_MAP.write().await.insert(addr.clone(), channel); - assert!(GLOBAL_CONN_MAP.read().await.contains_key(&addr)); + runtime_sources::cache_test_node_channel(addr.clone(), channel).await; + assert!(runtime_sources::test_node_channel_is_cached(&addr).await); let _ = remote_disk .execute_with_timeout( @@ -3351,7 +3351,7 @@ mod tests { .expect_err("timeout should fail"); assert!( - !GLOBAL_CONN_MAP.read().await.contains_key(&addr), + !runtime_sources::test_node_channel_is_cached(&addr).await, "timeout should evict cached connection" ); } @@ -3380,7 +3380,7 @@ mod tests { .unwrap(); let channel = TonicEndpoint::from_shared(addr.clone()).unwrap().connect_lazy(); - GLOBAL_CONN_MAP.write().await.insert(addr.clone(), channel); + runtime_sources::cache_test_node_channel(addr.clone(), channel).await; let err = remote_disk .execute_with_timeout( @@ -3407,7 +3407,7 @@ mod tests { "first timeout-like error should move the remote disk into suspect state" ); assert!( - !GLOBAL_CONN_MAP.read().await.contains_key(&addr), + !runtime_sources::test_node_channel_is_cached(&addr).await, "timeout-like errors should evict cached connection" ); } @@ -3436,7 +3436,7 @@ mod tests { .unwrap(); let channel = TonicEndpoint::from_shared(addr.clone()).unwrap().connect_lazy(); - GLOBAL_CONN_MAP.write().await.insert(addr.clone(), channel); + runtime_sources::cache_test_node_channel(addr.clone(), channel).await; let err = remote_disk .execute_with_timeout( @@ -3468,7 +3468,7 @@ mod tests { "first network-like error should move the remote disk into suspect state" ); assert!( - !GLOBAL_CONN_MAP.read().await.contains_key(&addr), + !runtime_sources::test_node_channel_is_cached(&addr).await, "network-like errors should evict cached connection" ); } @@ -3497,7 +3497,7 @@ mod tests { .unwrap(); let channel = TonicEndpoint::from_shared(addr.clone()).unwrap().connect_lazy(); - GLOBAL_CONN_MAP.write().await.insert(addr.clone(), channel); + runtime_sources::cache_test_node_channel(addr.clone(), channel).await; let err = remote_disk .execute_with_timeout_for_op_and_health_action( @@ -3521,7 +3521,7 @@ mod tests { "ignored network-like error should not mark remote disk faulty" ); assert!( - GLOBAL_CONN_MAP.read().await.contains_key(&addr), + runtime_sources::test_node_channel_is_cached(&addr).await, "ignored network-like error should not evict cached connection" ); } @@ -3550,7 +3550,7 @@ mod tests { .unwrap(); let channel = TonicEndpoint::from_shared(addr.clone()).unwrap().connect_lazy(); - GLOBAL_CONN_MAP.write().await.insert(addr.clone(), channel); + runtime_sources::cache_test_node_channel(addr.clone(), channel).await; let err = remote_disk .execute_with_timeout(|| async { Err::<(), Error>(DiskError::FileNotFound) }, Duration::from_secs(1)) @@ -3560,7 +3560,7 @@ mod tests { assert_eq!(err, DiskError::FileNotFound); assert!(remote_disk.is_online().await, "business errors should not mark remote disk faulty"); assert!( - GLOBAL_CONN_MAP.read().await.contains_key(&addr), + runtime_sources::test_node_channel_is_cached(&addr).await, "business errors should not evict cached connection" ); } diff --git a/crates/ecstore/src/rpc/remote_locker.rs b/crates/ecstore/src/rpc/remote_locker.rs index dda07e623..2a34922d1 100644 --- a/crates/ecstore/src/rpc/remote_locker.rs +++ b/crates/ecstore/src/rpc/remote_locker.rs @@ -536,7 +536,7 @@ impl LockClient for RemoteClient { #[cfg(test)] mod tests { use super::*; - use rustfs_common::GLOBAL_CONN_MAP; + use crate::runtime_sources; use rustfs_lock::{ObjectKey, types::LockPriority}; use tokio::net::TcpListener; use tokio::task::JoinHandle; @@ -560,11 +560,11 @@ mod tests { async fn cache_lazy_channel(addr: &str) { let channel = TonicEndpoint::from_shared(addr.to_string()).unwrap().connect_lazy(); - GLOBAL_CONN_MAP.write().await.insert(addr.to_string(), channel); + runtime_sources::cache_test_node_channel(addr.to_string(), channel).await; } fn ensure_test_rpc_secret() { - let _ = rustfs_credentials::GLOBAL_RUSTFS_RPC_SECRET.set("test-rpc-secret".to_string()); + runtime_sources::ensure_test_rpc_secret(); } fn test_lock_request(timeout_duration: Duration) -> LockRequest { @@ -581,7 +581,7 @@ mod tests { return; }; cache_lazy_channel(&addr).await; - assert!(GLOBAL_CONN_MAP.read().await.contains_key(&addr)); + assert!(runtime_sources::test_node_channel_is_cached(&addr).await); temp_env::async_with_vars([(rustfs_config::ENV_OBJECT_LOCK_RPC_TIMEOUT_MS, Some("50"))], async { let client = RemoteClient::new(addr.clone()); @@ -609,7 +609,7 @@ mod tests { response.error ); assert!( - !GLOBAL_CONN_MAP.read().await.contains_key(&addr), + !runtime_sources::test_node_channel_is_cached(&addr).await, "transport timeout should evict cached connection" ); }) @@ -626,7 +626,7 @@ mod tests { return; }; cache_lazy_channel(&addr).await; - assert!(GLOBAL_CONN_MAP.read().await.contains_key(&addr)); + assert!(runtime_sources::test_node_channel_is_cached(&addr).await); temp_env::async_with_vars([(rustfs_config::ENV_OBJECT_LOCK_RPC_TIMEOUT_MS, Some("50"))], async { let client = RemoteClient::new(addr.clone()); @@ -655,7 +655,7 @@ mod tests { responses[0].error ); assert!( - !GLOBAL_CONN_MAP.read().await.contains_key(&addr), + !runtime_sources::test_node_channel_is_cached(&addr).await, "batch transport timeout should evict cached connection" ); }) diff --git a/crates/ecstore/src/runtime_sources.rs b/crates/ecstore/src/runtime_sources.rs index a76e87544..3d7c76b19 100644 --- a/crates/ecstore/src/runtime_sources.rs +++ b/crates/ecstore/src/runtime_sources.rs @@ -50,6 +50,9 @@ use tokio::sync::RwLock; use tonic::transport::Channel; use uuid::Uuid; +#[cfg(test)] +const TEST_RPC_SECRET: &str = "test-rpc-secret"; + pub(crate) fn record_erasure_write_quorum_failure(stage: &'static str, dominant_error: &'static str) { global_internode_metrics().record_erasure_write_quorum_failure(stage, dominant_error); } @@ -165,6 +168,21 @@ pub(crate) async fn cached_node_channel(addr: &str) -> Option { GLOBAL_CONN_MAP.read().await.get(addr).cloned() } +#[cfg(test)] +pub(crate) async fn cache_test_node_channel(addr: String, channel: Channel) { + GLOBAL_CONN_MAP.write().await.insert(addr, channel); +} + +#[cfg(test)] +pub(crate) async fn test_node_channel_is_cached(addr: &str) -> bool { + GLOBAL_CONN_MAP.read().await.contains_key(addr) +} + +#[cfg(test)] +pub(crate) fn ensure_test_rpc_secret() { + let _ = rustfs_credentials::GLOBAL_RUSTFS_RPC_SECRET.set(TEST_RPC_SECRET.to_owned()); +} + pub(crate) fn storage_class_parity(storage_class: Option<&str>) -> Option { get_global_storage_class().and_then(|sc| sc.get_parity_for_sc(storage_class.unwrap_or_default())) } diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 562b69622..ca64ec4ef 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,9 +5,9 @@ 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-ecstore-batch-config-runtime-sources` -- Baseline: completed `C-011/C-012/C-013/API-055/API-059/API-079/API-080/API-081/API-082/API-083/API-084/API-085/API-086/API-087/API-088/API-089/API-090/API-091/API-092/API-093/API-094/API-095/API-096/API-097/API-098/API-099/API-100/API-101/API-102/API-103/API-104/API-105/API-106/API-107/API-108/API-109/API-110/API-111/API-112/API-113/API-114/API-115/API-116/API-117/API-118/API-119/API-120/API-121/API-122/API-123/API-124/API-125/API-126/API-127/API-128/API-129/API-130/API-131/API-132/API-133/API-134/API-135/API-136/API-137/API-138/API-139/API-140/API-141/API-142/API-143/API-144/API-145/API-146/API-147/API-148/API-149/API-150/API-151/API-152/API-153/API-154/API-155/API-156/API-157/API-158/API-159/API-160/API-161/API-162/API-163/API-164/API-165/API-166/API-167/API-168/API-169/API-170/API-171/API-172/API-173/API-174/API-175/API-176/API-177/API-178/API-179/API-180/API-181/API-182/API-183/API-184/API-185/API-186/API-187/API-188/API-189/API-190/API-191/API-192/API-193/API-194/API-195/API-196/API-197`. -- Based on: API-196 merged main. +- Branch: `overtrue/arch-ecstore-rpc-test-runtime-sources` +- Baseline: completed `C-011/C-012/C-013/API-055/API-059/API-079/API-080/API-081/API-082/API-083/API-084/API-085/API-086/API-087/API-088/API-089/API-090/API-091/API-092/API-093/API-094/API-095/API-096/API-097/API-098/API-099/API-100/API-101/API-102/API-103/API-104/API-105/API-106/API-107/API-108/API-109/API-110/API-111/API-112/API-113/API-114/API-115/API-116/API-117/API-118/API-119/API-120/API-121/API-122/API-123/API-124/API-125/API-126/API-127/API-128/API-129/API-130/API-131/API-132/API-133/API-134/API-135/API-136/API-137/API-138/API-139/API-140/API-141/API-142/API-143/API-144/API-145/API-146/API-147/API-148/API-149/API-150/API-151/API-152/API-153/API-154/API-155/API-156/API-157/API-158/API-159/API-160/API-161/API-162/API-163/API-164/API-165/API-166/API-167/API-168/API-169/API-170/API-171/API-172/API-173/API-174/API-175/API-176/API-177/API-178/API-179/API-180/API-181/API-182/API-183/API-184/API-185/API-186/API-187/API-188/API-189/API-190/API-191/API-192/API-193/API-194/API-195/API-196/API-197/API-198`. +- Based on: stacked on API-197 PR branch while PR #3817 is pending. - PR type for this branch: `consumer-migration` - Runtime behavior changes: none. - Rust code changes: route replication pool, outbound TLS generation, runtime @@ -32,8 +32,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block client paths, plus ECStore facade server-config, storage-class, notification, bucket-metadata, endpoint, region, tier-config, server-address, object-store publication, lock-client publication, and local-node publication - paths, plus ECStore batch processor, dynamic storage-class publication, and - RustFS cluster snapshot facade alias paths, + paths, plus ECStore batch processor, dynamic storage-class publication, + RustFS cluster snapshot facade alias paths, and ECStore RPC test runtime + global helpers, through AppContext-first or owner-crate resolver boundaries. - CI/script changes: lock completed owner and test/fuzz boundaries against bare/glob imports, scattered raw ECStore facade subpaths, and startup @@ -43,7 +44,7 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block and storage owner thin bridge regressions, plus app context and notify event-bridge thin module regressions; accept the reviewed AppContext resolver reverse dependencies in the layer baseline. -- Docs changes: record the API-136 through API-197 owner facade cleanup. +- Docs changes: record the API-136 through API-198 owner facade cleanup. ## Phase 0 Tasks @@ -4880,6 +4881,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block formatting, diff hygiene, residual runtime-source scan, migration/layer guards, PR-before-push quality gate, and three-expert review. +- [x] `API-198` Centralize ECStore RPC test runtime globals. + - Do: route RPC test connection-cache seeding, connection-cache assertions, + and test RPC secret initialization through the ECStore runtime-source + boundary. + - Acceptance: RPC client, HTTP auth, remote locker, and remote disk tests no + longer import or mutate `GLOBAL_CONN_MAP` or `GLOBAL_RUSTFS_RPC_SECRET` + directly. + - Must preserve: cached-channel eviction assertions, timeout behavior, + signature generation behavior, and test-only secret initialization. + - Verification: focused RPC tests, formatting, diff hygiene, residual runtime + global scan, migration/layer guards, fast PR gate, and full PR gate before + PR. + ## Next PRs 1. `consumer-migration`: continue reducing direct global reads behind AppContext resolver boundaries. @@ -4888,6 +4902,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| +| Quality/architecture | pass | API-198 keeps RPC test connection-cache and RPC-secret globals behind the ECStore runtime-source test boundary. | +| Migration preservation | pass | Cached-channel eviction assertions, timeout behavior, signature generation behavior, and test-only secret initialization preserve existing semantics. | +| Testing/verification | pass | Focused RPC tests, formatting, migration/layer guards, residual scan, fast PR gate, and full PR gate are planned before PR. | | Quality/architecture | pass | API-197 keeps set-disk batch processor lookup, dynamic storage-class publication, and RustFS cluster snapshot facade imports behind owner boundaries. | | Migration preservation | pass | Batch concurrency defaults, storage-class lookup by drive count, first-set publication, config error handling, and cluster snapshot response shape preserve existing behavior. | | Testing/verification | pass | ECStore compile, focused set-disk/config checks, formatting, migration/layer guards, residual scan, fast PR gate, and full PR gate are planned before PR. |