diff --git a/.github/scripts/gate_full_suite.sh b/.github/scripts/gate_full_suite.sh new file mode 100755 index 000000000..782178f15 --- /dev/null +++ b/.github/scripts/gate_full_suite.sh @@ -0,0 +1,133 @@ +#!/usr/bin/env bash +# Gate the expensive Buildkite full suite on the cheap GitHub checks. +# +# Polls the workflow runs for the PR head commit and only exits 0 once the +# watched cheap workflows (pre-commit, docs build) have succeeded, so the +# 'ready' label cannot burn ~20 GPU lanes on a head that a cheap check has +# already doomed. +# +# Semantics: +# - watched run completed with a bad conclusion -> exit 1 (fail CLOSED: +# no full suite; the next push re-arms via the 'synchronize' trigger) +# - watched run cancelled -> still pending: the docs +# workflow's repo-global 'pages' concurrency group cancels runs superseded +# by unrelated pushes, so 'cancelled' is not a verdict on this PR +# - watched runs pending -> poll until done +# - docs run absent -> not applicable after a +# short grace period ('Deploy Documentation' is path-filtered on PRs) +# - pre-commit run absent -> keep polling: pre-commit +# is never path-filtered, so its absence is always anomalous +# - 'ready' label removed while waiting -> exit 1 (fail CLOSED: +# un-labeling is a deliberate maintainer action) +# - GitHub API unreachable or timeout -> exit 0 (fail OPEN, +# loud warning: never brick CI on a GitHub outage) +# +# Required env: PR_SHA (PR head commit), PR_NUMBER, GITHUB_REPOSITORY, GH_TOKEN. +set -euo pipefail + +: "${PR_SHA:?PR_SHA (PR head commit) is required}" +: "${PR_NUMBER:?PR_NUMBER (pull request number) is required}" +: "${GITHUB_REPOSITORY:?GITHUB_REPOSITORY is required}" + +# Workflow-level `name:` values that must be green before the full suite +# may start. "Deploy Documentation" is path-filtered on PRs, so its run may +# legitimately never exist; pre-commit always runs, so it must appear. +WATCHED_NAMES='["pre-commit", "Deploy Documentation"]' +WATCHED_REGEX='^(pre-commit|Deploy Documentation)$' +POLL_SECS="${POLL_SECS:-20}" +GRACE_SECS="${GRACE_SECS:-60}" +MAX_WAIT_SECS="${MAX_WAIT_SECS:-1500}" + +# Bound each API call so a hung connection hits the 3-strike fail-open path +# instead of pinning the loop until the job timeout (which would fail closed +# on exactly the GitHub-outage case this script is meant to survive). +if command -v timeout >/dev/null 2>&1; then + gh_api() { timeout 30 gh api "$@"; } +else + gh_api() { gh api "$@"; } # macOS dev boxes; CI always has coreutils timeout +fi + +# The workflow checked the label before starting the gate, but the wait can +# last ~25 min: re-check once before any exit 0 and fail closed if 'ready' +# was removed in the meantime. An API error here proceeds (the label was +# present when the gate started; never brick CI on an outage). +recheck_ready_label() { + local pr_json + if pr_json=$(gh_api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" 2>/dev/null); then + if ! jq -e '[.labels[]?.name] | index("ready")' <<<"$pr_json" >/dev/null 2>&1; then + echo "::error::PR #${PR_NUMBER} no longer has the 'ready' label —" \ + "NOT triggering the Buildkite full suite. Re-add the label to re-arm." + exit 1 + fi + else + echo "::warning::Could not re-check the 'ready' label on PR #${PR_NUMBER}; proceeding (it was present when the gate started)." + fi +} + +start=$(date +%s) +api_fails=0 +missing="" + +while true; do + elapsed=$(( $(date +%s) - start )) + + if runs_json=$(gh_api "repos/${GITHUB_REPOSITORY}/actions/runs?head_sha=${PR_SHA}&per_page=100" 2>/dev/null) \ + && state=$(jq --arg re "$WATCHED_REGEX" ' + [.workflow_runs[]? | select(.name // "" | test($re))] + | group_by(.name) | map(max_by(.id)) + | map({name, status, conclusion})' <<<"$runs_json" 2>/dev/null); then + api_fails=0 + echo "t+${elapsed}s watched checks: $(jq -c . <<<"$state")" + + failed=$(jq -r '[.[] | select(.status == "completed" + and (.conclusion | IN("success", "skipped", "neutral", "cancelled") | not))] + | map(.name) | join(", ")' <<<"$state") + if [ -n "$failed" ]; then + echo "::error::Cheap check(s) failed on ${PR_SHA}: ${failed}." \ + "NOT triggering the Buildkite full suite. Push a fix (the 'ready'" \ + "label re-arms on every push), or re-run the failed check and then" \ + "re-run this workflow." + exit 1 + fi + + # 'cancelled' counts as pending: wait for a re-run to reach a real verdict + # (bounded by MAX_WAIT, then the fail-open below). + pending=$(jq '[.[] | select(.status != "completed" or .conclusion == "cancelled")] | length' <<<"$state") + missing=$(jq -r --argjson watched "$WATCHED_NAMES" '($watched - map(.name)) | join(", ")' <<<"$state") + if [ "$pending" -eq 0 ]; then + if [ -z "$missing" ]; then + recheck_ready_label + echo "All watched cheap checks are green — full suite may proceed." + exit 0 + fi + case "$missing" in + *pre-commit*) + echo "pre-commit run not found for ${PR_SHA} yet; waiting (pre-commit is never path-filtered, so its absence is anomalous)." + ;; + *) + if [ "$elapsed" -ge "$GRACE_SECS" ]; then + recheck_ready_label + echo "::warning::Watched run(s) never appeared for ${PR_SHA}: ${missing} (path-filtered, likely not applicable). Proceeding on the checks that did run." + exit 0 + fi + echo "Waiting up to ${GRACE_SECS}s grace for path-filtered run(s) to appear: ${missing}." + ;; + esac + fi + else + api_fails=$(( api_fails + 1 )) + echo "::warning::GitHub API error querying workflow runs for ${PR_SHA} (attempt ${api_fails}/3)." + if [ "$api_fails" -ge 3 ]; then + recheck_ready_label + echo "::warning::FAILING OPEN: cannot query GitHub check status — triggering the full suite WITHOUT the cheap-check gate." + exit 0 + fi + fi + + if [ "$elapsed" -ge "$MAX_WAIT_SECS" ]; then + recheck_ready_label + echo "::warning::FAILING OPEN: watched checks still pending after $(( MAX_WAIT_SECS / 60 )) min${missing:+ (never appeared: ${missing})} — triggering the full suite anyway." + exit 0 + fi + sleep "$POLL_SECS" +done diff --git a/.github/scripts/test_gate_full_suite.sh b/.github/scripts/test_gate_full_suite.sh new file mode 100755 index 000000000..884ec4a02 --- /dev/null +++ b/.github/scripts/test_gate_full_suite.sh @@ -0,0 +1,122 @@ +#!/usr/bin/env bash +# Self-test for gate_full_suite.sh using a mocked `gh`. No network, runs on +# any dev box: bash .github/scripts/test_gate_full_suite.sh +set -u +here=$(cd "$(dirname "$0")" && pwd) +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT + +# Mock gh. Asserts the exact endpoint (including head_sha) it is called +# with — an endpoint typo in the gate script fails the test rather than +# silently serving canned data. On the runs endpoint it serves +# $MOCK_DIR/response_.json, sticking on the highest existing file, +# and exits 1 if none exist (simulates a GitHub API outage). On the pulls +# endpoint it serves $MOCK_DIR/pr.json, defaulting to a 'ready'-labeled PR. +cat > "$tmp/gh" <<'EOF' +#!/usr/bin/env bash +if [ "${1:-}" != "api" ]; then + echo "unexpected gh invocation: $*" >> "$MOCK_DIR/endpoint_error" + exit 2 +fi +case "${2:-}" in + "repos/o/r/actions/runs?head_sha=deadbeef&per_page=100") + n=$(( $(cat "$MOCK_DIR/count" 2>/dev/null || echo 0) + 1 )) + echo "$n" > "$MOCK_DIR/count" + while [ "$n" -gt 0 ]; do + if [ -f "$MOCK_DIR/response_$n.json" ]; then + cat "$MOCK_DIR/response_$n.json" + exit 0 + fi + n=$(( n - 1 )) + done + echo "api outage" >&2 + exit 1 + ;; + "repos/o/r/pulls/42") + if [ -f "$MOCK_DIR/pr.json" ]; then + cat "$MOCK_DIR/pr.json" + else + echo '{"labels": [{"name": "ready"}]}' + fi + ;; + *) + echo "unexpected gh endpoint: $2" >> "$MOCK_DIR/endpoint_error" + exit 2 + ;; +esac +EOF +chmod +x "$tmp/gh" + +PC_OK='{"name": "pre-commit", "id": 1, "status": "completed", "conclusion": "success"}' +PC_BAD='{"name": "pre-commit", "id": 1, "status": "completed", "conclusion": "failure"}' +PC_PENDING='{"name": "pre-commit", "id": 1, "status": "in_progress", "conclusion": null}' +DOCS_OK='{"name": "Deploy Documentation", "id": 2, "status": "completed", "conclusion": "success"}' +DOCS_BAD='{"name": "Deploy Documentation", "id": 2, "status": "completed", "conclusion": "failure"}' +DOCS_CANCELLED='{"name": "Deploy Documentation", "id": 2, "status": "completed", "conclusion": "cancelled"}' +OTHER='{"name": "Trigger Full Suite", "id": 3, "status": "in_progress", "conclusion": null}' +NULL_NAME='{"name": null, "id": 4, "status": "completed", "conclusion": "failure"}' +PC_OK_RERUN='{"name": "pre-commit", "id": 5, "status": "completed", "conclusion": "success"}' + +fails=0 +want_log="" # optional: expect() also greps out.log for this regex, then resets +pr_json="" # optional: served for the pulls (label re-check) endpoint, then resets +raw_body="" # optional: serve responses verbatim instead of wrapping in workflow_runs +expect() { # ... + local name=$1 want=$2 dir i=1 + shift 2 + dir=$(mktemp -d "$tmp/test_XXXXXX") + for body in "$@"; do + if [ -n "$raw_body" ]; then + printf '%s' "$body" > "$dir/response_$i.json" + else + printf '{"workflow_runs": [%s]}' "$body" > "$dir/response_$i.json" + fi + i=$(( i + 1 )) + done + [ -n "$pr_json" ] && printf '%s' "$pr_json" > "$dir/pr.json" + ( export PATH="$tmp:$PATH" MOCK_DIR="$dir" PR_SHA=deadbeef PR_NUMBER=42 \ + GITHUB_REPOSITORY=o/r POLL_SECS=0 GRACE_SECS=1 MAX_WAIT_SECS=3 + bash "$here/gate_full_suite.sh" > "$dir/out.log" 2>&1 ) + local rc=$? + if [ "$rc" -ne "$want" ]; then + echo "FAIL: $name (exit $rc, want $want)" + cat "$dir/out.log" + fails=1 + elif [ -f "$dir/endpoint_error" ]; then + echo "FAIL: $name (mock gh got an unexpected call)" + cat "$dir/endpoint_error" + fails=1 + elif [ -n "$want_log" ] && ! grep -Eq "$want_log" "$dir/out.log"; then + echo "FAIL: $name (log does not match: $want_log)" + cat "$dir/out.log" + fails=1 + else + echo "ok: $name" + fi + want_log="" pr_json="" raw_body="" +} + +expect "both green -> proceed" 0 "$PC_OK, $DOCS_OK, $OTHER, $NULL_NAME" +expect "docs build failed -> blocked" 1 "$PC_OK, $DOCS_BAD" +expect "pre-commit failed -> blocked" 1 "$PC_BAD" +expect "pending then green -> proceed" 0 "$PC_PENDING" "$PC_OK, $DOCS_OK" +want_log="never appeared.*Deploy Documentation" +expect "docs run absent (path-filtered) -> proceed after grace" 0 "$PC_OK" +expect "API outage -> fail open" 0 +want_log="FAILING OPEN" +expect "pending past MAX_WAIT -> fail open" 0 "$PC_PENDING" +want_log="FAILING OPEN" +expect "unrelated runs only -> no grace, fail open at MAX_WAIT" 0 "$OTHER" +expect "cancelled docs then green -> proceed" 0 \ + "$PC_OK, $DOCS_CANCELLED" "$PC_OK, $DOCS_OK" +want_log="FAILING OPEN" +expect "cancelled docs forever -> fail open at MAX_WAIT" 0 "$PC_OK, $DOCS_CANCELLED" +want_log="FAILING OPEN" +expect "pre-commit absent -> no grace, fail open at MAX_WAIT" 0 "$DOCS_OK" +expect "duplicate run names -> latest wins" 0 "$PC_BAD, $PC_OK_RERUN, $DOCS_OK" +raw_body=1 +expect "garbage response body -> fail open" 0 "this is not json" +pr_json='{"labels": [{"name": "other"}]}' +expect "ready label removed mid-gate -> blocked" 1 "$PC_OK, $DOCS_OK" + +exit "$fails" diff --git a/.github/workflows/ci-precommit.yml b/.github/workflows/ci-precommit.yml index cf9ea608b..3729f845b 100644 --- a/.github/workflows/ci-precommit.yml +++ b/.github/workflows/ci-precommit.yml @@ -47,3 +47,6 @@ jobs: - uses: pre-commit/action@v3.0.1 with: extra_args: --all-files --hook-stage manual + # After pre-commit so a self-test failure cannot mask lint failures. + - name: Full-suite gate self-test + run: bash .github/scripts/test_gate_full_suite.sh diff --git a/.github/workflows/ci-trigger-full-suite.yml b/.github/workflows/ci-trigger-full-suite.yml index 7f982fb54..d055eb73c 100644 --- a/.github/workflows/ci-trigger-full-suite.yml +++ b/.github/workflows/ci-trigger-full-suite.yml @@ -7,6 +7,7 @@ on: permissions: contents: read pull-requests: read + actions: read concurrency: group: full-suite-${{ github.event.pull_request.number }} @@ -18,6 +19,8 @@ jobs: (github.event.action == 'labeled' && github.event.label.name == 'ready') || github.event.action == 'synchronize' runs-on: ubuntu-latest + # Gate below may wait for cheap checks (up to MAX_WAIT_SECS = 25 min). + timeout-minutes: 35 steps: - name: Check ready label id: check @@ -49,6 +52,20 @@ jobs: "https://api.buildkite.com/v2/organizations/${{ vars.BUILDKITE_ORG_SLUG }}/pipelines/${{ vars.BUILDKITE_PIPELINE_SLUG }}/builds/${build_num}/cancel" done + # Checks out the BASE branch (default for pull_request_target), so PR + # authors cannot tamper with the gate script. + - name: Checkout gate script + if: steps.check.outputs.has_ready == 'true' + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Wait for pre-commit and docs build + if: steps.check.outputs.has_ready == 'true' + env: + GH_TOKEN: ${{ github.token }} + PR_SHA: ${{ github.event.pull_request.head.sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: bash .github/scripts/gate_full_suite.sh + - name: Trigger Buildkite Full Suite if: steps.check.outputs.has_ready == 'true' env: diff --git a/docs/contributing/ci_architecture.md b/docs/contributing/ci_architecture.md index bec20daf6..7afde7d9d 100644 --- a/docs/contributing/ci_architecture.md +++ b/docs/contributing/ci_architecture.md @@ -99,6 +99,13 @@ status. Full Suite is also path-filtered. It validates broader behavior before Mergify can merge a PR. +A `ready`-labeled PR does not hit Buildkite immediately: +`ci-trigger-full-suite.yml` first runs `.github/scripts/gate_full_suite.sh`, +which waits for the cheap Tier-1 checks (pre-commit, docs build) on the PR +head. A red cheap check blocks the suite (fail closed; the next push re-arms +it), while a GitHub outage or a >25 min wait lets it run anyway (fail open). +`/test full` bypasses the gate. + | Buildkite label | `TEST_TYPE` | Main watched paths | |---|---|---| | SSIM Tests | `ssim` | `fastvideo/**/*.py`, `pyproject.toml`, `docker/Dockerfile` |