From f63af3df63309f043882f355fdbc27ba81f6123c Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 11 Jul 2026 13:42:56 +0800 Subject: [PATCH] chore: retire completed-migration scaffolding, wire orphaned boundary check (#4719) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ecstore/global-state migrations are done (backlog#815, #939, #1052 all closed). Review of every migration-era test/gate measure found three things actually retirable or broken — everything else is a live anti-regression guard and stays. Remove: - scripts/check_metrics_migration_refs.sh — guards a migration that finished: rustfs_metrics:: has zero hits, the metrics crate no longer exists, and the script was never wired into CI or make (only reference was one line in config-model-boundary-adr.md, also removed). - crates/obs init_metrics_collectors — the "backward-compatible alias kept during migration" the removed script was guarding. Zero callers; pure delegate to init_metrics_runtime. Archive (docs/superpowers/plans/, continuing the 2026-07 convention, with the standard archived banner): - startup-timeline.md, scheduler-baseline.md, profiling-numa-capability-inventory.md, kms-development-defaults-inventory.md — one-shot snapshots whose only consumer is the already-archived migration-progress ledger (their same-dir links there start resolving again after the move); zero script pins; fed the closed backlog#660/#665 architecture-review ledger. Fixed the one outbound link (startup-timeline -> readiness-matrix) that the move would have broken — check_doc_paths.sh deliberately does not scan plans/, so nothing else would have caught it. Wire (found orphaned by the same review): - scripts/check_extension_schema_boundaries.sh guards a live contract crate but was never invoked anywhere. Add lint-fmt.mak target, include in pre-commit/pre-pr/dev-check, add ci.yml Quick Checks step (job already installs ripgrep), sync the CONTRIBUTING.md enumerated list, and harden the script against a silently-passing rg probe when src/ is missing. Keep (verified live, documented so the next cleanup pass does not repeat this analysis): - scripts/check_architecture_migration_rules.sh — added a header stating it is a permanent boundary guard, not retirable migration scaffolding; 'migration' in the name is historical. - check_migration_gate_count.sh + floor, delete-marker e2e proof, all pinned docs, compat-cleanup-register sync, remaining inventories (referenced by live docs). Verification: all 7 guard scripts pass, actionlint clean, cargo check --workspace (excl e2e) clean, cargo fmt --check clean. Adversarially reviewed by two independent skeptic passes; their 7 findings (alias left behind, broken outbound link, missing banners, wrong backlog attribution, CONTRIBUTING drift, rg exit-2 hole, missing header rationale) are all folded in. --- .config/make/lint-fmt.mak | 5 ++ .config/make/pre-commit.mak | 6 +- .github/workflows/ci.yml | 3 + CONTRIBUTING.md | 5 +- crates/obs/src/lib.rs | 4 +- crates/obs/src/metrics/mod.rs | 4 +- crates/obs/src/metrics/scheduler.rs | 5 -- docs/architecture/README.md | 10 +-- .../architecture/config-model-boundary-adr.md | 1 - .../kms-development-defaults-inventory.md | 4 + .../profiling-numa-capability-inventory.md | 4 + .../plans}/scheduler-baseline.md | 4 + .../plans}/startup-timeline.md | 6 +- scripts/check_architecture_migration_rules.sh | 8 ++ scripts/check_extension_schema_boundaries.sh | 4 + scripts/check_metrics_migration_refs.sh | 73 ------------------- 16 files changed, 52 insertions(+), 94 deletions(-) rename docs/{architecture => superpowers/plans}/kms-development-defaults-inventory.md (97%) rename docs/{architecture => superpowers/plans}/profiling-numa-capability-inventory.md (96%) rename docs/{architecture => superpowers/plans}/scheduler-baseline.md (96%) rename docs/{architecture => superpowers/plans}/startup-timeline.md (98%) delete mode 100755 scripts/check_metrics_migration_refs.sh diff --git a/.config/make/lint-fmt.mak b/.config/make/lint-fmt.mak index c38a0a78b..73d315976 100644 --- a/.config/make/lint-fmt.mak +++ b/.config/make/lint-fmt.mak @@ -50,6 +50,11 @@ tokio-io-uring-check: ## Check tokio io-uring runtime feature stays removed @echo "🚫 Checking tokio io-uring feature guard..." ./scripts/check_no_tokio_io_uring.sh +.PHONY: extension-schema-check +extension-schema-check: ## Check extension-schema stays a lightweight contract crate + @echo "🧩 Checking extension schema boundaries..." + ./scripts/check_extension_schema_boundaries.sh + .PHONY: compilation-check compilation-check: core-deps ## Run compilation check @echo "🔨 Running compilation check..." diff --git a/.config/make/pre-commit.mak b/.config/make/pre-commit.mak index 416a87287..842f49ef4 100644 --- a/.config/make/pre-commit.mak +++ b/.config/make/pre-commit.mak @@ -14,13 +14,13 @@ doc-paths-check: ## Check that instruction/architecture docs reference existing ./scripts/check_doc_paths.sh .PHONY: pre-commit -pre-commit: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check doc-paths-check quick-check ## Run fast pre-commit checks without clippy/full tests +pre-commit: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check doc-paths-check quick-check ## Run fast pre-commit checks without clippy/full tests @echo "✅ All pre-commit checks passed!" .PHONY: pre-pr -pre-pr: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check doc-paths-check clippy-check test ## Run full pre-PR checks with clippy and tests +pre-pr: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check doc-paths-check clippy-check test ## Run full pre-PR checks with clippy and tests @echo "✅ All pre-PR checks passed!" .PHONY: dev-check -dev-check: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check doc-paths-check quick-check ## Run fast local development checks +dev-check: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check doc-paths-check quick-check ## Run fast local development checks @echo "✅ Fast development checks passed!" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eb3fd4d8b..7955f7998 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -128,6 +128,9 @@ jobs: - name: Check tokio io-uring feature guard run: ./scripts/check_no_tokio_io_uring.sh + - name: Check extension schema boundaries + run: ./scripts/check_extension_schema_boundaries.sh + test-and-lint: name: Test and Lint if: github.event_name != 'pull_request' || github.event.action != 'closed' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ea2051fe0..c4f5b079b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,8 +79,9 @@ make pre-pr 3. `architecture-migration-check` — `./scripts/check_architecture_migration_rules.sh` 4. `logging-guardrails-check` — `./scripts/check_logging_guardrails.sh` 5. `tokio-io-uring-check` — `./scripts/check_no_tokio_io_uring.sh` -6. `doc-paths-check` — `./scripts/check_doc_paths.sh` -7. `quick-check` — `cargo check --workspace --exclude e2e_test` +6. `extension-schema-check` — `./scripts/check_extension_schema_boundaries.sh` +7. `doc-paths-check` — `./scripts/check_doc_paths.sh` +8. `quick-check` — `cargo check --workspace --exclude e2e_test` **`make pre-commit` does NOT run clippy and does NOT run any tests.** A green `make pre-commit` is not enough to open a pull request. diff --git a/crates/obs/src/lib.rs b/crates/obs/src/lib.rs index c41165726..b2919d9b2 100644 --- a/crates/obs/src/lib.rs +++ b/crates/obs/src/lib.rs @@ -75,8 +75,8 @@ pub use metrics::schema::*; pub use metrics::{ MetricsRuntimeCancellationSource, MetricsRuntimeController, MetricsRuntimeControllerSnapshot, MetricsRuntimeDesiredSnapshot, MetricsRuntimeDesiredState, MetricsRuntimeIntervalsSnapshot, MetricsRuntimeReconcilePlan, MetricsRuntimeServiceState, - MetricsRuntimeShutdownHandle, MetricsRuntimeStatusSnapshot, MetricsRuntimeWorkerMutation, init_metrics_collectors, - init_metrics_runtime, metrics_runtime_controller_snapshot, metrics_runtime_status_snapshot, + MetricsRuntimeShutdownHandle, MetricsRuntimeStatusSnapshot, MetricsRuntimeWorkerMutation, init_metrics_runtime, + metrics_runtime_controller_snapshot, metrics_runtime_status_snapshot, }; pub use telemetry::{OtelGuard, Recorder}; diff --git a/crates/obs/src/metrics/mod.rs b/crates/obs/src/metrics/mod.rs index a083fc28d..30730bbed 100644 --- a/crates/obs/src/metrics/mod.rs +++ b/crates/obs/src/metrics/mod.rs @@ -27,8 +27,8 @@ pub use report::{PrometheusMetric, report_metrics}; pub use scheduler::{ MetricsRuntimeCancellationSource, MetricsRuntimeController, MetricsRuntimeControllerSnapshot, MetricsRuntimeDesiredSnapshot, MetricsRuntimeDesiredState, MetricsRuntimeIntervalsSnapshot, MetricsRuntimeReconcilePlan, MetricsRuntimeServiceState, - MetricsRuntimeShutdownHandle, MetricsRuntimeStatusSnapshot, MetricsRuntimeWorkerMutation, init_metrics_collectors, - init_metrics_runtime, metrics_runtime_controller_snapshot, metrics_runtime_status_snapshot, + MetricsRuntimeShutdownHandle, MetricsRuntimeStatusSnapshot, MetricsRuntimeWorkerMutation, init_metrics_runtime, + metrics_runtime_controller_snapshot, metrics_runtime_status_snapshot, }; pub(crate) use storage_api::metrics::{ BucketOperations, BucketOptions, ObsBucketBandwidthMonitor, ObsEcstoreResult, ObsStore, StorageAdminApi, diff --git a/crates/obs/src/metrics/scheduler.rs b/crates/obs/src/metrics/scheduler.rs index 5a8e0c808..2fbfd7b55 100644 --- a/crates/obs/src/metrics/scheduler.rs +++ b/crates/obs/src/metrics/scheduler.rs @@ -1494,11 +1494,6 @@ pub fn init_metrics_runtime(token: CancellationToken) { }); } -/// Backward-compatible alias kept during migration. -pub fn init_metrics_collectors(token: CancellationToken) { - init_metrics_runtime(token); -} - fn advance_deadline(deadline: &mut Instant, interval: Duration, now: Instant) { if *deadline > now { return; diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 0e12c515b..1b7de5b95 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -51,12 +51,12 @@ Two rules keep this directory healthy: - [obs-ecstore-dependency-inventory.md](obs-ecstore-dependency-inventory.md) - [background-services-inventory.md](background-services-inventory.md) - [admin-route-action-snapshot.md](admin-route-action-snapshot.md) -- [startup-timeline.md](startup-timeline.md) -- [scheduler-baseline.md](scheduler-baseline.md) -- [profiling-numa-capability-inventory.md](profiling-numa-capability-inventory.md) -- [kms-development-defaults-inventory.md](kms-development-defaults-inventory.md) - [compat-cleanup-register.md](compat-cleanup-register.md) Historical plans and trackers (rebalance/decommission phases, migration-progress ledger) were moved to -[`docs/superpowers/plans/`](../superpowers/plans/) in 2026-07. +[`docs/superpowers/plans/`](../superpowers/plans/) in 2026-07. One-shot +migration snapshots whose only consumer was that ledger (startup timeline, +scheduler baseline, profiling/NUMA capability inventory, KMS development +defaults inventory) followed once the architecture-review ledger they fed +closed out (backlog#660/#665). diff --git a/docs/architecture/config-model-boundary-adr.md b/docs/architecture/config-model-boundary-adr.md index 79fca4ba6..6fe7cbb00 100644 --- a/docs/architecture/config-model-boundary-adr.md +++ b/docs/architecture/config-model-boundary-adr.md @@ -173,7 +173,6 @@ Before pushing an extraction PR, run: - `cargo tree -p rustfs-ecstore --edges normal` - `./scripts/check_layer_dependencies.sh` - `./scripts/check_architecture_migration_rules.sh` -- `./scripts/check_metrics_migration_refs.sh` - `cargo fmt --all --check` - `make pre-commit` diff --git a/docs/architecture/kms-development-defaults-inventory.md b/docs/superpowers/plans/kms-development-defaults-inventory.md similarity index 97% rename from docs/architecture/kms-development-defaults-inventory.md rename to docs/superpowers/plans/kms-development-defaults-inventory.md index 0ed4de374..fa422ab9f 100644 --- a/docs/architecture/kms-development-defaults-inventory.md +++ b/docs/superpowers/plans/kms-development-defaults-inventory.md @@ -1,3 +1,7 @@ +> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07) +> when the architecture-review ledger it fed closed out. Kept for history; not +> maintained. + # KMS Development Defaults Inventory This inventory tracks `KMSD-001` for diff --git a/docs/architecture/profiling-numa-capability-inventory.md b/docs/superpowers/plans/profiling-numa-capability-inventory.md similarity index 96% rename from docs/architecture/profiling-numa-capability-inventory.md rename to docs/superpowers/plans/profiling-numa-capability-inventory.md index 590591605..b4aba5d1e 100644 --- a/docs/architecture/profiling-numa-capability-inventory.md +++ b/docs/superpowers/plans/profiling-numa-capability-inventory.md @@ -1,3 +1,7 @@ +> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07) +> when the architecture-review ledger it fed closed out. Kept for history; not +> maintained. + # Profiling And NUMA Capability Inventory This inventory covers `G-013` for `rustfs/backlog#667`. It records the current diff --git a/docs/architecture/scheduler-baseline.md b/docs/superpowers/plans/scheduler-baseline.md similarity index 96% rename from docs/architecture/scheduler-baseline.md rename to docs/superpowers/plans/scheduler-baseline.md index 94144adc7..3b148208b 100644 --- a/docs/architecture/scheduler-baseline.md +++ b/docs/superpowers/plans/scheduler-baseline.md @@ -1,3 +1,7 @@ +> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07) +> when the architecture-review ledger it fed closed out. Kept for history; not +> maintained. + # Scheduler Baseline Inventory This inventory covers `G-011` for `rustfs/backlog#675`. It is a docs-only diff --git a/docs/architecture/startup-timeline.md b/docs/superpowers/plans/startup-timeline.md similarity index 98% rename from docs/architecture/startup-timeline.md rename to docs/superpowers/plans/startup-timeline.md index 4d2acfd16..c68e84f31 100644 --- a/docs/architecture/startup-timeline.md +++ b/docs/superpowers/plans/startup-timeline.md @@ -1,3 +1,7 @@ +> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07) +> when the architecture-review ledger it fed closed out. Kept for history; not +> maintained. + # Startup Timeline Baseline This document records the current binary startup order before runtime/lifecycle @@ -33,7 +37,7 @@ new startup semantics. | `RUN-007` | `rustfs/src/startup_storage.rs` | Publish endpoints and erasure type. | Updates global endpoints and erasure type. | Non-fatal in this path. | None | | `RUN-008` | `rustfs/src/startup_storage.rs` | Initialize local disks, prewarm local disk id map, and initialize lock clients. | Opens local disk state, primes disk id lookup, and creates global lock clients. | Local disk init is fatal; prewarm and lock-client setup are non-fatal in this path. | None | | `RUN-009` | `rustfs/src/startup_server.rs` | Initialize capacity management and service state manager. | Starts capacity management and moves service state to `Starting`. | Non-fatal in this path. | None | -| `RUN-010` | `rustfs/src/startup_server.rs` | Start S3 HTTP listener and optional console listener before storage is ready. | Starts HTTP servers with readiness gates; console listener starts only when enabled and configured. | Fatal if a configured listener cannot start. | Requests remain gated until full readiness except probe/admin/console/rpc/tonic/table-catalog exempt paths; see [`readiness-matrix.md`](readiness-matrix.md) | +| `RUN-010` | `rustfs/src/startup_server.rs` | Start S3 HTTP listener and optional console listener before storage is ready. | Starts HTTP servers with readiness gates; console listener starts only when enabled and configured. | Fatal if a configured listener cannot start. | Requests remain gated until full readiness except probe/admin/console/rpc/tonic/table-catalog exempt paths; see [`readiness-matrix.md`](../../architecture/readiness-matrix.md) | | `RUN-011` | `rustfs/src/startup_storage.rs` | Create cancellation token and initialize `ECStore`. | Creates the runtime cancellation token and storage engine. | Fatal if `ECStore::new` fails. | None | | `RUN-012` | `rustfs/src/startup_storage.rs` | Initialize ECStore config and global config system. | Initializes ECStore config, attempts server-config migration, then retries global config init up to 15 times. | Migration attempt is non-fatal in this path; global config init becomes fatal after retries. | Marks the `GlobalReadiness` `StorageReady` stage after global config init succeeds; later runtime readiness still rechecks storage, IAM, lock quorum, and gated peer health before `FullReady` | | `RUN-013` | `rustfs/src/startup_storage.rs` and `rustfs/src/startup_services.rs` | Start replication and KMS systems. | Starts background replication pool, then initializes KMS from startup services. | Replication init is non-fatal in this path; KMS init is fatal on error. | `StorageReady` stage is already marked; dynamic runtime storage readiness is still checked before `FullReady`; KMS compatibility readiness remains feature-gated health behavior | diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index cf161d643..2980a2e1f 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -1,4 +1,12 @@ #!/usr/bin/env bash +# +# Permanent architecture-boundary guard (anti-regression), NOT a +# migration-progress gate. "migration" in the name is historical: these rules +# were established BY the ecstore/global-state migrations (backlog#815, #939, +# #1052 — all closed) and now keep the resulting boundaries from rotting +# (facade bypasses, compat-shim resurrection, owner-module drift). Closed +# migration issues are NOT a reason to retire this script or its pins. +# Runs in ci.yml Quick Checks and .github/workflows/architecture-migration-rules.yml. set -euo pipefail diff --git a/scripts/check_extension_schema_boundaries.sh b/scripts/check_extension_schema_boundaries.sh index a19d4830d..e181ac303 100755 --- a/scripts/check_extension_schema_boundaries.sh +++ b/scripts/check_extension_schema_boundaries.sh @@ -12,6 +12,10 @@ fail() { } [[ -f "$CARGO_TOML" ]] || fail "crates/extension-schema/Cargo.toml is missing" +# Without this, a moved/removed src/ makes the rg probe below exit 2, which the +# `if` treats as "no matches" — the check would silently pass while guarding +# nothing. +[[ -d "${CRATE_DIR}/src" ]] || fail "crates/extension-schema/src is missing" grep -qxF ' "crates/extension-schema", # Extension schema contracts' "${ROOT_DIR}/Cargo.toml" || fail "workspace members must include crates/extension-schema" diff --git a/scripts/check_metrics_migration_refs.sh b/scripts/check_metrics_migration_refs.sh deleted file mode 100755 index e77ceadd5..000000000 --- a/scripts/check_metrics_migration_refs.sh +++ /dev/null @@ -1,73 +0,0 @@ -#!/usr/bin/env bash - -set -euo pipefail - -ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" - -# Allowed references during migration bootstrap (T00 baseline). -# Keep entries minimal and remove them as callsites are migrated. -ALLOWLIST=( - "crates/obs/src/lib.rs" - "crates/obs/src/metrics/mod.rs" - "crates/obs/src/metrics/scheduler.rs" -) - -is_allowed_path() { - local path="$1" - local allow - for allow in "${ALLOWLIST[@]}"; do - if [[ "$path" == "$allow" ]]; then - return 0 - fi - done - - return 1 -} - -MATCHES=() -while IFS= read -r line; do - MATCHES+=("$line") -done < <( - cd "$ROOT_DIR" - rg -n --no-heading \ - -e 'rustfs_metrics::' \ - -e '\binit_metrics_system\b' \ - -e '\binit_metrics_collectors\b' \ - rustfs/src crates \ - --glob '**/*.rs' \ - --glob '!**/tests/**' \ - --glob '!docs/**' || true -) - -while IFS= read -r line; do - MATCHES+=("$line") -done < <( - cd "$ROOT_DIR" - rg -n --no-heading \ - -e 'rustfs-metrics' \ - -e 'crates/metrics' \ - Cargo.toml rustfs crates \ - --glob '**/*.toml' \ - --glob '**/*.rs' \ - --glob '!**/tests/**' \ - --glob '!docs/**' || true -) - -VIOLATIONS=() - -for hit in "${MATCHES[@]}"; do - file="${hit%%:*}" - if is_allowed_path "$file"; then - continue - fi - - VIOLATIONS+=("$hit") -done - -if (( ${#VIOLATIONS[@]} > 0 )); then - echo "Metrics migration reference guard failed: found non-allowlisted references" - printf '%s\n' "${VIOLATIONS[@]}" - exit 1 -fi - -echo "Metrics migration reference guard passed."