Skip to content

Commit 6793e23

Browse files
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: ``` <name-of-running-workflow>-<name-of-triggering-workflow>-<name-of-event>-<name-of-head-branch> ``` 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.
1 parent 7ba7720 commit 6793e23

File tree

12 files changed

+33
-25
lines changed

12 files changed

+33
-25
lines changed

.github/workflows/README.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,25 @@ Some architectural notes about key decisions and concepts in our workflows:
1818
- **head commit**: The HEAD commit in the pull request's branch. Same as `github.event.pull_request.head.sha`.
1919
- **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`.
2020
- **target commit**: The base branch's parent of the "test merge commit" to compare against.
21+
22+
## Concurrency Groups
23+
24+
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.
25+
When two workflows are in the same group, a newer workflow cancels an older workflow.
26+
Thus, it is important how to construct the group keys:
27+
28+
- 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`.
29+
30+
- 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.
31+
32+
- 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.
33+
34+
- Of course, we run multiple workflows at the same time, so we add `github.workflow` to the key. Otherwise workflows would cancel each other.
35+
36+
- 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.
37+
38+
This results in a key with the following semantics:
39+
40+
```
41+
<running-workflow>-<triggering-workflow>-<triggered-event>-<pull-request/fallback>
42+
```

.github/workflows/backport.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ on:
1010
types: [closed, labeled]
1111

1212
concurrency:
13-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
13+
group: backport-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
1414
cancel-in-progress: true
1515

1616
permissions:

.github/workflows/build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77
pull_request_target:
88

99
concurrency:
10-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
10+
group: build-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
1111
cancel-in-progress: true
1212

1313
permissions: {}

.github/workflows/check.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77
pull_request_target:
88

99
concurrency:
10-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
10+
group: check-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
1111
cancel-in-progress: true
1212

1313
permissions: {}

.github/workflows/codeowners-v2.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ on:
3030
types: [opened, ready_for_review, synchronize, reopened]
3131

3232
concurrency:
33-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
33+
group: codeowners-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
3434
cancel-in-progress: true
3535

3636
permissions: {}

.github/workflows/dismissed-review.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77
types: [completed]
88

99
concurrency:
10-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
10+
group: dismissed-review-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
1111
cancel-in-progress: true
1212

1313
permissions:

.github/workflows/edited.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ on:
1717
types: [edited]
1818

1919
concurrency:
20-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
20+
group: edited-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
2121
cancel-in-progress: true
2222

2323
permissions: {}

.github/workflows/eval-aliases.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77
pull_request_target:
88

99
concurrency:
10-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
10+
group: eval-aliases-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
1111
cancel-in-progress: true
1212

1313
permissions: {}

.github/workflows/eval.yml

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ on:
1717
- python-updates
1818

1919
concurrency:
20-
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
20+
group: eval-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
2121
cancel-in-progress: true
2222

2323
permissions: {}
@@ -256,8 +256,6 @@ jobs:
256256
permissions:
257257
issues: write
258258
pull-requests: write
259-
with:
260-
caller: ${{ github.workflow }}
261259

262260
reviewers:
263261
name: Reviewers
@@ -268,5 +266,3 @@ jobs:
268266
if: needs.prepare.outputs.targetSha
269267
uses: ./.github/workflows/reviewers.yml
270268
secrets: inherit
271-
with:
272-
caller: ${{ github.workflow }}

.github/workflows/labels.yml

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,14 @@ name: "Label PR"
77

88
on:
99
workflow_call:
10-
inputs:
11-
caller:
12-
description: Name of the calling workflow.
13-
required: true
14-
type: string
1510
workflow_run:
1611
workflows:
1712
- Review dismissed
1813
- Review submitted
1914
types: [completed]
2015

2116
concurrency:
22-
group: ${{ inputs.caller }}-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
17+
group: labels-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
2318
cancel-in-progress: true
2419

2520
permissions:

0 commit comments

Comments
 (0)