Cancel the runs of merged pull requests when they merge

.github/fork-ci-gate.toml

@@ -34,11 +34,6 @@ close_after_days = 7
 # status is posted either way.
 job_statuses = true
 
-# Cancel the unfinished `pull_request` runs of pull requests that have already merged, of any
-# author, so they stop using GitHub-hosted minutes and self-hosted runners for a result nobody
-# reads. The fork CI monitor does it on its schedule.
-cancel_merged_runs = true
-
 # Organisations exempted under AIP-120 §2.2. Keys are display names used in comments,
 # values are GitHub logins. Changes go through a reviewed pull request.
 [allowlist]

.github/workflows/cancel-merged-pr-runs.yml

@@ -0,0 +1,119 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+---
+# Cancels the `pull_request` runs that are still going after their pull request merged. Merging
+# pushes to the target branch, so this starts right after the merge. A push run can cancel the runs
+# of fork pull requests too. It never checks out or runs pull request code. See "Cancelling runs of
+# merged pull requests" in dev/breeze/doc/ci/05_workflows.md.
+name: "Cancel the runs of merged pull requests"
+on:  # yamllint disable-line rule:truthy
+  push:
+    # The branches pull requests merge into, as in the `pull_request` trigger of ci-amd.yml. A push
+    # runs the workflow file of the pushed branch, so a release branch needs its own copy.
+    branches:
+      - main
+      - v[0-9]+-[0-9]+-test
+      - v[0-9]+-[0-9]+-stable
+      - providers-[a-z]+-?[a-z]*/v[0-9]+-[0-9]+
+      - chart/v[0-9]+-[0-9]+x-test
+      - chart/v[0-9]+-[0-9]+x-stable
+      - airflow-ctl/v[0-9]+-[0-9]+-test
+      - airflow-ctl/v[0-9]+-[0-9]+-stable
+permissions:
+  actions: write
+  contents: read
+  pull-requests: read
+jobs:
+  cancel:
+    name: "Cancel the runs of merged pull requests"
+    if: github.repository == 'apache/airflow'
+    runs-on: ["ubuntu-slim"]
+    timeout-minutes: 10
+    steps:
+      - name: "Cancel the runs"
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          REPO: ${{ github.repository }}
+          SHA: ${{ github.sha }}
+        run: |
+          # The statuses of a run that has not finished. A run waiting for approval is "completed".
+          statuses=(queued in_progress waiting pending requested)
+          # Runs of a fork pull request do not list their pull request, so runs are matched by the
+          # head commit and repository. Only runs created before the merge count: workflows
+          # triggered by the merge itself, for example on `closed`, are meant to run.
+          list_runs() {
+            local head_repo=$1 head_sha=$2 merged_at=$3 status
+            for status in "${statuses[@]}"; do
+              gh api --paginate \
+                "repos/${REPO}/actions/runs?event=pull_request&head_sha=${head_sha}&status=${status}" \
+                --jq ".workflow_runs[] | select(.head_repository.full_name == \"${head_repo}\"
+                  and .created_at < \"${merged_at}\") | .id" || return 1
+            done
+          }
+          # Lookups run in command substitutions rather than process substitutions, so that a failed
+          # one fails the job instead of looking like nothing to cancel. GitHub associates the merge
+          # commit with its pull request some seconds after the push, as automatic-backport.yml
+          # also notes; a push that is not a merge finds nothing after a minute.
+          for _ in {1..6}; do
+            merged=$(gh api "repos/${REPO}/commits/${SHA}/pulls" --jq '.[]
+              | select(.merged_at != null)
+              | [.number, .head.repo.full_name, .head.sha, .merged_at] | @tsv')
+            [[ -n "${merged}" ]] && break
+            sleep 10
+          done
+          to_cancel=()
+          while IFS=$'\t' read -r number head_repo head_sha merged_at; do
+            [[ -n "${number}" ]] || continue
+            # The same commit can be the head of another pull request that is still open.
+            open=$(gh pr list -R "${REPO}" --state open --search "${head_sha}" --json headRefOid \
+              --jq "[.[] | select(.headRefOid == \"${head_sha}\")] | length")
+            if [[ "${open}" != 0 ]]; then
+              echo "#${number}: ${head_sha} is the head of an open pull request, nothing cancelled"
+              continue
+            fi
+            runs=$(list_runs "${head_repo}" "${head_sha}" "${merged_at}")
+            mapfile -t ids <<< "${runs}"
+            for id in "${ids[@]}"; do
+              [[ -n "${id}" ]] || continue
+              echo "#${number}: cancel run ${id}"
+              # A run that finished meanwhile cannot be cancelled (HTTP 409); that is fine.
+              gh api -X POST "repos/${REPO}/actions/runs/${id}/cancel" > /dev/null || true
+              to_cancel+=("${id}")
+            done
+          done <<< "${merged}"
+          # A job that ignores the cancellation keeps its runner, so a run still going after five
+          # minutes is force-cancelled.
+          for _ in {1..10}; do
+            remaining=()
+            for id in "${to_cancel[@]}"; do
+              [[ "$(gh api "repos/${REPO}/actions/runs/${id}" --jq .status)" == completed ]] \
+                || remaining+=("${id}")
+            done
+            to_cancel=("${remaining[@]}")
+            [[ ${#to_cancel[@]} == 0 ]] && exit 0
+            sleep 30
+          done
+          failed=0
+          for id in "${to_cancel[@]}"; do
+            echo "force-cancel run ${id}"
+            if ! out=$(gh api -X POST "repos/${REPO}/actions/runs/${id}/force-cancel" 2>&1); then
+              echo "${out}"
+              [[ "${out}" == *"(HTTP 409)"* ]] || failed=1
+            fi
+          done
+          exit "${failed}"

.github/workflows/fork-ci-monitor.yml

@@ -41,8 +41,8 @@ jobs:
     name: "Reconcile pull requests gated to fork CI"
     if: github.repository == 'apache/airflow'
     permissions:
-      # Listing the runs and jobs of the gate tampering check, deleting the runs that only gated a
-      # pull request to fork CI, and cancelling the unfinished runs of merged pull requests.
+      # Listing the runs and jobs of the gate tampering check, and deleting the runs that only
+      # gated a pull request to fork CI.
       actions: write
       contents: read
       # Labels, comments and closing of gated pull requests.

dev/breeze/doc/08_ci_tasks.rst

@@ -188,8 +188,7 @@ Pull requests from external contributors can run their CI in the contributor's f
 gated pull requests ``fork ci required``, keeps them in draft until the fork run is green, maintains
 one state comment and a ``Fork CI / Tests (AMD)`` commit status linking to the run in the fork, closes
 pull requests without fork CI activity for a week, closes gated pull requests that made project CI run
-by changing workflows, and writes everything it could not classify to a Slack message. It also cancels
-the unfinished workflow runs of pull requests, of any author, that have already merged. With
+by changing workflows, and writes everything it could not classify to a Slack message. With
 ``--dry-run`` it prints the changes instead of applying them. See
 `Fork CI for external contributors <ci/08_fork_ci_for_external_contributors.md>`_ for the design.
 

dev/breeze/doc/ci/05_workflows.md

@@ -244,6 +244,31 @@ Each composite workflow is stored in `.github/workflows/` and gets called using
 
 GitHub Actions includes a concurrency feature that we use to prevent wasted resources. If you push new commits while tests are already running, the system cancels the old test run and starts over with your latest code. This saves time and compute resources.
 
+### Cancelling runs of merged pull requests
+
+Merging a pull request does not stop its `pull_request` runs, and the concurrency group does not
+help, because the merge starts no new run in the pull request's group. Maintainers often merge
+once the checks they need are green, while slower jobs are still queued or running for a result
+nobody reads.
+
+Merging pushes to the target branch, which starts `cancel-merged-pr-runs.yml`. It looks up the
+merged pull request of the pushed commit and cancels its `pull_request` runs that are `queued`,
+`in_progress`, `waiting`, `pending` or `requested`. A push run has a token that can write to
+`apache/airflow`, so it cancels the runs of fork pull requests too. A run is cancelled when:
+
+- its head repository and commit are the merged pull request's (runs of pull requests from forks
+  do not list their pull request, so they are matched this way);
+- no open pull request has the same head commit; such runs are left to finish, since no later push
+  revisits them;
+- it was created before the merge. A workflow triggered by the merge itself, such as one on the
+  `closed` event, is meant to run.
+
+A run still not finished five minutes after the cancel, for example because a step ignores the
+cancellation, is force-cancelled. A run of an earlier commit of the pull request is normally
+cancelled by its concurrency group when the next commit is pushed. A push runs the workflow file
+of the pushed branch, so the cancellation works on a release branch only once the workflow is on
+that branch.
+
 ## Branch-Specific Behavior
 
 Different branches in our repository serve different purposes, so they get different levels of testing.

dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md

@@ -833,36 +833,6 @@ name within its time limit, and its run is the evidence. A re-run is kept too: d
 deletes every attempt, and an earlier attempt may have run project CI. Nothing is deleted for a
 pull request in the `gate_bypassed` state. A dry run only reports the deletions.
 
-### Cancelling runs of merged pull requests
-
-Merging a pull request does not stop its `pull_request` runs. They keep using `apache/airflow`'s
-capacity, GitHub-hosted minutes from the ASF's allocation and the self-hosted runners, for a
-result nobody reads once the change is on its target branch. This is not specific to fork CI; the
-monitor does it because it already runs every 15 minutes with `actions: write`.
-
-Each monitor run lists the `pull_request` runs of `apache/airflow` that are `queued`,
-`in_progress`, `waiting`, `pending` or `requested`, and looks up the pull requests of their head
-branches in one GraphQL query per 25 branches, paging through every pull request of a branch: a
-common branch name such as `main` has pull requests from many forks, and an open one with the
-run's head must not be missed. A run is cancelled when:
-
-- a merged pull request has the run's head repository, branch and commit (runs of pull requests
-  from forks do not list their pull request, so they are matched this way);
-- no open pull request has the same head, since a branch can be reused for a new pull request;
-- the run was created before the merge. A workflow triggered by the merge itself, such as one
-  on the `closed` event, is meant to run.
-
-It applies to the pull requests of every author, not only gated ones. A run is cancelled with
-`POST /repos/apache/airflow/actions/runs/<id>/cancel`; one still `in_progress` 30 minutes after the
-merge, for example because a step ignores the cancellation, is force-cancelled
-(`.../force-cancel`). A run that finished in the meantime (HTTP 409) is not a problem; any other
-failure is reported to Slack with the pull request, and a failure of the whole pass is reported
-as a note, without affecting the reconciliation of gated pull requests. A dry run only reports
-the cancellations. `cancel_merged_runs = false` in `.github/fork-ci-gate.toml` turns it off.
-
-Only runs of the merged head commit are matched. A run of an earlier commit of the pull request is
-normally cancelled by the workflow's concurrency group when the next commit is pushed.
-
 ### Far behind the base branch
 
 A fork run tests the pushed commit as it is, while project CI tests it merged with the base
@@ -917,8 +887,7 @@ Per run, with ~700 gated pull requests: ~28 GraphQL calls for pass 2, ~14 for fo
 calls for the recent `pull_request` runs, one REST call per base branch for its workflow
 files, a jobs call and a compare call only per long `ci-amd.yml` run of a gated pull request,
 one changed-files call per head commit that turns green, plus one mutation per actual state
-change. Cancelling runs of merged pull requests adds five paginated run listings, one GraphQL
-query per 25 head branches with unfinished runs, and one mutation per cancelled run. Job statuses add a jobs call and a status call per pull request with a fork run in
+change. Job statuses add a jobs call and a status call per pull request with a fork run in
 progress or a completed run not yet reported, and up to two status mutations per job, at most
 `MAX_JOB_STATUSES_PER_RUN` per run; see "Write budget". Well inside
 the `GITHUB_TOKEN` limits at a 15-minute cadence. The reconciler logs its call counts so the

dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py

@@ -129,7 +129,6 @@ class GateConfig:
     allowlist: Mapping[str, tuple[str, ...]] = field(default_factory=dict)
     pilot: Mapping[str, datetime] = field(default_factory=dict)
     job_statuses: bool = True
-    cancel_merged_runs: bool = True
 
     def is_allowlisted(self, login: str) -> bool:
         wanted = login.casefold()
@@ -178,17 +177,13 @@ def parse_config(text: str) -> GateConfig:
     job_statuses = data.get("job_statuses", True)
     if not isinstance(job_statuses, bool):
         raise ValueError(f"job_statuses must be true or false, got {job_statuses!r}")
-    cancel_merged_runs = data.get("cancel_merged_runs", True)
-    if not isinstance(cancel_merged_runs, bool):
-        raise ValueError(f"cancel_merged_runs must be true or false, got {cancel_merged_runs!r}")
     return GateConfig(
         enabled=enabled,
         enabled_since=enabled_since,
         close_after_days=close_after_days,
         allowlist=allowlist,
         pilot=pilot,
         job_statuses=job_statuses,
-        cancel_merged_runs=cancel_merged_runs,
     )
 
 

dev/breeze/src/airflow_breeze/utils/fork_ci_github.py

@@ -172,14 +172,6 @@ def delete_run(self, pr: int, run_id: int) -> None:
             None,
         )
 
-    def cancel_run(self, pr: int, run_id: int, *, force: bool = False) -> None:
-        action = "force-cancel" if force else "cancel"
-        self._mutate(
-            f"#{pr}: {action} workflow run {run_id}",
-            ["api", "-X", "POST", f"repos/{self.repo}/actions/runs/{run_id}/{action}"],
-            None,
-        )
-
     def set_status(
         self,
         pr: int,

dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py

@@ -74,7 +74,6 @@
     tamper_close_comment,
     ungated_comment,
 )
-from airflow_breeze.utils.merged_pull_request_runs import cancel_merged_pull_request_runs
 
 CI_WORKFLOW_FILE = "ci-amd.yml"
 # The files the gate job of ci-amd.yml reads from a pull request's base branch.
@@ -1548,13 +1547,6 @@ def run_reconcile(
             unknown.append((listed.number, f"{type(e).__name__}: {e}"))
 
     notes = [bypass_finder.incomplete] if bypass_finder.incomplete else []
-    if config.cancel_merged_runs:
-        # Not tied to gated pull requests; a failure here must not hide the reconciliation's own.
-        try:
-            unknown.extend(cancel_merged_pull_request_runs(client, repo=repo, now=now))
-        except Exception as e:
-            console_print(f"[error]Cancelling runs of merged pull requests failed: {e}[/]")
-            notes.append(f"cancelling runs of merged pull requests failed: {type(e).__name__}: {e}")
     if job_reporter.deferred:
         console_print(
             f"[info]Job statuses of {len(job_reporter.deferred)} pull requests left for the next run: "