From ba2047b18422aebffa5f0686cd304ad746e480fb Mon Sep 17 00:00:00 2001 From: Chris Date: Sat, 10 Oct 2026 22:55:48 -0700 Subject: [PATCH] ci: reuse workspace test builds for serial ILM checks (#8465) --- .github/workflows/cache-warm.yml | 2 +- .github/workflows/ci.yml | 128 +++++++++++-------------------- docs/testing/ci-gates.md | 9 ++- scripts/check_test_wiring.py | 35 ++++++++- scripts/ci_gate.py | 42 +++++++++- 5 files changed, 126 insertions(+), 90 deletions(-) diff --git a/.github/workflows/cache-warm.yml b/.github/workflows/cache-warm.yml index 699ba2990..c01dee315 100644 --- a/.github/workflows/cache-warm.yml +++ b/.github/workflows/cache-warm.yml @@ -99,7 +99,7 @@ env: RUST_BACKTRACE: 1 jobs: - # Readers: test-and-lint, test-ilm-integration-serial, build-rustfs-debug-binary, + # Readers: test-and-lint (workspace and ILM), build-rustfs-debug-binary, # e2e-tests, e2e-full. warm-ci-dev: name: Warm ci-dev diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d02f9966..2e92b226e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -168,13 +168,10 @@ jobs: echo "started_at=$(date --utc --iso-8601=seconds)" } > artifacts/test-and-lint/run-metadata.txt - # Clippy runs before the test pass: lint failures are the most common - # CI-only breakage and should surface in minutes, not after 20+ minutes - # of tests. - # Sampled too: clippy is the natural control arm for any CARGO_BUILD_JOBS - # experiment, since --all-targets is check-only for workspace members and - # never links the ~100 test binaries the limit exists to throttle. + # Keep full Clippy validation on main, merge groups, and scheduled/manual + # runs. PRs retain the complete default test suite and compile-free checks. - name: Run clippy lints + if: github.event_name != 'pull_request' run: | ./scripts/ci/resource_sampler.sh start clippy trap './scripts/ci/resource_sampler.sh stop' EXIT @@ -283,6 +280,49 @@ jobs: - name: Verify rebalance/decommission migration proofs run: ./scripts/check_migration_gate_count.sh evidence artifacts/test-and-lint/core-test-listing.json target/nextest/ci/junit.xml + # The workspace already compiled these ignored tests. Keep the same Cargo + # package selection to reuse their binaries, then run them serially in + # separate processes because they share fixed ports and global state. + # test_transition_and_restore_flows was re-enabled by rustfs/backlog#1303: + # its "missing xl.meta on disk2" was a test-util bug (open_disk hardcoded + # disk_index 0), not an EC metadata-distribution issue. + # restore_object_usecase_reports_ongoing_conflict_and_completion was + # re-enabled by backlog#1304 (restore accepts serialize on a short CAS + # guard; the copy-back no longer holds the #4877 whole-copy-back lock, + # so the mid-restore ongoing read and fast 409 rejection it asserts are + # the implemented contract). + - name: Run ignored ILM integration tests serially + if: needs.classify-changes.outputs.mode == 'full' + env: + # Match the preceding workspace build and reuse its feature graph. + CARGO_BUILD_JOBS: ${{ (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && '3' || '2' }} + run: | + mkdir -p artifacts/ilm-integration + set +e + NEXTEST_HIDE_PROGRESS_BAR=1 timeout --verbose --signal=TERM --kill-after=30s 80m \ + cargo nextest run --profile default -j1 --run-ignored ignored-only \ + --all --exclude e2e_test \ + -E 'binary(lifecycle_integration_test) or (package(rustfs) and test(lifecycle_transition_api_test))' \ + --status-level all --final-status-level all \ + 2>&1 | tee artifacts/ilm-integration/nextest.log + status=${PIPESTATUS[0]} + { + echo "exit_status=${status}" + echo "finished_at=$(date --utc --iso-8601=seconds)" + echo + echo "Remaining test-related processes:" + pgrep -af 'cargo|nextest|target/.*/deps/' || true + } > artifacts/ilm-integration/diagnostics.txt + exit "${status}" + + - name: Upload ILM test diagnostics + if: always() && needs.classify-changes.outputs.mode == 'full' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: ilm-integration-${{ github.run_number }}-${{ github.run_attempt }} + path: artifacts/ilm-integration + retention-days: 3 + # This gate builds into a fresh target directory to isolate the E2E root. # Give its cold build a separate budget from workspace tests and migration proofs. offline-enrollment-e2e: @@ -314,80 +354,6 @@ jobs: - name: Check offline enrollment E2E root boundary run: ./scripts/check_offline_enrollment_e2e.sh - # Dedicated serial lane for the ILM / lifecycle integration tests. These tests - # drive the object layer through process-global singletons (the GLOBAL_ENV - # ECStore, the global tier-config manager, background-expiry workers) and bind - # fixed ports (9002 for the scanner suite, 9003 for the app suite), so they are - # marked #[ignore] to keep them out of the default parallel `cargo nextest run - # --all` pass. Run them here explicitly with `--run-ignored ignored-only` and - # `-j1` so no two of them share global state or race for a port at once. - # serial_test's #[serial] does NOT serialize across nextest's process-per-test - # boundary (see .config/nextest.toml); `-j1` is what actually serializes them. - # See rustfs/backlog#1148 (ilm-1) and #1155. - test-ilm-integration-serial: - name: ILM Integration (serial) - if: needs.classify-changes.outputs.mode == 'full' && (github.event_name != 'pull_request' || github.event.action != 'closed') - needs: [ quick-checks, classify-changes ] - runs-on: sm-standard-4 - timeout-minutes: 90 - env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" - steps: - - name: Checkout repository - uses: actions/checkout@f548e57e544e1ff5a4c46bf1e1b8685f8e4a348a # v7 - with: - persist-credentials: false - - - name: Setup Rust environment - uses: ./.github/actions/setup - with: - rust-version: stable - cache-shared-key: ci-dev - cache-save-if: 'false' - install-build-packaging-tools: 'false' - - # test_transition_and_restore_flows was re-enabled by rustfs/backlog#1303: - # its "missing xl.meta on disk2" was a test-util bug (open_disk hardcoded - # disk_index 0), not an EC metadata-distribution issue. - # restore_object_usecase_reports_ongoing_conflict_and_completion was - # re-enabled by backlog#1304 (restore accepts serialize on a short CAS - # guard; the copy-back no longer holds the #4877 whole-copy-back lock, - # so the mid-restore ongoing read and fast 409 rejection it asserts are - # the implemented contract). - - name: Run ignored ILM integration tests serially - env: - # Match the measured Test and Lint link budget. The default exposed - # all 14 pod CPUs and a cold cache spent the full 80m compiling - # without starting one ILM test (main run 32982910990). - CARGO_BUILD_JOBS: ${{ (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && '3' || '2' }} - run: | - mkdir -p artifacts/ilm-integration - set +e - NEXTEST_HIDE_PROGRESS_BAR=1 timeout --verbose --signal=TERM --kill-after=30s 80m \ - cargo nextest run -j1 --run-ignored ignored-only \ - -p rustfs-scanner -p rustfs \ - -E 'binary(lifecycle_integration_test) or (package(rustfs) and test(lifecycle_transition_api_test))' \ - --status-level all --final-status-level all \ - 2>&1 | tee artifacts/ilm-integration/nextest.log - status=${PIPESTATUS[0]} - { - echo "exit_status=${status}" - echo "finished_at=$(date --utc --iso-8601=seconds)" - echo - echo "Remaining test-related processes:" - pgrep -af 'cargo|nextest|target/.*/deps/' || true - } > artifacts/ilm-integration/diagnostics.txt - exit "${status}" - - - name: Upload ILM test diagnostics - if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: ilm-integration-${{ github.run_number }}-${{ github.run_attempt }} - path: | - artifacts/ilm-integration - target/nextest/ci/junit.xml - test-and-lint-rio-v2: name: Test and Lint (rio-v2) # Alternate feature configurations run on main, merge groups, and scheduled @@ -1266,7 +1232,6 @@ jobs: - quick-checks - test-and-lint - offline-enrollment-e2e - - test-ilm-integration-serial - test-and-lint-rio-v2 - connect-short-credential-boundary - test-and-lint-protocols @@ -1300,7 +1265,6 @@ jobs: - quick-checks - test-and-lint - offline-enrollment-e2e - - test-ilm-integration-serial - test-and-lint-rio-v2 - test-and-lint-protocols - build-rustfs-debug-binary diff --git a/docs/testing/ci-gates.md b/docs/testing/ci-gates.md index 389193326..8ea7e09f2 100644 --- a/docs/testing/ci-gates.md +++ b/docs/testing/ci-gates.md @@ -21,6 +21,10 @@ PRs changing only Rust sources under `crates/e2e_test/src/` and known E2E select Alternate-feature unit tests and Clippy for rio-v2, Swift, SFTP, and HTTP/3 run on main pushes, merge groups, and scheduled/manual validation. PRs run the default workspace and retain ILM, S3, real io_uring, and Connect security boundaries in their applicable scopes. The aggregate requires the alternate-feature jobs to be skipped on PRs and successful on non-PR events. This reduces repeated builds across PR revisions; feature-specific regressions can be detected after merging when the merge queue is not used. For changes to these features, dispatch `ci.yml` against the candidate branch before merging when that signal is needed, including PRs targeting `gateway/integration`, which has no automatic push trigger in this workflow. Scheduled results supplement the main baseline and do not replace it. +Workspace Clippy uses the same non-PR cadence. Every code PR still builds and runs the complete default workspace test suite, documentation tests, and migration proofs; Quick Checks retain formatting and script/workflow linting. Without a merge queue, Clippy diagnostics may first fail main after merge. A manual `ci.yml` dispatch against the candidate branch runs Clippy before merging when needed. + +Serial ILM tests run at the end of `test-and-lint` for the `full` scope, reusing the workspace's Cargo package and feature selection. The existing ignored-test filter and `-j1` remain; `--profile default` keeps the no-retry execution and preserves the separate core CI JUnit report. Any ILM failure fails the workspace job and required aggregate. Logs remain in the `ilm-integration` artifact. There is no separate ILM runner or cache restore. + `required-checks` runs even after failed or skipped dependencies. `scripts/ci_gate.py verify` rejects missing jobs, unexpected jobs, failure, cancellation, and unexpected skips; optional lanes are required only on their declared events. `Workspace Test and Lint` is the ordinary Rust job, while `Test and Lint` uniquely names the aggregate. New validation jobs must update both its direct dependencies and the script contract. Test this wiring and its failure cases with `python3 scripts/ci_gate.py --self-test`. The `gateway/integration` ruleset (`24663286`) requires the same three contexts with the same review rules for the s3s to gateway integration branch (rustfs/backlog#2734). It omits `required_linear_history` because `gateway-integration-sync.yml` lands `--no-ff` merge commits from `main` on that branch every weekday, and it names the `rustfs-cla-assistant` app as the bypass actor that pushes them. `ci.yml` runs on pull requests targeting either branch. @@ -47,9 +51,10 @@ The four site-replication state-writer concurrency proofs reserve the available | Event | Check name | Workflow / job | Merge status | Reproduce | |---|---|---|---|---| | PR, non-doc change | `Quick Checks` | `ci.yml` `quick-checks` | Required | `make pre-commit` | -| PR, non-doc change | `Workspace Test and Lint` | `ci.yml` `test-and-lint` | Via aggregate | `cargo clippy --all-targets -- -D warnings`; `cargo nextest run --profile ci --all --exclude e2e_test`; `cargo test --all --doc`; migration evidence check described below | +| PR, non-doc change | `Workspace Test and Lint` | `ci.yml` `test-and-lint` | Via aggregate | `cargo nextest run --profile ci --all --exclude e2e_test`; `cargo test --all --exclude e2e_test --doc`; migration evidence check described below | +| Main push, merge group, schedule, manual dispatch | `Workspace Test and Lint` | `ci.yml` `test-and-lint` | Via aggregate on these events | workspace tests plus `cargo clippy --all-targets -- -D warnings` | | PR, non-doc change | `Typos` | `ci.yml` `typos` | Via aggregate | `typos` | -| PR, full selection | `ILM Integration (serial)` | `ci.yml` `test-ilm-integration-serial` | Via aggregate | exact command in the job | +| Full selection | `Workspace Test and Lint` (serial ILM step) | `ci.yml` `test-and-lint` | Via aggregate | exact `Run ignored ILM integration tests serially` command in the job | | Main push, merge group, schedule, manual dispatch | `Test and Lint (rio-v2)`, `Test and Lint (swift)`, `Test and Lint (sftp)`, `Test and Lint (http3)` | `ci.yml` `test-and-lint-rio-v2`, `test-and-lint-protocols` | Via aggregate on these events; skipped on PRs | `cargo nextest run` with the job's `--features` | | PR, full selection | `Connect Short Credential Boundary` | `ci.yml` `connect-short-credential-boundary` | Via aggregate | `cargo test -p rustfs --test connect_registration --features connect-e2e-short-credentials`; `cargo check -p rustfs --release --features connect-e2e-short-credentials` must fail | | PR, full selection | `Offline Enrollment E2E Root Boundary` | `ci.yml` `offline-enrollment-e2e` | Via aggregate | `scripts/check_offline_enrollment_e2e.sh` | diff --git a/scripts/check_test_wiring.py b/scripts/check_test_wiring.py index bd9f7013f..3a8251775 100755 --- a/scripts/check_test_wiring.py +++ b/scripts/check_test_wiring.py @@ -684,12 +684,12 @@ def check_ilm_build_budget(root: Path) -> list[str]: return [".github/workflows/ci.yml: missing workflow"] workflow = workflow_path.read_text() job = re.search( - r"^ test-ilm-integration-serial:\n(?P.*?)(?=^ [a-z0-9-]+:\n)", + r"^ test-and-lint:\n(?P.*?)(?=^ [a-z0-9-]+:\n)", workflow, re.MULTILINE | re.DOTALL, ) if job is None: - return [".github/workflows/ci.yml: missing test-ilm-integration-serial job"] + return [".github/workflows/ci.yml: ILM integration must share the workspace test job"] step = re.search( r"^ - name: Run ignored ILM integration tests serially\n(?P.*?)(?=^ - name:|\Z)", @@ -705,6 +705,16 @@ def check_ilm_build_budget(root: Path) -> list[str]: ) if budget not in step.group("body"): return [".github/workflows/ci.yml: ILM integration must keep the measured 3/2 Cargo build budget"] + for required in ( + " if: needs.classify-changes.outputs.mode == 'full'", + "cargo nextest run --profile default -j1 --run-ignored ignored-only", + "--all --exclude e2e_test", + "-E 'binary(lifecycle_integration_test) or (package(rustfs) and test(lifecycle_transition_api_test))'", + 'status=${PIPESTATUS[0]}', + 'exit "${status}"', + ): + if required not in step.group("body"): + return [f".github/workflows/ci.yml: ILM integration is missing {required.strip()}"] return [] @@ -5566,12 +5576,17 @@ class SelfTests(unittest.TestCase): workflow.parent.mkdir(parents=True) valid = ( "jobs:\n" - " test-ilm-integration-serial:\n" + " test-and-lint:\n" " steps:\n" " - name: Run ignored ILM integration tests serially\n" + " if: needs.classify-changes.outputs.mode == 'full'\n" " env:\n" " CARGO_BUILD_JOBS: ${{ (github.event_name == 'push' || github.event_name == 'workflow_dispatch') && '3' || '2' }}\n" - " run: cargo nextest run\n" + " run: |\n" + " cargo nextest run --profile default -j1 --run-ignored ignored-only --all --exclude e2e_test \\\n" + " -E 'binary(lifecycle_integration_test) or (package(rustfs) and test(lifecycle_transition_api_test))'\n" + " status=${PIPESTATUS[0]}\n" + ' exit "${status}"\n' " next-job:\n" " steps: []\n" ) @@ -5580,6 +5595,18 @@ class SelfTests(unittest.TestCase): workflow.write_text(valid.replace(" CARGO_BUILD_JOBS:", " BUILD_JOBS:")) self.assertEqual(len(check_ilm_build_budget(root)), 1) + for old, new in ( + ("test-and-lint:", "test-ilm-integration-serial:"), + ("outputs.mode == 'full'", "outputs.mode == 'docs'"), + ("--profile default", "--profile ci"), + ("-j1", "-j2"), + ("ignored-only", "default"), + ("--all --exclude e2e_test", "-p rustfs-scanner -p rustfs"), + ("binary(lifecycle_integration_test)", "none()"), + ('exit "${status}"', "exit 0"), + ): + workflow.write_text(valid.replace(old, new)) + self.assertTrue(check_ilm_build_budget(root), (old, new)) def test_vault_tests_require_cross_process_serialization(self) -> None: with tempfile.TemporaryDirectory() as tmp: diff --git a/scripts/ci_gate.py b/scripts/ci_gate.py index e8cf52e75..b99c10927 100644 --- a/scripts/ci_gate.py +++ b/scripts/ci_gate.py @@ -16,7 +16,7 @@ import unittest ROOT = Path(__file__).resolve().parent.parent ALWAYS_JOBS = ("classify-changes", "typos", "quick-checks") CODE_JOBS = ( - "test-and-lint", "offline-enrollment-e2e", "test-ilm-integration-serial", "test-and-lint-rio-v2", + "test-and-lint", "offline-enrollment-e2e", "test-and-lint-rio-v2", "connect-short-credential-boundary", "test-and-lint-protocols", "build-rustfs-debug-binary", "uring-integration", "e2e-tests", "s3-implemented-tests", "s3-lifecycle-behavior-tests", @@ -224,6 +224,10 @@ def check_workflow(root: Path) -> list[str]: block = yaml_block(jobs, job, 2) or [] if scalar(block, "if", 4) != "needs.classify-changes.outputs.mode == 'full' && github.event_name != 'pull_request'": errors.append(f"{job} must retain full validation on every non-PR event") + workspace = "\n".join(yaml_block(jobs, "test-and-lint", 2) or []) + clippy = re.search(r"^ - name: Run clippy lints\n(?P.*?)(?=^ - name:|\Z)", workspace, re.MULTILINE | re.DOTALL) + if clippy is None or scalar(clippy["body"].splitlines(), "if", 8) != "github.event_name != 'pull_request'": + errors.append("workspace Clippy must run on every non-PR event") if scalar(gate, "shell", 8) != "bash" or scalar(gate, "run", 8) != "python3 scripts/ci_gate.py verify" or scalar(gate, "CI_NEEDS", 10) != "${{ toJSON(needs) }}": errors.append("required-checks must verify the actual needs results") if any(re.match(r'''\s+(?:- )?(?:["']?continue-on-error["']?\s*:|["']?if["']?\s*:)''', line) and not line.startswith(" if:") for line in gate): @@ -387,6 +391,9 @@ class SelfTests(unittest.TestCase): (root / ".github/workflows").mkdir(parents=True) source = (ROOT / ".github/workflows/ci.yml").read_text() path = root / ".github/workflows/ci.yml" + for condition in ("false", "github.event_name == 'schedule'", "github.event_name == 'pull_request'"): + path.write_text(source.replace(" - name: Run clippy lints\n if: github.event_name != 'pull_request'", f" - name: Run clippy lints\n if: {condition}")) + self.assertIn("workspace Clippy must run on every non-PR event", check_workflow(root)) for job in EXTENDED_FEATURE_JOBS: before, block = source.split(f" {job}:\n", 1) for condition in ("false", "github.event_name == 'schedule'", "github.event_name != 'pull_request'"): @@ -462,6 +469,39 @@ class SelfTests(unittest.TestCase): self.assertEqual(result.returncode, code, result.stderr) self.assertIn("ERROR:" if code else "CI contract passed", result.stderr if code else result.stdout) + def test_merged_ilm_step_preserves_exit_status_and_core_report(self): + from check_test_wiring import yaml_block + from test_security_workflow import named_steps, shell_body + + workflow = (ROOT / ".github/workflows/ci.yml").read_text().splitlines() + step = named_steps(yaml_block(workflow, "test-and-lint", 2))["Run ignored ILM integration tests serially"] + body = shell_body(step) + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + commands = root / "bin" + commands.mkdir() + # Exercise the actual pipeline without building the workspace. + fake_timeout = commands / "timeout" + fake_timeout.write_text('#!/bin/sh\nshift 4\nexec "$@"\n') + fake_timeout.chmod(0o755) + cargo = commands / "cargo" + cargo.write_text('#!/bin/sh\nprintf "%s\\n" "$@" > "$ILM_ARGS"\nexit "$ILM_STATUS"\n') + cargo.chmod(0o755) + report = root / "target/nextest/ci/junit.xml" + report.parent.mkdir(parents=True) + report.write_text("existing workspace report") + args = root / "args.txt" + for status in (0, 23, 124): + env = dict(os.environ, PATH=f"{commands}{os.pathsep}{os.environ['PATH']}", + ILM_ARGS=str(args), ILM_STATUS=str(status)) + result = subprocess.run(["bash", "-e", "-o", "pipefail", "-c", body], cwd=root, env=env, capture_output=True, text=True) + self.assertEqual(result.returncode, status, result.stderr) + self.assertEqual(report.read_text(), "existing workspace report") + arguments = args.read_text().splitlines() + self.assertEqual(arguments[:9], ["nextest", "run", "--profile", "default", "-j1", "--run-ignored", "ignored-only", "--all", "--exclude"]) + self.assertEqual(arguments[9], "e2e_test") + self.assertIn(f"exit_status={status}", (root / "artifacts/ilm-integration/diagnostics.txt").read_text()) + def test_actual_selector_bootstrap_uses_base_policy_and_fails_closed(self): from check_test_wiring import yaml_block jobs = yaml_block((ROOT / ".github/workflows/ci.yml").read_text().splitlines(), "jobs", 0)