diff --git a/Cargo.lock b/Cargo.lock index 8a867b8f6..656a0560f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9699,6 +9699,7 @@ dependencies = [ "rustfs-iam", "rustfs-io-metrics", "rustfs-notify", + "rustfs-storage-api", "rustfs-utils", "serde", "sysinfo", diff --git a/crates/ecstore/src/admin_server_info.rs b/crates/ecstore/src/admin_server_info.rs index 060b06981..8ef3d3ed9 100644 --- a/crates/ecstore/src/admin_server_info.rs +++ b/crates/ecstore/src/admin_server_info.rs @@ -20,7 +20,6 @@ use crate::{ global::{GLOBAL_BOOT_TIME, GLOBAL_Endpoints, get_global_deployment_id}, new_object_layer_fn, notification_sys::get_global_notification_sys, - store_api::StorageAPI, }; use crate::data_usage::load_data_usage_cache; @@ -32,6 +31,7 @@ use rustfs_protos::{ models::{PingBody, PingBodyBuilder}, proto_gen::node_service::{PingRequest, PingResponse}, }; +use rustfs_storage_api::StorageAdminApi; use std::{ collections::{HashMap, HashSet}, time::{Duration, SystemTime}, @@ -186,7 +186,7 @@ pub async fn get_local_server_property() -> ServerProperties { // sensitive.insert(rustfs_config::ENV_RUSTFS_ACCESS_KEY.to_string()); // sensitive.insert(rustfs_config::ENV_RUSTFS_SECRET_KEY.to_string()); if let Some(store) = new_object_layer_fn() { - let storage_info = store.local_storage_info().await; + let storage_info = StorageAdminApi::local_storage_info(store.as_ref()).await; props.state = ITEM_ONLINE.to_string(); props.disks = storage_info.disks; } else { @@ -252,7 +252,7 @@ pub async fn get_server_info(get_pools: bool) -> InfoMessage { warn!("load_data_usage_from_backend end {:?}", after3 - after2); - let backend_info = store.clone().backend_info().await; + let backend_info = StorageAdminApi::backend_info(store.as_ref()).await; let after4 = OffsetDateTime::now_utc(); diff --git a/crates/ecstore/src/metrics_realtime.rs b/crates/ecstore/src/metrics_realtime.rs index d2e6b7313..a2020fae2 100644 --- a/crates/ecstore/src/metrics_realtime.rs +++ b/crates/ecstore/src/metrics_realtime.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{admin_server_info::get_local_server_property, new_object_layer_fn, store_api::StorageAPI}; +use crate::{admin_server_info::get_local_server_property, new_object_layer_fn}; use chrono::Utc; use rustfs_common::{GLOBAL_LOCAL_NODE_NAME, GLOBAL_RUSTFS_ADDR, heal_channel::DriveState, metrics::global_metrics}; use rustfs_io_metrics::internode_metrics::global_internode_metrics; @@ -22,6 +22,7 @@ use rustfs_madmin::metrics::{ ScannerPacingPressureSnapshot as MadminScannerPacingPressureSnapshot, ScannerSourceCycleSnapshot as MadminScannerSourceCycleSnapshot, TimedAction as MadminTimedAction, }; +use rustfs_storage_api::StorageAdminApi; use rustfs_utils::os::get_drive_stats; use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; @@ -269,7 +270,7 @@ async fn collect_local_disks_metrics(disks: &HashSet) -> HashMap(&self, api: &S) -> rustfs_madmin::StorageInfo { + pub async fn storage_info(&self, api: &S) -> rustfs_madmin::StorageInfo + where + S: StorageAdminApi, + { let mut futures = Vec::with_capacity(self.peer_clients.len()); let endpoints = get_global_endpoints(); let peer_timeout = Duration::from_secs(5); @@ -307,14 +310,14 @@ impl NotificationSys { let mut replies = join_all(futures).await; - replies.push(Some(api.local_storage_info().await)); + replies.push(Some(StorageAdminApi::local_storage_info(api).await)); let mut disks = Vec::new(); for info in replies.into_iter().flatten() { disks.extend(info.disks); } - let backend = api.backend_info().await; + let backend = StorageAdminApi::backend_info(api).await; rustfs_madmin::StorageInfo { disks, backend } } diff --git a/crates/obs/Cargo.toml b/crates/obs/Cargo.toml index c32167c5b..ff6947fb8 100644 --- a/crates/obs/Cargo.toml +++ b/crates/obs/Cargo.toml @@ -41,6 +41,7 @@ rustfs-ecstore = { workspace = true } rustfs-iam = { workspace = true } rustfs-io-metrics = { workspace = true } rustfs-notify = { workspace = true } +rustfs-storage-api = { workspace = true } rustfs-utils = { workspace = true, features = ["ip"] } chrono = { workspace = true } flate2 = { workspace = true } diff --git a/crates/obs/src/metrics/stats_collector.rs b/crates/obs/src/metrics/stats_collector.rs index e94a5fa78..b932c00c0 100644 --- a/crates/obs/src/metrics/stats_collector.rs +++ b/crates/obs/src/metrics/stats_collector.rs @@ -34,12 +34,13 @@ use rustfs_ecstore::bucket::metadata_sys::get_quota_config; use rustfs_ecstore::bucket::replication::GLOBAL_REPLICATION_STATS; use rustfs_ecstore::data_usage::load_data_usage_from_backend; use rustfs_ecstore::global::get_global_bucket_monitor; +use rustfs_ecstore::new_object_layer_fn; use rustfs_ecstore::pools::{get_total_usable_capacity, get_total_usable_capacity_free}; use rustfs_ecstore::store_api::{BucketOperations, BucketOptions}; -use rustfs_ecstore::{StorageAPI, new_object_layer_fn}; use rustfs_iam::{get_global_iam_sys, oidc::oidc_plugin_authn_metrics_snapshot}; use rustfs_io_metrics::internode_metrics::global_internode_metrics; use rustfs_io_metrics::{ProcessStatusSnapshot, snapshot_process_resource_and_system}; +use rustfs_storage_api::StorageAdminApi; use std::collections::HashMap; use std::time::Duration; use sysinfo::{Networks, System}; @@ -151,7 +152,7 @@ pub async fn collect_cluster_and_health_stats() -> (ClusterStats, ClusterHealthS return (ClusterStats::default(), ClusterHealthStats::default()); }; - let storage_info = store.storage_info().await; + let storage_info = StorageAdminApi::storage_info(store.as_ref()).await; let raw_capacity: u64 = storage_info.disks.iter().map(|d| d.total_space).sum(); let used: u64 = storage_info.disks.iter().map(|d| d.used_space).sum(); let usable_capacity = get_total_usable_capacity(&storage_info.disks, &storage_info) as u64; @@ -525,7 +526,7 @@ pub async fn collect_disk_and_system_drive_stats() -> (Vec, Vec Option { /// Collect cluster config metrics from backend parity configuration. pub async fn collect_cluster_config_stats() -> Option { let store = new_object_layer_fn()?; - let backend = store.backend_info().await; + let backend = StorageAdminApi::backend_info(store.as_ref()).await; Some(ClusterConfigStats { rrs_parity: backend.rr_sc_parity.unwrap_or_default() as u32, @@ -724,8 +725,8 @@ pub async fn collect_erasure_set_stats() -> Vec { return Vec::new(); }; - let storage_info = store.storage_info().await; - let backend = store.backend_info().await; + let storage_info = StorageAdminApi::storage_info(store.as_ref()).await; + let backend = StorageAdminApi::backend_info(store.as_ref()).await; let mut grouped: HashMap<(usize, usize), ErasureSetStats> = HashMap::new(); for disk in &storage_info.disks { diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 83a9ee4d5..f14e50d9b 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,15 +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-admin-readiness-storage-admin` -- Baseline: `origin/main` at `b48d7b1fa514e5da274d652a0cb7f282521f46c0` +- Branch: `overtrue/arch-admin-observability-storage-reads` +- Baseline: `origin/main` at `8ae0cad6671562c0fffe56a7f288cd97fb87309d` - PR type for this branch: `consumer-migration` - Runtime behavior changes: none. -- Rust code changes: migrate grouped admin/readiness read-side consumers from - old `StorageAPI::{backend_info, storage_info}` trait imports to the - inventory-facing `StorageAdminApi` contract. +- Rust code changes: migrate grouped observability, RPC health, server-info, + realtime metrics, and notification read-side consumers from old + `StorageAPI::{backend_info, storage_info, local_storage_info}` trait imports + to the inventory-facing `StorageAdminApi` contract. - CI/script changes: none. -- Docs changes: record API-007 grouped consumer-migration context, +- Docs changes: record API-007 expanded read-side consumer-migration context, verification evidence, and expert review outcomes. ## Phase 0 Tasks @@ -201,18 +202,24 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Completed third slice: `rustfs/rustfs#3333` migrated `DefaultAdminUsecase` storage-info reads to `StorageAdminApi::storage_info`. - - Current branch slice: migrate grouped read-side admin/readiness consumers: - account-info `backend_info`, rebalance status `storage_info`, and runtime - readiness `storage_info`. - - Acceptance: account-info, rebalance status, and readiness no longer import - old `StorageAPI` only to read admin storage information. + - Completed fourth slice: `rustfs/rustfs#3334` migrated account-info + `backend_info`, rebalance status `storage_info`, and runtime readiness + `storage_info`. + - Current branch slice: migrate grouped read-side observability and health + consumers: obs cluster/disk/config/erasure-set metrics, RPC local storage + info response construction, ECStore server-info local disk/backend reads, + realtime disk metrics, and notification storage-info aggregation. + - Acceptance: these consumers no longer import old `StorageAPI` only to read + admin storage information; peer RPC client calls remain unchanged. - Must preserve: old `StorageAPI` trait shape, `StorageAPI::get_disks` - behavior, account-info response shape, rebalance used-space aggregation, - readiness degraded-state semantics, RPC `local_storage_info`, heal/scanner - consumers, and storage hot paths. + behavior, obs metric values, RPC msgpack map encoding and response shape, + server-info shape, realtime metric shape, notification peer + aggregation/cache fallback, heal/scanner consumers, object paths, + replication/config persistence, and storage hot paths. - Risk defense: group only read-side callers that delegate to the existing - ECStore admin info implementation; do not migrate RPC `local_storage_info`, - heal, scanner, observability, or storage hot-path consumers in this PR. + ECStore admin info implementation; do not migrate object APIs, scanner, + heal, replication, config persistence, or storage implementation internals + in this PR. ## Phase 8 Background Controller Tasks @@ -256,39 +263,48 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | Confirmed the diff stays limited to three read-side consumers and migration notes, with no manifest, ECStore, storage-api, RPC, heal, scanner, observability, or hot-path scope creep. | -| Migration preservation | pass | Confirmed the new and old ECStore trait paths still delegate to the same backend/storage-info handlers, while account-info response construction, rebalance aggregation, and readiness cache/degraded-state logic remain unchanged. | -| Testing/verification | pass | Confirmed touched-consumer focused tests, compile checks, migration guards, diff hygiene, and full pre-commit evidence are sufficient; no missing success-path integration test is a blocker for this call-path migration. | +| Quality/architecture | pass | Confirmed the diff stays limited to read-side consumer migration and the `rustfs-obs` contract dependency, with no object, scanner, heal, replication, config persistence, or storage hot-path scope creep. | +| Migration preservation | pass | Confirmed obs metric calculations, RPC response encoding, server-info shape, realtime disk metric shape, notification peer aggregation/timeout/cache fallback, and peer REST calls remain unchanged. | +| Testing/verification | pass | Confirmed focused tests, joint compile check, migration guards, diff hygiene, and added-line Rust quality scan are sufficient for this equivalent trait-entry migration while skipping full pre-commit under the current instruction. | ## Verification Notes Passed: +- `cargo fmt --all`. - `cargo fmt --all --check`. -- `cargo check -p rustfs --lib`. -- `cargo test -p rustfs admin::handlers::account_info --lib`; 3 passed. -- `cargo test -p rustfs admin::handlers::rebalance --lib`; 19 passed. -- `cargo test -p rustfs server::readiness --lib`; 13 passed. -- `cargo check -p rustfs-storage-api -p rustfs-ecstore -p rustfs --lib`. +- `cargo check -p rustfs-storage-api -p rustfs-ecstore -p rustfs-obs -p rustfs --lib`. +- `cargo test -p rustfs-obs stats_collector --lib`; 14 passed. +- `cargo test -p rustfs-ecstore admin_server_info --lib`; 1 passed. +- `cargo test -p rustfs-ecstore metrics_realtime --lib`; 5 passed. +- `cargo test -p rustfs-ecstore notification_sys --lib`; 17 passed. +- `cargo test -p rustfs local_storage_info_rpc_payload_uses_msgpack_map_encoding --lib`; 1 passed. - `./scripts/check_architecture_migration_rules.sh`. - `./scripts/check_layer_dependencies.sh`. - `./scripts/check_metrics_migration_refs.sh`. - `./scripts/check_unsafe_code_allowances.sh`. - `git diff --check`. -- `make NUM_CORES=1 pre-commit`. +- Rust code-quality scan on changed `.rs` files, plus added-line scan for + unwrap/expect, numeric casts, `Result<_, String>`, `Box`, + println/eprintln, and `Ordering::Relaxed`. Notes: -- This branch relies on the existing direct `rustfs` dependency on - `rustfs-storage-api` from earlier API-007 slices. -- No ECStore handler, old `StorageAPI` trait, RPC consumer, heal/scanner - consumer, observability consumer, or storage hot path is changed. -- Full pre-commit passed with nextest `5757 passed, 111 skipped`; workspace - doctests passed. +- This branch adds a direct `rustfs-obs` dependency on the existing + `rustfs-storage-api` workspace contract crate. +- Full pre-commit was intentionally skipped because the focused tests and guards + above passed, per the current migration instruction to increase PR granularity. +- The broad changed-file quality scan reports pre-existing test unwrap/expect + and pre-existing `admin_server_info.rs` println/eprintln; the added-line scan + found no new risky code patterns. +- No ECStore handler implementation, old `StorageAPI` trait, peer RPC client, + heal/scanner consumer, object path, replication/config persistence path, or + storage hot path is changed. - No temporary compatibility shim was added. ## Handoff Notes -- Keep this API-007 slice as a grouped read-side `consumer-migration` PR. -- Do not migrate RPC `local_storage_info`, heal, scanner, observability, or +- Keep this API-007 slice as a grouped observability/health/server-info + read-side `consumer-migration` PR. +- Do not migrate object APIs, scanner, heal, replication, config persistence, or storage hot-path consumers in this PR. - Do not remove or route around `StorageAPI::get_disks` in this PR. - Do not make the old `StorageAPI` trait inherit `StorageAdminApi` in this PR. diff --git a/rustfs/src/storage/rpc/health.rs b/rustfs/src/storage/rpc/health.rs index 4c7841c0c..3721fe095 100644 --- a/rustfs/src/storage/rpc/health.rs +++ b/rustfs/src/storage/rpc/health.rs @@ -14,6 +14,7 @@ use super::*; use crate::storage::rpc::encode_msgpack_map; +use rustfs_storage_api::StorageAdminApi; impl NodeService { pub(super) async fn handle_get_proc_info( @@ -221,7 +222,7 @@ impl NodeService { })); }; - let info = store.local_storage_info().await; + let info = StorageAdminApi::local_storage_info(store.as_ref()).await; match encode_msgpack_map(&info) { Ok(buf) => Ok(Response::new(LocalStorageInfoResponse { success: true, diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index f54ad3b3c..e3f9e5db6 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -37,7 +37,7 @@ use rustfs_ecstore::{ SERVICE_SIGNAL_RELOAD_DYNAMIC, }, store::{all_local_disk_path, find_local_disk_by_ref}, - store_api::{BucketOptions, DeleteBucketOptions, MakeBucketOptions, StorageAPI}, + store_api::{BucketOptions, DeleteBucketOptions, MakeBucketOptions}, }; use rustfs_filemeta::{FileInfo, MetacacheReader}; use rustfs_iam::{get_global_iam_sys, store::UserType};