From 4aa51a994c09f697b100cbc8286bf1dbd68fa63e Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Fri, 13 Jun 2025 15:17:33 +0200 Subject: [PATCH] workflows/{labels,reviewers}: fix concurrency groups for nested workflows This didn't work as intended. When a workflow is run with `workflow_call`, it will have `github.workflow` set to the *parent* workflow. So the `caller` input that we passed, resulted in this concurrency key: ``` Eval-Eval-... ``` But that's bad, because the labels and reviewers workflows will cancel each other! What we actually want is this: - Label and Reviewers workflow should have different groups. - Reviewers called via Eval and called directly via undraft should have *different* groups. We can't use the default condition we use everywhere else, because `github.workflow` is the same for Label and Reviewers. Thus, we hardcode the workflow's name as well. This essentially means we have this as a key: ``` --- ``` This should do what we want. Since workflows can be made reusable workflows later on, we add those hardcoded names to *all* concurrency groups. This avoids copy&paste errors later on. (cherry picked from commit 6793e238faeffcb58f1f6e1241c7390649fb8197) --- .github/workflows/README.md | 22 ++++++++++++++++++++++ .github/workflows/backport.yml | 2 +- .github/workflows/build.yml | 2 +- .github/workflows/check.yml | 2 +- .github/workflows/codeowners-v2.yml | 2 +- .github/workflows/dismissed-review.yml | 2 +- .github/workflows/edited.yml | 2 +- .github/workflows/eval-aliases.yml | 2 +- .github/workflows/eval.yml | 6 +----- .github/workflows/labels.yml | 7 +------ .github/workflows/lint.yml | 2 +- .github/workflows/reviewers.yml | 7 +------ 12 files changed, 33 insertions(+), 25 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 7089501d5e40..78303c2c64b8 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -18,3 +18,25 @@ Some architectural notes about key decisions and concepts in our workflows: - **head commit**: The HEAD commit in the pull request's branch. Same as `github.event.pull_request.head.sha`. - **merge commit**: The temporary "test merge commit" that GitHub Actions creates and updates for the pull request. Same as `refs/pull/${{ github.event.pull_request.number }}/merge`. - **target commit**: The base branch's parent of the "test merge commit" to compare against. + +## Concurrency Groups + +We use [GitHub's Concurrency Groups](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/control-the-concurrency-of-workflows-and-jobs) to cancel older jobs on pushes to Pull Requests. +When two workflows are in the same group, a newer workflow cancels an older workflow. +Thus, it is important how to construct the group keys: + +- Because we want to run jobs for different events at same time, we add `github.event_name` to the key. This is the case for the `pull_request` which runs on changes to the workflow files to test the new files and the same workflow from the base branch run via `pull_request_event`. + +- We don't want workflows of different Pull Requests to cancel each other, so we include `github.event.pull_request.number`. The [GitHub docs](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/control-the-concurrency-of-workflows-and-jobs#example-using-a-fallback-value) show using `github.head_ref` for this purpose, but this doesn't work well with forks: Different users could have the same head branch name in their forks and run CI for their PRs at the same time. + +- Sometimes, there is no `pull_request.number`. That's the case for `push` or `workflow_run` events. To ensure non-PR runs are never cancelled, we add a fallback of `github.run_id`. This is a unique value for each workflow run. + +- Of course, we run multiple workflows at the same time, so we add `github.workflow` to the key. Otherwise workflows would cancel each other. + +- There is a special case for reusable workflows called via `workflow_call` - they will have `github.workflow` set to their parent workflow's name. Thus, they would cancel each other. That's why we additionally hardcode the name of the workflow as well. + +This results in a key with the following semantics: + +``` +--- +``` diff --git a/.github/workflows/backport.yml b/.github/workflows/backport.yml index c5ba0126aeb2..72b8482ef439 100644 --- a/.github/workflows/backport.yml +++ b/.github/workflows/backport.yml @@ -10,7 +10,7 @@ on: types: [closed, labeled] concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: backport-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5013c7da1524..a46a382ff42d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,7 +7,7 @@ on: pull_request_target: concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: build-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {} diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 3bcac620c7ed..dfe5999df23f 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -7,7 +7,7 @@ on: pull_request_target: concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: check-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {} diff --git a/.github/workflows/codeowners-v2.yml b/.github/workflows/codeowners-v2.yml index f8f909aff6d7..947f441f2e60 100644 --- a/.github/workflows/codeowners-v2.yml +++ b/.github/workflows/codeowners-v2.yml @@ -30,7 +30,7 @@ on: types: [opened, ready_for_review, synchronize, reopened] concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: codeowners-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {} diff --git a/.github/workflows/dismissed-review.yml b/.github/workflows/dismissed-review.yml index 82cfe75dc3fc..e8ab48bda075 100644 --- a/.github/workflows/dismissed-review.yml +++ b/.github/workflows/dismissed-review.yml @@ -7,7 +7,7 @@ on: types: [completed] concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: dismissed-review-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: diff --git a/.github/workflows/edited.yml b/.github/workflows/edited.yml index 23e7afb624a1..49fccb5f48ba 100644 --- a/.github/workflows/edited.yml +++ b/.github/workflows/edited.yml @@ -17,7 +17,7 @@ on: types: [edited] concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: edited-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {} diff --git a/.github/workflows/eval-aliases.yml b/.github/workflows/eval-aliases.yml index 91930ea3d6e3..599d82a93e51 100644 --- a/.github/workflows/eval-aliases.yml +++ b/.github/workflows/eval-aliases.yml @@ -7,7 +7,7 @@ on: pull_request_target: concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: eval-aliases-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {} diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 447561dca73c..dd7c433815dc 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -17,7 +17,7 @@ on: - python-updates concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: eval-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {} @@ -256,8 +256,6 @@ jobs: permissions: issues: write pull-requests: write - with: - caller: ${{ github.workflow }} reviewers: name: Reviewers @@ -268,5 +266,3 @@ jobs: if: needs.prepare.outputs.targetSha uses: ./.github/workflows/reviewers.yml secrets: inherit - with: - caller: ${{ github.workflow }} diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 225c913c08b2..1a8595352950 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -7,11 +7,6 @@ name: "Label PR" on: workflow_call: - inputs: - caller: - description: Name of the calling workflow. - required: true - type: string workflow_run: workflows: - Review dismissed @@ -19,7 +14,7 @@ on: types: [completed] concurrency: - group: ${{ inputs.caller }}-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: labels-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b5ed0c4323c4..3b212a18e0e0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,7 +7,7 @@ on: pull_request_target: concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: lint-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {} diff --git a/.github/workflows/reviewers.yml b/.github/workflows/reviewers.yml index 1a47e38d7b0f..894da05c32e1 100644 --- a/.github/workflows/reviewers.yml +++ b/.github/workflows/reviewers.yml @@ -10,14 +10,9 @@ on: pull_request_target: types: [ready_for_review] workflow_call: - inputs: - caller: - description: Name of the calling workflow. - required: true - type: string concurrency: - group: ${{ inputs.caller }}-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + group: reviewers-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true permissions: {}