mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 08:18:18 +00:00
refactor(admin): use storage admin drive counts (#3332)
This commit is contained in:
Generated
+1
@@ -9107,6 +9107,7 @@ dependencies = [
|
||||
"rustfs-scanner",
|
||||
"rustfs-security-governance",
|
||||
"rustfs-signer",
|
||||
"rustfs-storage-api",
|
||||
"rustfs-targets",
|
||||
"rustfs-tls-runtime",
|
||||
"rustfs-trusted-proxies",
|
||||
|
||||
@@ -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-storage-admin-api-bind-ecstore`
|
||||
- Baseline: `origin/main` at `d9ddd1bedc3ee8d048511511e310018e2326f913`
|
||||
- PR type for this branch: `contract`
|
||||
- Branch: `overtrue/arch-storage-admin-consumers`
|
||||
- Baseline: `origin/main` at `4b1714438e13afd2b44872bf45fbe084065c9d18`
|
||||
- PR type for this branch: `consumer-migration`
|
||||
- Runtime behavior changes: none.
|
||||
- Rust code changes: implement the pure `rustfs-storage-api`
|
||||
`StorageAdminApi` contract for `ECStore` by delegating to existing ECStore
|
||||
handlers.
|
||||
- Rust code changes: migrate the admin storage-class config consumer from the
|
||||
old `StorageAPI::set_drive_counts` import to the inventory-facing
|
||||
`StorageAdminApi::set_drive_counts` contract.
|
||||
- CI/script changes: none
|
||||
- Docs changes: record API-007 first-slice branch context, verification
|
||||
evidence, and expert review outcomes.
|
||||
- Docs changes: record API-007 admin config consumer-migration context,
|
||||
verification evidence, and expert review outcomes.
|
||||
|
||||
## Phase 0 Tasks
|
||||
|
||||
@@ -193,16 +193,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
- Verification: focused storage-api tests, dependency tree, migration guards,
|
||||
formatting, and diff hygiene.
|
||||
- [~] `API-007` Dual-route `get_disks` consumers.
|
||||
- Current branch first slice: bind `ECStore` to `StorageAdminApi` while
|
||||
keeping `StorageAPI::get_disks` and all existing consumers unchanged.
|
||||
- Acceptance: `ECStore` satisfies the new admin contract with ECStore-owned
|
||||
associated types and a compile-bound compatibility test.
|
||||
- Completed first slice: `rustfs/rustfs#3331` bound `ECStore` to
|
||||
`StorageAdminApi` while keeping all consumers unchanged.
|
||||
- Current branch slice: migrate the admin storage-class config consumer to
|
||||
`StorageAdminApi::set_drive_counts`.
|
||||
- Acceptance: `rustfs/src/admin/service/config.rs` no longer imports old
|
||||
`StorageAPI` only to read drive-count inventory, and `rustfs` owns an
|
||||
explicit dependency on the contract crate.
|
||||
- Must preserve: old `StorageAPI` trait shape, `StorageAPI::get_disks`
|
||||
behavior, readiness/admin/capacity/heal/scanner consumers, and storage hot
|
||||
paths.
|
||||
- Risk defense: do not make `StorageAPI` inherit `StorageAdminApi` in this
|
||||
slice, because `Sets`, `SetDisks`, and test implementations still own the
|
||||
old trait surface.
|
||||
behavior, storage-class parsing/default semantics, admin config validation,
|
||||
readiness/capacity/heal/scanner consumers, and storage hot paths.
|
||||
- Risk defense: keep this slice to the admin config drive-count consumer;
|
||||
do not migrate `backend_info`, `local_storage_info`, readiness, heal,
|
||||
scanner, or RPC consumers in the same PR.
|
||||
|
||||
## Phase 8 Background Controller Tasks
|
||||
|
||||
@@ -227,8 +230,8 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
|
||||
## Next PRs
|
||||
|
||||
1. `consumer-migration`: migrate readiness/admin/capacity consumers to the
|
||||
inventory-facing admin contract one group at a time.
|
||||
1. `consumer-migration`: migrate the remaining readiness/admin/capacity
|
||||
consumers to the inventory-facing admin contract one group at a time.
|
||||
2. `dependency-migration`: remove duplicate old-path admin surfaces only after
|
||||
consumer migration proves equivalent behavior.
|
||||
3. `api-extraction`: move only the pure server-config model into
|
||||
@@ -246,21 +249,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
|
||||
| Expert | Status | Notes |
|
||||
|---|---|---|
|
||||
| Quality/architecture | pass | Confirmed the branch only adds ECStore's `StorageAdminApi` binding, keeps `rustfs-storage-api` independent from ECStore and madmin, and does not make old `StorageAPI` inherit the new trait. |
|
||||
| Migration preservation | pass | Confirmed old `StorageAPI`, `StorageAPI::get_disks`, all consumers, and storage hot paths remain unchanged; no compatibility shim was added. |
|
||||
| Testing/verification | pass | Confirmed the compile-bound compatibility test covers `ECStore: StorageAdminApi`, the clippy helper issue was fixed without behavior changes, and focused checks plus full pre-commit passed. |
|
||||
| Quality/architecture | pass | Confirmed this remains a narrow admin config consumer-migration slice, with an explicit `rustfs` dependency on the contract crate and no ECStore or storage-api boundary reshaping. |
|
||||
| Migration preservation | pass | Confirmed storage-class parsing/default behavior, old `StorageAPI`, `StorageAPI::get_disks`, ECStore handlers, and storage hot paths remain unchanged. |
|
||||
| Testing/verification | pass | Confirmed focused admin config tests, dependency guards, migration guards, formatting, diff hygiene, and full pre-commit evidence passed before push. |
|
||||
|
||||
## Verification Notes
|
||||
|
||||
Passed:
|
||||
- baseline `cargo check -p rustfs-storage-api -p rustfs-ecstore`
|
||||
- `cargo test -p rustfs-ecstore --test storage_api_compat_test`
|
||||
- `cargo test -p rustfs-storage-api`
|
||||
- `cargo check -p rustfs-storage-api`
|
||||
- `cargo check -p rustfs-storage-api -p rustfs-ecstore`
|
||||
- `cargo tree -p rustfs-storage-api --edges normal`
|
||||
- `cargo tree -p rustfs-ecstore --edges normal`
|
||||
- `cargo clippy -p rustfs-ecstore --test storage_api_compat_test --all-targets -- -D warnings`
|
||||
- `cargo check -p rustfs --lib`
|
||||
- `cargo check -p rustfs-storage-api -p rustfs-ecstore -p rustfs --lib`
|
||||
- `cargo test -p rustfs admin::service::config --lib`
|
||||
- `cargo tree -p rustfs --edges normal`
|
||||
- `cargo fmt --all`
|
||||
- `cargo fmt --all --check`
|
||||
- `./scripts/check_architecture_migration_rules.sh`
|
||||
@@ -268,23 +267,26 @@ Passed:
|
||||
- `./scripts/check_metrics_migration_refs.sh`
|
||||
- `./scripts/check_unsafe_code_allowances.sh`
|
||||
- `git diff --check`
|
||||
- Rust quality scan on changed Rust files.
|
||||
- `make NUM_CORES=1 pre-commit`
|
||||
|
||||
Notes:
|
||||
- This branch adds no new dependency to `rustfs-storage-api`.
|
||||
- ECStore already depends on `rustfs-storage-api`; this branch uses that
|
||||
existing dependency to implement the new contract.
|
||||
- No existing ECStore handler, consumer import, admin route, readiness path,
|
||||
capacity calculation, or storage hot path is changed.
|
||||
- Full pre-commit passed with nextest `5745 passed, 111 skipped`; workspace
|
||||
- This branch adds a direct `rustfs` dependency on `rustfs-storage-api` so the
|
||||
admin config consumer can call the contract crate instead of importing the
|
||||
old ECStore `StorageAPI` trait.
|
||||
- `./scripts/check_layer_dependencies.sh` passes with the new dependency.
|
||||
- Only the admin storage-class config drive-count consumer changed.
|
||||
- No existing ECStore handler, admin route, readiness path, capacity
|
||||
calculation, heal/scanner consumer, RPC consumer, or storage hot path is
|
||||
changed.
|
||||
- Full pre-commit passed with nextest `5757 passed, 111 skipped`; workspace
|
||||
doctests passed.
|
||||
- No temporary compatibility shim was added.
|
||||
|
||||
## Handoff Notes
|
||||
|
||||
- Keep this API-007 first-slice branch as a focused `contract` PR.
|
||||
- Do not migrate readiness, admin, capacity, heal, scanner, or RPC consumers in
|
||||
this PR.
|
||||
- Keep this API-007 slice as a focused `consumer-migration` PR.
|
||||
- Do not migrate account-info `backend_info`, RPC `local_storage_info`,
|
||||
readiness, capacity, heal, or scanner 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.
|
||||
- Do not add temporary compatibility code unless a matching
|
||||
|
||||
@@ -87,6 +87,7 @@ rustfs-data-usage = { workspace = true }
|
||||
rustfs-s3select-api = { workspace = true }
|
||||
rustfs-s3select-query = { workspace = true }
|
||||
rustfs-targets = { workspace = true }
|
||||
rustfs-storage-api = { workspace = true }
|
||||
rustfs-tls-runtime = { workspace = true }
|
||||
rustfs-trusted-proxies = { workspace = true }
|
||||
rustfs-utils = { workspace = true, features = ["full"] }
|
||||
|
||||
@@ -19,13 +19,13 @@ use rustfs_config::oidc::IDENTITY_OPENID_SUB_SYS;
|
||||
use rustfs_config::{AUDIT_DEFAULT_DIR, EVENT_DEFAULT_DIR};
|
||||
use rustfs_config::{DEFAULT_DELIMITER, ENABLE_KEY, EnableState};
|
||||
use rustfs_config::{HEAL_SUB_SYS, SCANNER_SUB_SYS};
|
||||
use rustfs_ecstore::StorageAPI;
|
||||
use rustfs_ecstore::config::com::{STORAGE_CLASS_SUB_SYS, read_config_without_migrate};
|
||||
use rustfs_ecstore::config::storageclass;
|
||||
use rustfs_ecstore::config::{Config as ServerConfig, KVS, set_global_server_config, set_global_storage_class};
|
||||
use rustfs_ecstore::new_object_layer_fn;
|
||||
use rustfs_ecstore::notification_sys::get_global_notification_sys;
|
||||
use rustfs_iam::oidc::load_oidc_provider_configs_from_server_config;
|
||||
use rustfs_storage_api::StorageAdminApi;
|
||||
use rustfs_targets::config::{
|
||||
validate_amqp_config, validate_kafka_config, validate_mqtt_config, validate_mysql_config, validate_nats_config,
|
||||
validate_postgres_config, validate_pulsar_config, validate_redis_config, validate_webhook_config,
|
||||
@@ -55,7 +55,10 @@ async fn apply_storage_class_runtime_config(config: &ServerConfig) -> S3Result<(
|
||||
};
|
||||
|
||||
let kvs = config.get_value(STORAGE_CLASS_SUB_SYS, DEFAULT_DELIMITER).unwrap_or_default();
|
||||
let set_drive_count = store.set_drive_counts().into_iter().next().unwrap_or(1);
|
||||
let set_drive_count = StorageAdminApi::set_drive_counts(store.as_ref())
|
||||
.into_iter()
|
||||
.next()
|
||||
.unwrap_or(1);
|
||||
let parsed = storageclass::lookup_config(&kvs, set_drive_count)
|
||||
.map_err(|err| internal_error(format!("failed to apply storage class config: {err}")))?;
|
||||
set_global_storage_class(parsed);
|
||||
@@ -77,7 +80,7 @@ async fn validate_storage_class_config(config: &ServerConfig) -> S3Result<()> {
|
||||
};
|
||||
|
||||
let kvs = config.get_value(STORAGE_CLASS_SUB_SYS, DEFAULT_DELIMITER).unwrap_or_default();
|
||||
let set_drive_counts = store.set_drive_counts();
|
||||
let set_drive_counts = StorageAdminApi::set_drive_counts(store.as_ref());
|
||||
if set_drive_counts.is_empty() {
|
||||
return validate_storage_class_kvs(&kvs, &[1]);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user