diff --git a/Cargo.lock b/Cargo.lock index 6709dae4b..4cf3733b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9991,9 +9991,11 @@ dependencies = [ name = "rustfs-storage-api" version = "1.0.0-beta.8" dependencies = [ + "async-trait", "serde", "serde_json", "time", + "tokio", ] [[package]] diff --git a/crates/storage-api/Cargo.toml b/crates/storage-api/Cargo.toml index a2ef260d6..5cf495276 100644 --- a/crates/storage-api/Cargo.toml +++ b/crates/storage-api/Cargo.toml @@ -28,11 +28,13 @@ categories = ["web-programming", "development-tools", "filesystem"] doctest = false [dependencies] +async-trait.workspace = true serde.workspace = true time.workspace = true [dev-dependencies] serde_json.workspace = true +tokio = { workspace = true, features = ["macros", "rt"] } [lints] workspace = true diff --git a/crates/storage-api/src/admin.rs b/crates/storage-api/src/admin.rs new file mode 100644 index 000000000..2f7c533f0 --- /dev/null +++ b/crates/storage-api/src/admin.rs @@ -0,0 +1,129 @@ +// 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. + +use std::fmt::Debug; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct DiskSetSelector { + pub pool_idx: usize, + pub set_idx: usize, +} + +impl DiskSetSelector { + pub const fn new(pool_idx: usize, set_idx: usize) -> Self { + Self { pool_idx, set_idx } + } +} + +#[async_trait::async_trait] +pub trait StorageAdminApi: Send + Sync + Debug { + type BackendInfo: Send + Sync + Debug + 'static; + type StorageInfo: Send + Sync + Debug + 'static; + type Disk: Send + Sync + Debug + 'static; + type Error: std::error::Error + Send + Sync + 'static; + + async fn backend_info(&self) -> Self::BackendInfo; + async fn storage_info(&self) -> Self::StorageInfo; + async fn local_storage_info(&self) -> Self::StorageInfo; + async fn disk_set_inventory(&self, selector: DiskSetSelector) -> Result>, Self::Error>; + fn set_drive_counts(&self) -> Vec; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(Debug)] + struct TestError; + + impl std::fmt::Display for TestError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("test storage admin error") + } + } + + impl std::error::Error for TestError {} + + #[derive(Debug, Clone, PartialEq, Eq)] + struct TestBackendInfo { + drives_per_set: Vec, + } + + #[derive(Debug, Clone, PartialEq, Eq)] + struct TestStorageInfo { + disk_count: usize, + } + + #[derive(Debug, Clone, PartialEq, Eq)] + struct TestDisk { + uuid: String, + disk_index: usize, + } + + #[derive(Debug)] + struct FakeStorageAdmin; + + #[async_trait::async_trait] + impl StorageAdminApi for FakeStorageAdmin { + type BackendInfo = TestBackendInfo; + type StorageInfo = TestStorageInfo; + type Disk = TestDisk; + type Error = TestError; + + async fn backend_info(&self) -> Self::BackendInfo { + TestBackendInfo { drives_per_set: vec![2] } + } + + async fn storage_info(&self) -> Self::StorageInfo { + TestStorageInfo { disk_count: 2 } + } + + async fn local_storage_info(&self) -> Self::StorageInfo { + self.storage_info().await + } + + async fn disk_set_inventory(&self, selector: DiskSetSelector) -> Result>, Self::Error> { + assert_eq!(selector, DiskSetSelector::new(0, 0)); + Ok(vec![Some(disk("disk-0", 0)), None]) + } + + fn set_drive_counts(&self) -> Vec { + vec![2] + } + } + + fn disk(uuid: &str, disk_index: usize) -> TestDisk { + TestDisk { + uuid: uuid.to_owned(), + disk_index, + } + } + + #[tokio::test] + async fn fake_storage_admin_api_exposes_disk_inventory_contract() { + let api = FakeStorageAdmin; + + let disks = api + .disk_set_inventory(DiskSetSelector::new(0, 0)) + .await + .expect("fake disk inventory should be available"); + + assert_eq!(api.backend_info().await.drives_per_set, vec![2]); + assert_eq!(api.storage_info().await.disk_count, 2); + assert_eq!(api.set_drive_counts(), vec![2]); + assert_eq!(disks.len(), 2); + assert_eq!(disks[0].as_ref().map(|disk| disk.uuid.as_str()), Some("disk-0")); + assert!(disks[1].is_none()); + } +} diff --git a/crates/storage-api/src/lib.rs b/crates/storage-api/src/lib.rs index 8b36b1420..5de9b2b37 100644 --- a/crates/storage-api/src/lib.rs +++ b/crates/storage-api/src/lib.rs @@ -14,8 +14,10 @@ //! Storage API contracts for RustFS. +pub mod admin; pub mod bucket; pub mod error; +pub use admin::{DiskSetSelector, StorageAdminApi}; pub use bucket::{BucketInfo, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp}; pub use error::{StorageErrorCode, StorageResult}; diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index a63148485..6b9e333c4 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,16 +5,15 @@ 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-storage-api-bucket-dtos` -- Baseline: `upstream/main` at `5fef10548477d9d25b0d391874f8280bf259d10e` -- PR type for this branch: `api-extraction` +- Branch: `overtrue/arch-storage-api-disk-inventory` +- Baseline: `upstream/main` at `3fed21c68ae858f4bbb7f1be375d73b2af522f02` +- PR type for this branch: `contract` - Runtime behavior changes: none. -- Rust code changes: move the pure bucket/options DTO subset from - `rustfs-ecstore` into `rustfs-storage-api`, while preserving old - `ecstore::store_api` import paths through a temporary compatibility - re-export. +- Rust code changes: add a pure `rustfs-storage-api` admin/disk inventory + contract trait. - CI/script changes: none -- Docs changes: record API-003 bucket DTO compatibility cleanup. +- Docs changes: record API-006 branch context, verification evidence, and + expert review outcomes. ## Phase 0 Tasks @@ -167,8 +166,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block quorum classification, and reserved code gaps `0x2B/0x2C`. - Risk defense: no storage hot-path enum move in this PR; only numeric code mapping uses the new contract. -- [~] `API-003` Move DTOs. - - Current branch: move the pure bucket/options DTO subset: +- [x] `API-003` Move DTOs. + - Current PR: `rustfs/rustfs#3314` merged. + - Completed slice: move the pure bucket/options DTO subset: `MakeBucketOptions`, `SRBucketDeleteOp`, `DeleteBucketOptions`, `BucketOptions`, and `BucketInfo`. - Acceptance: `rustfs-storage-api` exports these DTOs, ECStore re-exports @@ -177,6 +177,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Must preserve: no `ObjectOptions`, `ObjectInfo`, reader, compression, encryption, filemeta conversion, multipart conversion, route, storage, or runtime behavior changes in this PR. +- [~] `API-006` Add disk inventory/admin trait. + - Acceptance: `StorageAdminApi` exposes backend info, global storage info, + local storage info, disk-set inventory, and drive-count surfaces without + depending on ECStore implementation types. + - Must preserve: no `StorageAPI::get_disks` removal, no ECStore implementation + change, no admin/readiness/capacity behavior change. + - Risk defense: use associated types for backend/storage/disk DTOs so this + contract slice does not pull `rustfs-madmin` or `rustfs-ecstore` into + `rustfs-storage-api`. + - Verification: focused storage-api tests, dependency tree, migration guards, + formatting, and diff hygiene. ## Phase 8 Background Controller Tasks @@ -201,12 +212,12 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Next PRs -1. `api-extraction`: continue API-003 with the next pure DTO subset only after - the bucket/options compatibility re-export is reviewed. -2. `contract`: wait for API-002/#3313 before adding error-aware storage API - traits. -3. `test-only`: add focused preservation tests before moving scanner, heal, - replication, lifecycle, or disk health workers. +1. `contract`: bind ECStore to `StorageAdminApi` while keeping + `StorageAPI::get_disks` available. +2. `consumer-migration`: migrate readiness/admin/capacity consumers to the + inventory-facing admin contract one group at a time. +3. `dependency-migration`: remove duplicate old-path admin surfaces only after + consumer migration proves equivalent behavior. 4. `api-extraction`: move only the pure server-config model into rustfs-config as CFG-003. 5. `api-extraction`: keep the old rustfs_ecstore::config::* path with @@ -222,58 +233,40 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | Only pure bucket/options DTOs moved into `rustfs-storage-api`; object, reader, compression, encryption, filemeta, multipart, storage, and runtime logic stayed in ECStore. | -| Migration preservation | pass | Old `ecstore::store_api` import paths remain through `RUSTFS_COMPAT_TODO(API-003)` compatibility re-export, with cleanup registered. | -| Testing/verification | pass | Focused DTO tests, ECStore compatibility test, migration guards, formatting, rio-v2 clippy, dependency review, diff checks, and pre-commit passed. | -| Quality/architecture | pass | Single `docs-only` PR; ADR chooses existing rustfs-config, records module path and dependency boundaries, and avoids a speculative new crate. | -| Migration preservation | pass | No code movement; ADR explicitly keeps persistence helpers, global server-config state, startup order, and old-path compatibility requirements out of CFG-002. | -| Testing/verification | pass | Docs-only verification uses migration guard scripts, metrics reference guard, layer dependency guard, and whitespace checks. | -| Quality/architecture | pass | Single `docs-only` PR; the inventory is isolated under `docs/architecture`, uses existing KMS source files as evidence, and introduces no new abstraction or dependency edge. | -| Migration preservation | pass | No runtime code, config persistence, admin authorization, startup order, storage path, global state, or crate boundary changes are made. | -| Testing/verification | pass | Docs-only verification is bounded to diff review, architecture migration rules, metrics reference guard, layer dependency guard, and whitespace checks. | +| Quality/architecture | pass | `StorageAdminApi` remains contract-only and introduces no `storage-api -> ecstore` or `storage-api -> madmin` coupling. | +| Migration preservation | pass | No ECStore implementation, `StorageAPI::get_disks`, readiness, admin, capacity, or storage hot-path behavior changed. | +| Testing/verification | pass | Focused tests, dependency tree, migration guards, Rust quality scan, and full pre-commit passed before push. | ## Verification Notes Passed: - - `cargo test -p rustfs-storage-api` -- `cargo test -p rustfs-ecstore --test storage_api_compat_test` +- `cargo check -p rustfs-storage-api` - `cargo check -p rustfs-storage-api -p rustfs-ecstore` -- `cargo check -p rustfs-ecstore --features rio-v2` -- `cargo clippy -p rustfs-ecstore --features rio-v2 --all-targets -- -D warnings` +- `cargo tree -p rustfs-storage-api --edges normal` - `cargo fmt --all` - `cargo fmt --all --check` -- `cargo test -p rustfs-ecstore error -- --nocapture` - `./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` -- `cargo tree -p rustfs-storage-api --edges normal` +- Rust quality scan on changed Rust files. - `make NUM_CORES=1 pre-commit` Notes: -- Plain `make pre-commit` runs `fmt` and `unsafe-code-check` concurrently via - global Makefile parallelism; `unsafe-code-check` passes standalone, and the - full pre-commit target passed when run serially with `NUM_CORES=1`. -- Full nextest in pre-commit: 5704 passed, 111 skipped. +- Normal `rustfs-storage-api` dependencies added by this branch are limited to + `async-trait`. +- `tokio` is a dev-dependency only for the unit test runtime. +- No ECStore implementation, consumer import, admin route, readiness path, + capacity calculation, or storage hot path is changed. +- Full nextest in pre-commit: 5740 passed, 111 skipped. - Workspace doctests passed. ## Handoff Notes -- Keep this API-003 branch as a focused `api-extraction` PR for bucket/options - DTOs only. -- Do not move `ObjectOptions`, `ObjectInfo`, `CompletePart`, reader types, - compression/encryption helpers, filemeta conversions, S3 DTO conversions, or - storage traits in this PR. -- Keep the old `ecstore::store_api` compatibility re-export until all consumers - import bucket DTOs from `rustfs_storage_api`. -- Do not add temporary compatibility code without a matching - `RUSTFS_COMPAT_TODO()` marker and cleanup-register entry. -- KMS production default hardening remains a separate task group; do not bundle - it with this inventory PR. -- The CFG-003 extraction PR must preserve the tuple-struct shape, serde fields, - `hiddenIfEmpty` alias, `Config::new` default behavior, marshal/unmarshal - behavior, and old `rustfs_ecstore::config::*` path. -- Do not create a new config-model crate unless a later implementation attempt - proves `rustfs-config` cannot hold the pure model boundary. +- Keep this API-006 branch as a focused `contract` PR. +- Do not implement the trait for ECStore in this PR. +- Do not remove or route around `StorageAPI::get_disks` in this PR. +- Do not add temporary compatibility code unless a matching + `RUSTFS_COMPAT_TODO()` marker and cleanup-register entry are added.