From 61e0edce16c8ae2baedfaaec56485ff36740b903 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sun, 6 Sep 2026 10:45:09 +0800 Subject: [PATCH] test(odm): pin gcs error classes and require the backend contracts in ci (#7250) * test(odm): pin gcs source status-to-error-class mapping The native GCS backend classifies every failure from the HTTP status alone, because GCS states its error code in a body this backend never reads. Only NotFound is negative-cached and only a retryable class may be re-sent, so cover 401/403 -> AccessDenied, 429/503 -> Throttled, 500/502 -> ServerError and 404 -> NotFound over both HEAD and GET. * ci(odm): require the source-backend contract tests in test-and-lint The shared contract tests already run in ci/test-and-lint, but only because gcs is a rustfs default feature; nothing failed if that selection went away. Pin the S3, Azure and native GCS contracts in the core required-test manifest so a lost selection fails the lane. --- .config/ecstore-required-tests.json | 15 +++++++++++ docs/testing/ci-gates.md | 2 +- rustfs/src/on_demand_migration/gcs.rs | 37 +++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/.config/ecstore-required-tests.json b/.config/ecstore-required-tests.json index 6cadc7815..24b723e39 100644 --- a/.config/ecstore-required-tests.json +++ b/.config/ecstore-required-tests.json @@ -40,6 +40,21 @@ "invariant": "corrupt-part-arrays", "suite": "rustfs-filemeta", "name": "filemeta::test::crc_valid_but_part_arrays_corrupt_into_fileinfo_errors_not_panics" + }, + { + "invariant": "odm-source-contract-s3", + "suite": "rustfs", + "name": "on_demand_migration::source_client::tests::s3_backend_satisfies_the_shared_backend_contract" + }, + { + "invariant": "odm-source-contract-azure", + "suite": "rustfs", + "name": "on_demand_migration::azure::tests::azure_backend_satisfies_the_shared_backend_contract" + }, + { + "invariant": "odm-source-contract-gcs", + "suite": "rustfs", + "name": "on_demand_migration::gcs::tests::gcs_native_backend_satisfies_the_shared_backend_contract" } ], "fixtures": [ diff --git a/docs/testing/ci-gates.md b/docs/testing/ci-gates.md index 1779fe458..79f9fe0f5 100644 --- a/docs/testing/ci-gates.md +++ b/docs/testing/ci-gates.md @@ -121,7 +121,7 @@ Update this file in the same PR when a job or check name changes, a workflow gai The existing `ci.yml` test-and-lint job runs the ordinary ECStore and filemeta tests. After that run, `scripts/check_test_wiring.py --check-core` checks the same nextest profile and package selection against `.config/ecstore-required-tests.json`. Every named test must exist, match the filter, and be non-ignored; the job also requires a nonempty JUnit report. This checks membership without running the tests twice. `core-test-listing.json`, JUnit, and the run log are retained in the existing test-and-lint artifact. -The manifest records a minimum set of invariants: write quorum, metadata rollback, stale-writer lock loss, plaintext Range content, multipart cancellation, hiding uncommitted LIST versions, real MinIO metadata, and corrupt part arrays. Renaming or moving a required test must update the manifest in the same change after checking the compiled listing. Extend this list as new deterministic regressions land; it is not a claim that all storage invariants are covered. +The manifest records a minimum set of invariants: write quorum, metadata rollback, stale-writer lock loss, plaintext Range content, multipart cancellation, hiding uncommitted LIST versions, real MinIO metadata, corrupt part arrays, and the shared on-demand-migration source-backend contract for each provider dialect (S3, Azure, native GCS). The three contract entries live in the `rustfs` suite and reach the lane through that package's default features, so dropping `gcs` from `rustfs`'s defaults fails this check instead of silently deselecting the GCS contract (rustfs/backlog#2323). Renaming or moving a required test must update the manifest in the same change after checking the compiled listing. Extend this list as new deterministic regressions land; it is not a claim that all storage invariants are covered. The checked-in MinIO corpus is pinned by file SHA256 and its documented source release. The static wiring guard and the CI selection check both reject missing or changed fixtures. These are metadata fixtures, not a legacy shard-body corpus or proof of crash durability. Optional `legacy_bitrot_read_test` runs may still skip when their external corpus is absent; they do not satisfy a required compatibility lane. Real encrypted fixture reads remain in `minio-interop.yml`, and multi-node fault schedules remain in the existing nightly cluster lane. In-process reopen tests do not establish power-loss durability. diff --git a/rustfs/src/on_demand_migration/gcs.rs b/rustfs/src/on_demand_migration/gcs.rs index bce647b97..5b43ca6ec 100644 --- a/rustfs/src/on_demand_migration/gcs.rs +++ b/rustfs/src/on_demand_migration/gcs.rs @@ -561,4 +561,41 @@ mod tests { } } } + + /// GCS states its error code in the response body, which this backend never + /// reads, so every class must follow from the status alone. The classes are + /// what the runtime acts on: only `NotFound` is negative-cached, and only a + /// retryable class may be re-sent rather than counted against the breaker. + /// The 404 row scripts the readable-bucket probe as well, because a GCS + /// object 404 is only a key miss once the bucket has answered + /// (`object_404_requires_a_readable_source_bucket` pins that rule). + #[tokio::test] + async fn gcs_statuses_map_onto_the_shared_error_classes() { + for method in [Method::HEAD, Method::GET] { + for (status, expected, retryable) in [ + (404_u16, "not_found", false), + (403, "access_denied", false), + (401, "access_denied", false), + (429, "throttled", true), + (503, "throttled", true), + (500, "server_error", true), + (502, "server_error", true), + ] { + let mut script = vec![ScriptedResponse::new(status, Vec::new(), String::new())]; + if status == 404 { + script.push(ScriptedResponse::new(200, Vec::new(), "{}".to_string())); + } + let (endpoint, _) = scripted_server(script).await; + let backend = backend(&endpoint); + let result = if method == Method::HEAD { + backend.head("a.txt").await.map(|_| ()) + } else { + backend.get("a.txt", None).await.map(|_| ()) + }; + let err = result.expect_err("a non-2xx status must fail"); + assert_eq!(err.class_label(), expected, "{method} HTTP {status}: {err:?}"); + assert_eq!(err.is_retryable(), retryable, "{method} HTTP {status}: {err:?}"); + } + } + } }