mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-28 17:18:58 +00:00
ci: guard migration loss-prevention coverage (#3475)
This commit is contained in:
@@ -819,13 +819,23 @@ impl TransitionState {
|
||||
})
|
||||
}
|
||||
|
||||
fn schedule_bucket_compensation(self: &Arc<Self>, bucket: &str) -> bool {
|
||||
let mut scheduled = self.compensation_buckets.lock().unwrap();
|
||||
if !scheduled.insert(bucket.to_string()) {
|
||||
fn reserve_bucket_compensation(&self, bucket: &str) -> bool {
|
||||
let inserted = match self.compensation_buckets.lock() {
|
||||
Ok(mut scheduled) => scheduled.insert(bucket.to_string()),
|
||||
Err(poisoned) => poisoned.into_inner().insert(bucket.to_string()),
|
||||
};
|
||||
if !inserted {
|
||||
return false;
|
||||
}
|
||||
Self::inc_counter(&self.compensation_scheduled_tasks);
|
||||
self.record_scanner_transition_state();
|
||||
true
|
||||
}
|
||||
|
||||
fn schedule_bucket_compensation(self: &Arc<Self>, bucket: &str) -> bool {
|
||||
if !self.reserve_bucket_compensation(bucket) {
|
||||
return false;
|
||||
}
|
||||
let bucket = bucket.to_string();
|
||||
let scheduled = Arc::clone(&self.compensation_buckets);
|
||||
let state = Arc::clone(self);
|
||||
@@ -3158,16 +3168,17 @@ mod tests {
|
||||
});
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn schedule_bucket_compensation_deduplicates_same_bucket() {
|
||||
#[test]
|
||||
fn reserve_bucket_compensation_deduplicates_same_bucket() {
|
||||
let state = TransitionState::new_with_capacity(1);
|
||||
|
||||
let first = state.schedule_bucket_compensation("bucket-a");
|
||||
let second = state.schedule_bucket_compensation("bucket-a");
|
||||
let first = state.reserve_bucket_compensation("bucket-a");
|
||||
let second = state.reserve_bucket_compensation("bucket-a");
|
||||
|
||||
assert!(first);
|
||||
assert!(!second);
|
||||
assert_eq!(state.compensation_scheduled_tasks(), 1);
|
||||
assert_eq!(state.compensation_pending_tasks(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
|
||||
@@ -82,3 +82,28 @@ The current decision is recorded in
|
||||
global server-config state in `ecstore`, and preserve the old
|
||||
`rustfs_ecstore::config::*` path with a temporary compatibility marker during
|
||||
the first extraction.
|
||||
|
||||
## Loss-Prevention Coverage
|
||||
|
||||
Architecture migration checks must keep public contract re-exports and storage
|
||||
trait coverage from silently drifting during cleanup PRs.
|
||||
|
||||
Required `rustfs-storage-api` public re-exports:
|
||||
|
||||
- `pub use admin::{DiskSetSelector, StorageAdminApi};`
|
||||
- `pub use bucket::{BucketInfo, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp};`
|
||||
- `pub use error::{StorageErrorCode, StorageResult};`
|
||||
|
||||
Required `StorageAPI` operation groups:
|
||||
|
||||
- `ObjectIO`
|
||||
- `BucketOperations`
|
||||
- `ObjectOperations`
|
||||
- `ListOperations`
|
||||
- `MultipartOperations`
|
||||
- `HealOperations`
|
||||
- `Debug`
|
||||
|
||||
`NamespaceLocking` must remain a separate operation group from the full
|
||||
`StorageAPI` facade. ECStore must keep compile-time coverage for both
|
||||
`StorageAdminApi` and `StorageAPI + NamespaceLocking`.
|
||||
|
||||
@@ -5,18 +5,18 @@ 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-startup-command-bootstrap`
|
||||
- Baseline: `origin/main` at `941b8afee8619c6a038c20aac51565a65c959ee4`
|
||||
- PR type for this branch: `pure-move`
|
||||
- Runtime behavior changes: no external behavior change expected; the binary
|
||||
entrypoint still builds the Tokio runtime, applies preflight, dispatches
|
||||
non-server commands, and starts server/bootstrap stages in the same order.
|
||||
- Rust code changes: add `startup_entrypoint::run_process`, move remaining
|
||||
command dispatch, server bootstrap orchestration, and fatal stderr helpers
|
||||
out of `main.rs`, and keep `main.rs` as the process entry shim.
|
||||
- CI/script changes: none.
|
||||
- Docs changes: record `R-019` startup command/bootstrap entrypoint progress
|
||||
and update startup timeline ownership.
|
||||
- Branch: `overtrue/arch-loss-prevention-guards`
|
||||
- Baseline: `origin/main` at `dd6b4c35ad2c24be432c32645ce17e2d312eac78`
|
||||
- PR type for this branch: `ci-gate`
|
||||
- Runtime behavior changes: no external behavior change expected.
|
||||
- Rust code changes: split lifecycle transition compensation bucket
|
||||
reservation from background backfill spawning so the dedupe unit test does
|
||||
not launch an actual compensation task on shared test state.
|
||||
- CI/script changes: extend `scripts/check_architecture_migration_rules.sh` to
|
||||
guard public storage-api re-exports, StorageAPI operation-group coverage,
|
||||
NamespaceLocking separation, and ECStore compatibility tests.
|
||||
- Docs changes: record `G-006` completion and document the guarded
|
||||
loss-prevention coverage in `crate-boundaries.md`.
|
||||
|
||||
## Phase 0 Tasks
|
||||
|
||||
@@ -33,12 +33,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
- [x] `G-005` Add dependency direction guard.
|
||||
- Acceptance: `./scripts/check_layer_dependencies.sh` passes on current
|
||||
`upstream/main` while still rejecting new unaccepted layer dependencies.
|
||||
- [~] `G-006` Create migration loss-prevention checks.
|
||||
- Current branch: add a mechanical admin route matrix guard from
|
||||
- [x] `G-006` Create migration loss-prevention checks.
|
||||
- Completed slices: add a mechanical admin route matrix guard from
|
||||
[`admin-route-action-snapshot.md`](admin-route-action-snapshot.md) and
|
||||
`rustfs/src/admin/route_registration_test.rs`.
|
||||
- Remaining follow-up: add checks for public re-export and storage trait
|
||||
coverage before pure moves.
|
||||
`rustfs/src/admin/route_registration_test.rs`; add migration rules for
|
||||
public storage-api re-export coverage, StorageAPI operation-group coverage,
|
||||
NamespaceLocking separation, and ECStore compatibility-test coverage.
|
||||
- Acceptance: architecture migration rules fail if the public storage-api
|
||||
contract re-export surface drifts, if `StorageAPI` stops covering the
|
||||
documented storage operation groups, if `NamespaceLocking` is folded back
|
||||
into the full storage facade, or if ECStore compile-time compatibility tests
|
||||
for these contracts are removed.
|
||||
- [x] `G-007` Create startup timeline table.
|
||||
- Acceptance: [`startup-timeline.md`](startup-timeline.md) records current
|
||||
binary startup order, side effects, fatal boundaries, and readiness stages.
|
||||
@@ -740,52 +745,51 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
|
||||
## Next PRs
|
||||
|
||||
1. `pure-move`: continue shrinking startup entrypoint orchestration only where a
|
||||
clean module boundary remains without changing fatal stderr behavior.
|
||||
2. `ci-gate`: finish `G-006` public re-export and storage trait coverage checks
|
||||
before the remaining cleanup slices.
|
||||
1. `pure-move`/`consumer-migration`: continue larger cleanup slices with the
|
||||
loss-prevention guards active for public re-exports and storage trait
|
||||
coverage.
|
||||
|
||||
## Pre-Push Review Log
|
||||
|
||||
| Expert | Status | Notes |
|
||||
|---|---|---|
|
||||
| Quality/architecture | passed | Pure-move slice removes command dispatch and bootstrap orchestration details from binary startup behind a dedicated startup entrypoint boundary. |
|
||||
| Migration preservation | passed | Command parse stderr, info/TLS dispatch, observability fatal sentinel, startup order, readiness publication, and shutdown ownership are preserved. |
|
||||
| Testing/verification | passed | Focused startup entrypoint and observability guardrail tests, compile checks, formatting, migration/layer guards, Rust risk scan, branch freshness check, and full `make pre-commit` passed. |
|
||||
| Quality/architecture | passed | CI-gate slice extends the existing migration rule script instead of adding a parallel guard system. |
|
||||
| Migration preservation | passed | Guards fail on public storage-api re-export drift, StorageAPI operation-group drift, NamespaceLocking collapse, or missing ECStore compatibility tests without changing runtime behavior. |
|
||||
| Testing/verification | passed | Script syntax, migration/layer guards, lifecycle compensation dedupe test, ECStore compatibility test, compile check, formatting, diff hygiene, and full `make pre-commit` passed. |
|
||||
|
||||
## Verification Notes
|
||||
|
||||
Passed on `941b8afee8619c6a038c20aac51565a65c959ee4`:
|
||||
Passed on `dd6b4c35ad2c24be432c32645ce17e2d312eac78`:
|
||||
|
||||
- `cargo test -p rustfs startup_entrypoint --no-fail-fast`: passed.
|
||||
- `cargo test -p rustfs-obs startup_ --no-fail-fast`: passed.
|
||||
- `cargo check -p rustfs --lib`: passed.
|
||||
- `cargo check -p rustfs --bin rustfs`: passed.
|
||||
- `cargo fmt --all --check`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- `bash -n scripts/check_architecture_migration_rules.sh`: passed.
|
||||
- `./scripts/check_architecture_migration_rules.sh`: passed.
|
||||
- `./scripts/check_layer_dependencies.sh`: passed.
|
||||
- Added-line and changed-file Rust risk scan: expected fatal-boundary
|
||||
`expect`/`eprintln!` only.
|
||||
- Full-file risk scan for changed Rust files: expected process exit and fatal
|
||||
stderr entries are isolated in `startup_entrypoint.rs`.
|
||||
- `make pre-commit`: passed, including nextest `6038` passed / `111` skipped
|
||||
and doctests.
|
||||
- `cargo test -p rustfs-ecstore --test storage_api_compat_test --no-fail-fast`:
|
||||
passed.
|
||||
- `cargo test -p rustfs-ecstore bucket::lifecycle::bucket_lifecycle_ops::tests::reserve_bucket_compensation_deduplicates_same_bucket --no-fail-fast`:
|
||||
passed.
|
||||
- `cargo check -p rustfs-ecstore`: passed.
|
||||
- `cargo fmt --all --check`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- Rust risk scan: no new production `unwrap`/`expect`, lossy casts, string
|
||||
errors, public boxed errors, or production `println`/`eprintln`.
|
||||
- `make pre-commit`: passed.
|
||||
- `git rev-list --left-right --count HEAD...origin/main`: returned `1 0`
|
||||
after commit.
|
||||
|
||||
Notes:
|
||||
|
||||
- This slice centralizes remaining command/bootstrap entrypoint orchestration
|
||||
without changing startup ordering, fatal stderr behavior, readiness ownership,
|
||||
shutdown token ownership, or shutdown handle ownership.
|
||||
- Ready publication, scanner start, and shutdown wait remain owned by
|
||||
`startup_services`; `startup_entrypoint` only orchestrates the existing
|
||||
startup sequence and process-level fatal handling.
|
||||
- This slice completes the `G-006` loss-prevention follow-up with script-level
|
||||
checks for the public re-export and storage trait coverage called out in the
|
||||
migration checklist.
|
||||
- The script reads existing source/test surfaces and does not introduce a new
|
||||
runtime behavior path.
|
||||
- The lifecycle compensation dedupe test now covers reservation directly,
|
||||
avoiding a spawned backfill task that can inherit shared object-store test
|
||||
state during the full pre-commit run.
|
||||
|
||||
## Handoff Notes
|
||||
|
||||
- R-019 is implemented, locally verified, and current with `origin/main`.
|
||||
- Next startup slices can keep using larger pure moves, but must keep startup
|
||||
ordering, fatal/non-fatal boundaries, shutdown ownership, readiness ownership,
|
||||
and fatal stderr behavior explicit in tests and review notes.
|
||||
- G-006 is implemented, locally verified, and current with `origin/main`.
|
||||
- Larger cleanup slices can now rely on migration rules to catch accidental
|
||||
public storage-api re-export drift and storage trait coverage loss.
|
||||
|
||||
@@ -21,6 +21,26 @@ contains_line() {
|
||||
grep -qxF "$needle" "$file"
|
||||
}
|
||||
|
||||
require_source_line() {
|
||||
local file="$1"
|
||||
local expected="$2"
|
||||
local description="$3"
|
||||
|
||||
if ! contains_line "$expected" "${ROOT_DIR}/${file}"; then
|
||||
report_failure "${description} missing exact source line in ${file}: ${expected}"
|
||||
fi
|
||||
}
|
||||
|
||||
require_source_contains() {
|
||||
local file="$1"
|
||||
local expected="$2"
|
||||
local description="$3"
|
||||
|
||||
if ! grep -qF "$expected" "${ROOT_DIR}/${file}"; then
|
||||
report_failure "${description} missing source text in ${file}: ${expected}"
|
||||
fi
|
||||
}
|
||||
|
||||
TMP_DIR="$(mktemp -d)"
|
||||
trap 'rm -rf "$TMP_DIR"' EXIT
|
||||
|
||||
@@ -29,6 +49,7 @@ PR_TYPE_HITS_FILE="${TMP_DIR}/pr_type_hits.txt"
|
||||
SOURCE_MARKERS_FILE="${TMP_DIR}/source_markers.txt"
|
||||
SOURCE_IDS_FILE="${TMP_DIR}/source_ids.txt"
|
||||
REGISTER_IDS_FILE="${TMP_DIR}/register_ids.txt"
|
||||
STORAGE_API_SUPERTRAITS_FILE="${TMP_DIR}/storage_api_supertraits.txt"
|
||||
|
||||
awk '
|
||||
/^## PR Types$/ {
|
||||
@@ -153,6 +174,56 @@ while IFS= read -r register_id; do
|
||||
fi
|
||||
done <"$REGISTER_IDS_FILE"
|
||||
|
||||
require_source_line \
|
||||
"crates/storage-api/src/lib.rs" \
|
||||
"pub use admin::{DiskSetSelector, StorageAdminApi};" \
|
||||
"storage-api public admin contract re-export"
|
||||
require_source_line \
|
||||
"crates/storage-api/src/lib.rs" \
|
||||
"pub use bucket::{BucketInfo, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp};" \
|
||||
"storage-api public bucket DTO re-export"
|
||||
require_source_line \
|
||||
"crates/storage-api/src/lib.rs" \
|
||||
"pub use error::{StorageErrorCode, StorageResult};" \
|
||||
"storage-api public error contract re-export"
|
||||
|
||||
perl -0ne '
|
||||
if (/pub trait StorageAPI:\s*(.*?)\s*\{\s*\}/s) {
|
||||
my $body = $1;
|
||||
while ($body =~ /\b([A-Z][A-Za-z0-9_]*)\b/g) {
|
||||
print "$1\n";
|
||||
}
|
||||
}
|
||||
' "${ROOT_DIR}/crates/ecstore/src/store_api/traits.rs" |
|
||||
sort -u >"$STORAGE_API_SUPERTRAITS_FILE"
|
||||
|
||||
if [[ ! -s "$STORAGE_API_SUPERTRAITS_FILE" ]]; then
|
||||
report_failure "StorageAPI supertrait declaration not found in crates/ecstore/src/store_api/traits.rs"
|
||||
fi
|
||||
|
||||
for required_trait in ObjectIO BucketOperations ObjectOperations ListOperations MultipartOperations HealOperations Debug; do
|
||||
if ! contains_line "$required_trait" "$STORAGE_API_SUPERTRAITS_FILE"; then
|
||||
report_failure "StorageAPI no longer covers required operation group ${required_trait}"
|
||||
fi
|
||||
done
|
||||
|
||||
if contains_line "NamespaceLocking" "$STORAGE_API_SUPERTRAITS_FILE"; then
|
||||
report_failure "NamespaceLocking must remain separate from the full StorageAPI facade"
|
||||
fi
|
||||
|
||||
require_source_contains \
|
||||
"crates/ecstore/src/store_api/traits.rs" \
|
||||
"pub trait NamespaceLocking: Send + Sync + Debug + 'static" \
|
||||
"separate namespace-locking operation-group trait"
|
||||
require_source_contains \
|
||||
"crates/ecstore/tests/storage_api_compat_test.rs" \
|
||||
"fn ecstore_implements_storage_admin_api_contract()" \
|
||||
"ECStore StorageAdminApi compile-time coverage test"
|
||||
require_source_contains \
|
||||
"crates/ecstore/tests/storage_api_compat_test.rs" \
|
||||
"fn ecstore_implements_storage_api_and_namespace_locking_contracts()" \
|
||||
"ECStore StorageAPI and NamespaceLocking compile-time coverage test"
|
||||
|
||||
if (( FAILURES > 0 )); then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user