refactor(ecstore): route admin read internals (#3336)

crates/ecstore/src/pools.rs

@@ -40,7 +40,7 @@ use crate::notification_sys::get_global_notification_sys;
 use crate::set_disk::SetDisks;
 use crate::store_api::{
     BucketOperations, BucketOptions, GetObjectReader, HealOperations, MakeBucketOptions, ObjectIO, ObjectOperations,
-    ObjectOptions, StorageAPI,
+    ObjectOptions,
 };
 use crate::{global::GLOBAL_LifecycleSys, sets::Sets, store::ECStore};
 use byteorder::{ByteOrder, LittleEndian, WriteBytesExt};
@@ -53,6 +53,7 @@ use rustfs_common::defer;
 use rustfs_common::heal_channel::HealOpts;
 use rustfs_concurrency::workers::Workers;
 use rustfs_filemeta::{FileInfoVersions, MetaCacheEntries, MetaCacheEntry, MetadataResolutionParams};
+use rustfs_storage_api::StorageAdminApi;
 use rustfs_utils::path::{encode_dir_object, path_join, path_to_bucket_object, path_to_bucket_object_with_base_path};
 use s3s::dto::{BucketLifecycleConfiguration, DefaultRetention, ReplicationConfiguration};
 use serde::{Deserialize, Serialize};
@@ -1233,8 +1234,8 @@ impl ECStore {
 
     async fn get_decommission_pool_space_info(&self, idx: usize) -> Result<PoolSpaceInfo> {
         if let Some(sets) = self.pools.get(idx) {
-            let mut info = sets.storage_info().await;
+            let mut info = sets.storage_info_snapshot().await;
-            info.backend = self.backend_info().await;
+            info.backend = StorageAdminApi::backend_info(self).await;
 
             let total = get_total_usable_capacity(&info.disks, &info);
             let free = get_total_usable_capacity_free(&info.disks, &info);

crates/ecstore/src/set_disk.rs

@@ -1557,6 +1557,34 @@ impl SetDisks {
     }
 }
 
+impl SetDisks {
+    pub(crate) async fn storage_info_snapshot(&self) -> rustfs_madmin::StorageInfo {
+        let disks = self.get_disks_internal().await;
+
+        get_storage_info(&disks, &self.set_endpoints).await
+    }
+
+    pub(crate) async fn local_storage_info_snapshot(&self) -> rustfs_madmin::StorageInfo {
+        let disks = self.get_disks_internal().await;
+
+        let mut local_disks: Vec<Option<DiskStore>> = Vec::new();
+        let mut local_endpoints = Vec::new();
+
+        for (i, ep) in self.set_endpoints.iter().enumerate() {
+            if ep.is_local {
+                local_disks.push(disks[i].clone());
+                local_endpoints.push(ep.clone());
+            }
+        }
+
+        get_storage_info(&local_disks, &local_endpoints).await
+    }
+
+    pub(crate) async fn disk_inventory(&self) -> Vec<Option<DiskStore>> {
+        self.get_disks_internal().await
+    }
+}
+
 #[async_trait::async_trait]
 impl StorageAPI for SetDisks {
     #[tracing::instrument(skip(self))]
@@ -1592,30 +1620,16 @@ impl StorageAPI for SetDisks {
     }
     #[tracing::instrument(skip(self))]
     async fn storage_info(&self) -> rustfs_madmin::StorageInfo {
-        let disks = self.get_disks_internal().await;
+        self.storage_info_snapshot().await
-
-        get_storage_info(&disks, &self.set_endpoints).await
     }
     #[tracing::instrument(skip(self))]
     async fn local_storage_info(&self) -> rustfs_madmin::StorageInfo {
-        let disks = self.get_disks_internal().await;
+        self.local_storage_info_snapshot().await
-
-        let mut local_disks: Vec<Option<DiskStore>> = Vec::new();
-        let mut local_endpoints = Vec::new();
-
-        for (i, ep) in self.set_endpoints.iter().enumerate() {
-            if ep.is_local {
-                local_disks.push(disks[i].clone());
-                local_endpoints.push(ep.clone());
-            }
-        }
-
-        get_storage_info(&local_disks, &local_endpoints).await
     }
 
     #[tracing::instrument(skip(self))]
     async fn get_disks(&self, _pool_idx: usize, _set_idx: usize) -> Result<Vec<Option<DiskStore>>> {
-        Ok(self.get_disks_internal().await)
+        Ok(self.disk_inventory().await)
     }
 
     #[tracing::instrument(skip(self))]

crates/ecstore/src/sets.rs

@@ -287,6 +287,46 @@ impl Sets {
         self.get_disks(self.get_hashed_set_index(key))
     }
 
+    pub(crate) async fn storage_info_snapshot(&self) -> rustfs_madmin::StorageInfo {
+        let mut futures = Vec::with_capacity(self.disk_set.len());
+
+        for set in self.disk_set.iter() {
+            futures.push(set.storage_info_snapshot())
+        }
+
+        let results = join_all(futures).await;
+        let mut disks = Vec::new();
+
+        for res in results.into_iter() {
+            disks.extend_from_slice(&res.disks);
+        }
+
+        rustfs_madmin::StorageInfo {
+            disks,
+            ..Default::default()
+        }
+    }
+
+    pub(crate) async fn local_storage_info_snapshot(&self) -> rustfs_madmin::StorageInfo {
+        let mut futures = Vec::with_capacity(self.disk_set.len());
+
+        for set in self.disk_set.iter() {
+            futures.push(set.local_storage_info_snapshot())
+        }
+
+        let results = join_all(futures).await;
+        let mut disks = Vec::new();
+
+        for res in results.into_iter() {
+            disks.extend_from_slice(&res.disks);
+        }
+
+        rustfs_madmin::StorageInfo {
+            disks,
+            ..Default::default()
+        }
+    }
+
     fn get_hashed_set_index(&self, input: &str) -> usize {
         match self.distribution_algo {
             DistributionAlgoVersion::V1 => crc_hash(input, self.disk_set.len()),
@@ -867,44 +907,11 @@ impl StorageAPI for Sets {
     }
     #[tracing::instrument(skip(self))]
     async fn storage_info(&self) -> rustfs_madmin::StorageInfo {
-        let mut futures = Vec::with_capacity(self.disk_set.len());
+        self.storage_info_snapshot().await
-
-        for set in self.disk_set.iter() {
-            futures.push(set.storage_info())
-        }
-
-        let results = join_all(futures).await;
-
-        let mut disks = Vec::new();
-
-        for res in results.into_iter() {
-            disks.extend_from_slice(&res.disks);
-        }
-
-        rustfs_madmin::StorageInfo {
-            disks,
-            ..Default::default()
-        }
     }
     #[tracing::instrument(skip(self))]
     async fn local_storage_info(&self) -> rustfs_madmin::StorageInfo {
-        let mut futures = Vec::with_capacity(self.disk_set.len());
+        self.local_storage_info_snapshot().await
-
-        for set in self.disk_set.iter() {
-            futures.push(set.local_storage_info())
-        }
-
-        let results = join_all(futures).await;
-
-        let mut disks = Vec::new();
-
-        for res in results.into_iter() {
-            disks.extend_from_slice(&res.disks);
-        }
-        rustfs_madmin::StorageInfo {
-            disks,
-            ..Default::default()
-        }
     }
 
     #[tracing::instrument(skip(self))]

crates/ecstore/src/store/rebalance.rs

@@ -14,6 +14,7 @@
 
 use super::*;
 use crate::config::get_global_storage_class;
+use rustfs_storage_api::StorageAdminApi;
 
 struct LatestObjectInfoCandidate {
     info: Option<ObjectInfo>,
@@ -700,7 +701,7 @@ impl ECStore {
         let mut drives_per_set = Vec::new();
         let mut total_sets = Vec::new();
 
-        for (idx, set_count) in self.set_drive_counts().iter().enumerate() {
+        for (idx, set_count) in StorageAdminApi::set_drive_counts(self).iter().enumerate() {
             if let Some(sc_parity) = standard_sc_parity {
                 standard_sc_data.push(set_count - sc_parity);
             }
@@ -755,7 +756,7 @@ impl ECStore {
         let mut futures = Vec::with_capacity(self.pools.len());
 
         for pool in self.pools.iter() {
-            futures.push(pool.local_storage_info())
+            futures.push(pool.local_storage_info_snapshot())
         }
 
         let results = join_all(futures).await;
@@ -776,14 +777,14 @@ impl ECStore {
             warn!("Local storage info deduplication: {} -> {}", original_count, disks.len());
         }
 
-        let backend = self.backend_info().await;
+        let backend = StorageAdminApi::backend_info(self).await;
         rustfs_madmin::StorageInfo { backend, disks }
     }
 
     #[instrument(skip(self))]
     pub(super) async fn handle_get_disks(&self, pool_idx: usize, set_idx: usize) -> Result<Vec<Option<DiskStore>>> {
         if pool_idx < self.pools.len() && set_idx < self.pools[pool_idx].disk_set.len() {
-            self.pools[pool_idx].disk_set[set_idx].get_disks(0, 0).await
+            Ok(self.pools[pool_idx].disk_set[set_idx].disk_inventory().await)
         } else {
             Err(rebalance_disk_set_lookup_error(pool_idx, set_idx, self.pools.len()))
         }

docs/architecture/migration-progress.md

@@ -5,17 +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-admin-observability-storage-reads`
+- Branch: `overtrue/arch-storage-admin-read-cleanup`
-- Baseline: `origin/main` at `8ae0cad6671562c0fffe56a7f288cd97fb87309d`
+- Baseline: `origin/main` at `94c53af264b8d011b19b0b35eed5990a21592d70`
 - PR type for this branch: `consumer-migration`
 - Runtime behavior changes: none.
-- Rust code changes: migrate grouped observability, RPC health, server-info,
+- Rust code changes: route ECStore internal admin-read aggregation through
-  realtime metrics, and notification read-side consumers from old
+  crate-internal `Sets`/`SetDisks` snapshot helpers and the inventory-facing
-  `StorageAPI::{backend_info, storage_info, local_storage_info}` trait imports
+  `StorageAdminApi` contract while preserving the old `StorageAPI` compatibility
-  to the inventory-facing `StorageAdminApi` contract.
+  surface.
 - CI/script changes: none.
-- Docs changes: record API-007 expanded read-side consumer-migration context,
+- Docs changes: record API-007 internal admin-read cleanup context, verification
-  verification evidence, and expert review outcomes.
+  evidence, and expert review outcomes.
 
 ## Phase 0 Tasks
 
@@ -205,21 +205,27 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
   - 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
+  - Completed fifth slice: `rustfs/rustfs#3335` migrated grouped observability,
-    consumers: obs cluster/disk/config/erasure-set metrics, RPC local storage
+    RPC health, server-info, realtime metrics, and notification read-side
-    info response construction, ECStore server-info local disk/backend reads,
+    consumers.
-    realtime disk metrics, and notification storage-info aggregation.
+  - Current branch slice: add crate-internal admin snapshot helpers for
-  - Acceptance: these consumers no longer import old `StorageAPI` only to read
+    `Sets`/`SetDisks`, then migrate ECStore internal decommission space,
-    admin storage information; peer RPC client calls remain unchanged.
+    local-storage-info, backend-info, drive-count, and disk-inventory admin
+    handlers away from old `StorageAPI` method calls.
+  - Acceptance: ECStore internal admin-read aggregation no longer relies on old
+    `StorageAPI` method calls where crate-internal helpers or
+    `StorageAdminApi` already represent the same read-only contract.
   - Must preserve: old `StorageAPI` trait shape, `StorageAPI::get_disks`
-    behavior, obs metric values, RPC msgpack map encoding and response shape,
+    behavior, storage-info disk aggregation, local-only disk filtering,
-    server-info shape, realtime metric shape, notification peer
+    decommission pool space calculation, storage-info deduplication, backend
-    aggregation/cache fallback, heal/scanner consumers, object paths,
+    info construction, object/rebalance selection paths, scanner/heal
-    replication/config persistence, and storage hot paths.
+    consumers, object paths, replication/config persistence, and storage hot
-  - Risk defense: group only read-side callers that delegate to the existing
+    paths.
-    ECStore admin info implementation; do not migrate object APIs, scanner,
+  - Risk defense: keep the old trait implementation as a delegating
-    heal, replication, config persistence, or storage implementation internals
+    compatibility surface, avoid implementing the full admin contract for
-    in this PR.
+    partial internal types, and do not migrate object APIs, scanner, heal,
+    replication, config persistence, or storage implementation hot paths in
+    this PR.
 
 ## Phase 8 Background Controller Tasks
 
@@ -263,21 +269,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
 
 | Expert | Status | Notes |
 |---|---|---|
-| 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. |
+| Quality/architecture | pass | Confirmed the diff stays limited to ECStore internal admin-read cleanup plus migration notes; helper visibility and naming are scoped, and the Handoff Notes correctly exclude object/hot-path `get_disks` consumers. |
-| 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. |
+| Migration preservation | pass | Confirmed old `StorageAPI` shape remains, `Sets`/`SetDisks` helpers preserve previous aggregation/filtering/get-disks logic, and decommission/local-info/admin inventory call sites only change entry point. |
-| 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. |
+| Testing/verification | pass | Confirmed focused ECStore checks, migration guards, diff hygiene, and added-line Rust quality scan are sufficient for this equivalent internal call-path cleanup while skipping full pre-commit under the current instruction. |
 
 ## Verification Notes
 
 Passed:
 - `cargo fmt --all`.
 - `cargo fmt --all --check`.
-- `cargo check -p rustfs-storage-api -p rustfs-ecstore -p rustfs-obs -p rustfs --lib`.
+- `cargo check -p rustfs-ecstore`.
-- `cargo test -p rustfs-obs stats_collector --lib`; 14 passed.
+- `cargo test -p rustfs-ecstore store::rebalance --lib`; 19 passed.
-- `cargo test -p rustfs-ecstore admin_server_info --lib`; 1 passed.
+- `cargo test -p rustfs-ecstore pools --lib`; 141 passed.
-- `cargo test -p rustfs-ecstore metrics_realtime --lib`; 5 passed.
+- `cargo test -p rustfs-ecstore set_disk --lib`; 86 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`.
@@ -288,25 +292,26 @@ Passed:
   println/eprintln, and `Ordering::Relaxed`.
 
 Notes:
-- 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
+  plus pre-existing casts and relaxed atomics in touched ECStore files; the
-  found no new risky code patterns.
+  added-line scan found no new risky code patterns.
-- No ECStore handler implementation, old `StorageAPI` trait, peer RPC client,
+- Old `StorageAPI` trait shape and implementations remain in place; `Sets` and
-  heal/scanner consumer, object path, replication/config persistence path, or
+  `SetDisks` delegate the admin-read subset to crate-internal helpers.
-  storage hot path is changed.
+- Object/rebalance selection paths, scanner/heal consumers, object APIs,
+  replication/config persistence paths, and storage hot paths are unchanged.
 - No temporary compatibility shim was added.
 
 ## Handoff Notes
 
-- Keep this API-007 slice as a grouped observability/health/server-info
+- Keep this API-007 slice as an ECStore-internal admin-read cleanup
-  read-side `consumer-migration` PR.
+  `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 remove `StorageAPI::get_disks` or route object/hot-path consumers
+  around it in this PR; only the ECStore internal admin disk-inventory handler
+  is in scope.
 - Do not make the old `StorageAPI` trait inherit `StorageAdminApi` in this PR.
 - Do not add temporary compatibility code unless a matching
   `RUSTFS_COMPAT_TODO(<task-id>)` marker and cleanup-register entry are added.