From 707d0621745afbdad67c12d21d59c23b0d77a727 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 1 Aug 2026 10:49:19 +0800 Subject: [PATCH 1/2] ci: stop running expensive jobs that cannot inform the result (#5528) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent fixes that all avoid burning self-hosted runners on work whose outcome is already determined. None of them changes what is tested. - ci-docs-only: add a "Quick Checks" companion job. It is a prerequisite for gating ci.yml's expensive jobs behind quick-checks (rustfs/backlog#1599): once "Quick Checks" is a required check, a docs-only PR would otherwise wait on it forever. The steps are a byte-identical copy of ci.yml's quick-checks rather than an `echo`, so that on a mixed PR the two same-named check runs execute the same commands against the same merge ref and cannot disagree — GitHub has no written contract for how it picks between same-named required check runs, and the real job only takes 45-51s, leaving no timing margin to rely on. - ci: guard uring-integration with the same `closed` check every other job already has. The pull_request trigger includes `closed` only so the concurrency group cancels in-flight runs; this job had no guard and no `needs`, so every closed or merged PR ran the full io_uring suite (4m17s, 7m19s and 7m31s on runs 30678272341, 30678117601 and 30662728539). - ci: gate s3-lifecycle-behavior-tests on e2e-tests, matching s3-implemented-tests. Both lanes only download the prebuilt debug binary, and s3-implemented-tests already finishes later, so a green PR's wall clock is unchanged; a red one stops holding a sm-standard-4 for up to 30 minutes. Refs: rustfs/backlog#1598, rustfs/backlog#1599 --- .github/workflows/ci-docs-only.yml | 70 +++++++++++++++++++++++++++--- .github/workflows/ci.yml | 15 ++++++- 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci-docs-only.yml b/.github/workflows/ci-docs-only.yml index 6cb41278f..ecd4419a7 100644 --- a/.github/workflows/ci-docs-only.yml +++ b/.github/workflows/ci-docs-only.yml @@ -12,18 +12,20 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Companion to ci.yml for the required "Test and Lint" status check. +# Companion to ci.yml for the required "Test and Lint" and "Quick Checks" +# status checks. # # ci.yml skips docs-only pull requests via paths-ignore, but the branch -# ruleset requires a check named "Test and Lint" — without this workflow a -# docs-only PR would wait on that check forever. This workflow triggers on -# exactly the paths ci.yml ignores and reports an instant success under the -# same job name. Mixed PRs trigger both workflows and the real check still -# gates: a required check with any failing run blocks the merge. +# ruleset requires checks named "Test and Lint" and "Quick Checks" — without +# this workflow a docs-only PR would wait on those checks forever. This +# workflow triggers on exactly the paths ci.yml ignores and reports success +# under the same job names. Mixed PRs trigger both workflows and the real +# checks still gate: a required check with any failing run blocks the merge. # https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks # # Keep the paths list below in sync with the pull_request paths-ignore list -# in ci.yml. +# in ci.yml, and keep the quick-checks steps below byte-identical to the +# quick-checks job in ci.yml (see the comment on that job). name: Continuous Integration (docs only) @@ -52,9 +54,63 @@ permissions: contents: read jobs: + # Deliberately NOT a bare `echo`. Once "Quick Checks" becomes a required + # check, ci.yml gates every expensive job behind it, so a mixed PR reports + # two check runs with this name: the real one (45-51s) and this companion. + # GitHub has no written contract for how it picks between same-named + # required check runs ("latest wins" vs "any failure blocks"), so instead of + # relying on ordering we make both runs execute the same commands against + # the same merge ref — their conclusions are then necessarily identical and + # the choice does not matter. Keep these steps byte-identical to the + # quick-checks job in ci.yml (a guard script that asserts this, and the paths + # sync below, is tracked in rustfs/backlog#1603). + # + # For a genuinely docs-only PR this adds no strictness (no code changed, so + # fmt and the guards always pass) and costs ~50s of ubuntu-latest. + quick-checks: + name: Quick Checks + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + + - name: Install ripgrep + run: sudo apt-get update && sudo apt-get install -y ripgrep + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + components: rustfmt + + - name: Check code formatting + run: cargo fmt --all --check + + - name: Check unsafe code allowances + run: ./scripts/check_unsafe_code_allowances.sh + + - name: Check layered dependencies + run: ./scripts/check_layer_dependencies.sh + + - name: Check architecture migration rules + run: ./scripts/check_architecture_migration_rules.sh + + - 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 + + - name: Check body-cache whitelist guard + run: ./scripts/check_body_cache_whitelist.sh + + - name: Check no planning docs committed + run: ./scripts/check_no_planning_docs.sh + test-and-lint: name: Test and Lint runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 184771d4e..096a9cffd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -448,6 +448,13 @@ jobs: uring-integration: name: io_uring Integration (real) + # The pull_request trigger includes `closed` purely so the concurrency + # group cancels in-flight runs of a closed PR; every other job opts out of + # that run with this guard (or is skipped through its `needs` chain). This + # job had neither, so each closed/merged PR really ran the whole io_uring + # suite (measured 4m17s / 7m19s / 7m31s on runs 30678272341 / 30678117601 / + # 30662728539) and kept the cancellation run in progress for minutes. + if: github.event_name != 'pull_request' || github.event.action != 'closed' # GitHub-hosted ubuntu-latest runs a recent kernel with io_uring and, unlike # a container, applies no seccomp filter that would block io_uring_setup — so # the probe succeeds and the tests exercise the real UringBackend/FdCache/ @@ -746,7 +753,13 @@ jobs: # evaluates ILM within ~2s of the due time, well inside the poll window. s3-lifecycle-behavior-tests: name: S3 Lifecycle Behavior Tests - needs: [ build-rustfs-debug-binary ] + # Also gated on e2e-tests, matching s3-implemented-tests: when the e2e smoke + # suite is already red this lane cannot tell us anything new, and it holds a + # sm-standard-4 for up to 30 minutes doing so. Both lanes only download the + # prebuilt debug binary (no cargo build), and s3-implemented-tests — which + # already waits on e2e-tests — finishes later anyway, so a green PR's total + # wall clock is unchanged. + needs: [ build-rustfs-debug-binary, e2e-tests ] runs-on: sm-standard-4 timeout-minutes: 30 steps: From 790bdc0e63e0c4b4cbe4a575f5c8f9feb57c8919 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 1 Aug 2026 10:57:02 +0800 Subject: [PATCH 2/2] ci: gate the seven expensive jobs on Quick Checks (#5529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ci: stop running expensive jobs that cannot inform the result Three independent fixes that all avoid burning self-hosted runners on work whose outcome is already determined. None of them changes what is tested. - ci-docs-only: add a "Quick Checks" companion job. It is a prerequisite for gating ci.yml's expensive jobs behind quick-checks (rustfs/backlog#1599): once "Quick Checks" is a required check, a docs-only PR would otherwise wait on it forever. The steps are a byte-identical copy of ci.yml's quick-checks rather than an `echo`, so that on a mixed PR the two same-named check runs execute the same commands against the same merge ref and cannot disagree — GitHub has no written contract for how it picks between same-named required check runs, and the real job only takes 45-51s, leaving no timing margin to rely on. - ci: guard uring-integration with the same `closed` check every other job already has. The pull_request trigger includes `closed` only so the concurrency group cancels in-flight runs; this job had no guard and no `needs`, so every closed or merged PR ran the full io_uring suite (4m17s, 7m19s and 7m31s on runs 30678272341, 30678117601 and 30662728539). - ci: gate s3-lifecycle-behavior-tests on e2e-tests, matching s3-implemented-tests. Both lanes only download the prebuilt debug binary, and s3-implemented-tests already finishes later, so a green PR's wall clock is unchanged; a red one stops holding a sm-standard-4 for up to 30 minutes. Refs: rustfs/backlog#1598, rustfs/backlog#1599 * ci: gate the seven expensive jobs on Quick Checks Every expensive job started in parallel with quick-checks, so a formatting or architecture-guard failure still paid for the full pipeline. On run 30673292690 Quick Checks failed after 0.8 minutes and the run went on to burn 424.5 runner-minutes — 99.8% of it after the gate had already failed. The self-hosted pool is 15-21 ARC runners and one full PR run needs about seven sm-standard-4 concurrently, so those minutes come straight out of other PRs' queue time (six runs measured 72-488 minutes queued). quick-checks itself is compile-free and takes 45-51s, so a passing PR pays about a minute of extra critical path. REQUIRES the branch ruleset to list "Quick Checks" as a required check BEFORE this merges. Adding `needs` gives these jobs a `skipped` conclusion for the first time, and GitHub treats a skipped required check as satisfied — with required_approving_review_count=0, a failing quick-checks would otherwise let a broken PR merge. Ordering is tracked in rustfs/backlog#1599. Refs: rustfs/backlog#1598, rustfs/backlog#1599 * ci: stop a PR run once Test and Lint has failed (#5530) On run 30674613104 the e2e, ILM and sftp lanes had all failed while Test and Lint and the rio-v2 variant kept running past 70 minutes. The run's verdict was settled; the remaining lanes were spending sm-standard-4 time on a result nobody could act on, and with one full PR run needing about seven of those runners, that time comes out of other PRs' queue time. Two mechanisms, both scoped to pull_request so main pushes, the merge queue and the weekly schedule keep the full failure signal: - test-and-lint-protocols: fail-fast on PRs, so one failing protocol leg stops its sibling. This is the only part that also covers fork PRs, since it needs no token. - test-and-lint: on failure, cancel the run through the REST API. Only test-and-lint may cancel. The lanes that are not required checks (protocols, ILM, e2e, s3-tests) must never hold that power: a flake in one of them would turn the required "Test and Lint" into `cancelled`, which blocks the merge. A maintainer can merge today with sftp red, and that has to stay true. The cancel step uses curl, not `gh`: every existing `gh` call in this repo runs on ubuntu-latest, and the sm-standard-* images are custom and trimmed, so `gh` is not known to exist there. Fork PRs are excluded by an explicit condition rather than left to fail, since their GITHUB_TOKEN is forced read-only and job-level permissions cannot raise it. Job-level permissions must list contents: read alongside actions: write — job-level permissions replace the workflow block instead of merging with it, and dropping contents would break this job's checkout and the repo-token the setup action passes to setup-protoc. Because that token can now cancel runs and delete Actions caches, the checkout also sets persist-credentials: false so a PR's own build.rs or proc-macro cannot read it back out of .git/config. Refs: rustfs/backlog#1598, rustfs/backlog#1599 * ci: correct the companion-workflow comments Addresses review feedback on #5528, which merged before these fixes were pushed. The ci-docs-only header claimed the ruleset already requires "Quick Checks". It does not — that ruleset change is a separate step, and this file's whole purpose is to land first so that change does not strand docs-only PRs. Say what is true today. Also move the byte-identical requirement onto ci.yml's quick-checks job, which is the more likely edit site, instead of pointing at a comment that was not there. --- .github/workflows/ci-docs-only.yml | 22 +++++---- .github/workflows/ci.yml | 73 +++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci-docs-only.yml b/.github/workflows/ci-docs-only.yml index ecd4419a7..01e0db124 100644 --- a/.github/workflows/ci-docs-only.yml +++ b/.github/workflows/ci-docs-only.yml @@ -12,20 +12,24 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Companion to ci.yml for the required "Test and Lint" and "Quick Checks" -# status checks. +# Companion to ci.yml for required status checks. # -# ci.yml skips docs-only pull requests via paths-ignore, but the branch -# ruleset requires checks named "Test and Lint" and "Quick Checks" — without -# this workflow a docs-only PR would wait on those checks forever. This -# workflow triggers on exactly the paths ci.yml ignores and reports success -# under the same job names. Mixed PRs trigger both workflows and the real -# checks still gate: a required check with any failing run blocks the merge. +# ci.yml skips docs-only pull requests via paths-ignore, but the branch ruleset +# requires a check named "Test and Lint" — without this workflow a docs-only PR +# would wait on it forever. This workflow triggers on exactly the paths ci.yml +# ignores and reports success under the same job name. Mixed PRs trigger both +# workflows and the real check still gates: a required check with any failing +# run blocks the merge. # https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks # +# "Quick Checks" is mirrored here ahead of the ruleset change that will make it +# required too (rustfs/backlog#1599). Until that change lands this job is +# inert; mirroring it first is what lets the ruleset change happen without +# stranding docs-only PRs on a check nobody reports. +# # Keep the paths list below in sync with the pull_request paths-ignore list # in ci.yml, and keep the quick-checks steps below byte-identical to the -# quick-checks job in ci.yml (see the comment on that job). +# quick-checks job in ci.yml. name: Continuous Integration (docs only) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 096a9cffd..450ef6ac1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,6 +96,10 @@ jobs: # Fast, compile-free checks that fail early so contributors get feedback in # ~1 minute instead of waiting for the full test job. + # + # These steps are mirrored byte-for-byte in ci-docs-only.yml so that a mixed + # PR, which reports two check runs named "Quick Checks", cannot get one red + # and one green. Edit both jobs together. quick-checks: name: Quick Checks if: github.event_name != 'pull_request' || github.event.action != 'closed' @@ -140,13 +144,26 @@ jobs: test-and-lint: name: Test and Lint if: github.event_name != 'pull_request' || github.event.action != 'closed' + needs: [ quick-checks ] runs-on: sm-standard-4 timeout-minutes: 90 + # Both lines are required. Job-level `permissions` replaces the workflow + # block rather than merging with it, so declaring only `actions: write` + # would drop `contents: read` and break this job's checkout and the + # repo-token the setup action hands to setup-protoc. + permissions: + contents: read + actions: write env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" steps: - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + with: + # This job's token can cancel runs and delete Actions caches. Checkout + # otherwise writes it into .git/config, where a PR's own build.rs or + # proc-macro could read it back out. + persist-credentials: false - name: Setup Rust environment uses: ./.github/actions/setup @@ -277,6 +294,48 @@ jobs: - name: Run rebalance/decommission migration proofs run: ./scripts/check_migration_gate_count.sh + # Early stop. Once this job has failed the PR cannot merge, so the sibling + # lanes are burning runners on a result nobody can act on: on run + # 30674613104 three lanes had already failed while Test and Lint and the + # rio-v2 variant kept going past 70 minutes. + # + # Only this job may cancel. The lanes that are NOT required checks + # (protocols, ILM, e2e, s3-tests) must never hold that power: a flake in + # one of them would turn the required "Test and Lint" into `cancelled`, + # which blocks the merge. Today a maintainer can merge with sftp red, and + # that has to stay true. + # + # These steps run last so the `if: always()` artifact upload above still + # captures logs and diagnostics before the run goes away. + - name: Annotate early-stop reason + if: failure() && github.event_name == 'pull_request' + run: | + echo "## CI early-stop" >> "$GITHUB_STEP_SUMMARY" + echo "Job \`${GITHUB_JOB}\` (Test and Lint) failed; cancelling run ${GITHUB_RUN_ID} to free runners." >> "$GITHUB_STEP_SUMMARY" + echo "Sibling jobs showing **cancelled** were stopped by this job, not by their own failure." >> "$GITHUB_STEP_SUMMARY" + + # curl rather than `gh`: every existing `gh` call in this repo runs on + # ubuntu-latest, and the sm-standard-* images are custom and trimmed (they + # ship no C toolchain, see the e2e job below), so `gh` is not known to + # exist here. + # + # Fork PRs are excluded explicitly instead of relying on the error path: + # their GITHUB_TOKEN is forced read-only and job-level permissions cannot + # raise it, so the call would always 403. Skipping keeps their logs clean. + - name: Cancel run on failure (same-repo PR only) + if: >- + failure() && github.event_name == 'pull_request' + && github.event.pull_request.head.repo.full_name == github.repository + continue-on-error: true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + curl -fsS -X POST \ + -H "Authorization: Bearer ${GH_TOKEN}" \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}/cancel" || true + # 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 @@ -290,6 +349,7 @@ jobs: test-ilm-integration-serial: name: ILM Integration (serial) if: github.event_name != 'pull_request' || github.event.action != 'closed' + needs: [ quick-checks ] runs-on: sm-standard-4 timeout-minutes: 45 env: @@ -327,6 +387,7 @@ jobs: test-and-lint-rio-v2: name: Test and Lint (rio-v2) if: github.event_name != 'pull_request' || github.event.action != 'closed' + needs: [ quick-checks ] runs-on: sm-standard-4 timeout-minutes: 60 env: @@ -354,10 +415,17 @@ jobs: test-and-lint-protocols: name: "Test and Lint (${{ matrix.features.name }})" if: github.event_name != 'pull_request' || github.event.action != 'closed' + needs: [ quick-checks ] runs-on: sm-standard-4 timeout-minutes: 60 strategy: - fail-fast: false + # On a PR, one failing protocol leg is enough to know the PR is not ready, + # so stop the sibling leg instead of paying another ~40 minutes for it. + # Everywhere else (main pushes, the merge queue, the weekly schedule) keep + # the full signal: there we want to know whether swift AND sftp are broken, + # not just whichever failed first. This is the only part of the early-stop + # work that also covers fork PRs, since it needs no token. + fail-fast: ${{ github.event_name == 'pull_request' }} matrix: features: - name: swift @@ -389,6 +457,7 @@ jobs: build-rustfs-debug-binary: name: Build RustFS Debug Binary if: github.event_name != 'pull_request' || github.event.action != 'closed' + needs: [ quick-checks ] runs-on: sm-standard-4 timeout-minutes: 30 env: @@ -419,6 +488,7 @@ jobs: build-rustfs-debug-binary-rio-v2: name: Build RustFS Debug Binary (rio-v2) if: github.event_name != 'pull_request' || github.event.action != 'closed' + needs: [ quick-checks ] runs-on: sm-standard-4 timeout-minutes: 30 env: @@ -455,6 +525,7 @@ jobs: # suite (measured 4m17s / 7m19s / 7m31s on runs 30678272341 / 30678117601 / # 30662728539) and kept the cancellation run in progress for minutes. if: github.event_name != 'pull_request' || github.event.action != 'closed' + needs: [ quick-checks ] # GitHub-hosted ubuntu-latest runs a recent kernel with io_uring and, unlike # a container, applies no seccomp filter that would block io_uring_setup — so # the probe succeeds and the tests exercise the real UringBackend/FdCache/