Skip to content

Commit

Permalink
Add an option for easily avoiding deadlocks in matrix use (#767)
Browse files Browse the repository at this point in the history
  • Loading branch information
kachick committed Jun 7, 2024
1 parent 491543b commit dcfa392
Show file tree
Hide file tree
Showing 11 changed files with 544 additions and 120 deletions.
70 changes: 70 additions & 0 deletions .github/workflows/GH-761-matrix.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
name: GH-761 - Matrix
on:
push:
branches: [main]
paths:
- '**GH-761**'
- 'action.yml'
- 'dist/**'
pull_request:
paths:
- '**GH-761**'
- 'action.yml'
- 'dist/**'

# Disable all permissions in workflow global as to setup clean room
# However PRs will have read permissions because this project is on a public repository
permissions: {}

jobs:
echo:
runs-on: ubuntu-24.04
timeout-minutes: 5
steps:
- run: |
sleep 10
echo ':)'
wait:
strategy:
fail-fast: false
matrix:
os:
- ubuntu-24.04
- ubuntu-22.04
wait-seconds-before-first-polling:
- '3'
- '7'
runs-on: ${{ matrix.os }}
# name: # By default, GitHub sets with `github.job` and the matrix values
timeout-minutes: 10
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
- uses: ./
id: wait
continue-on-error: true
with:
retry-method: 'equal_intervals'
min-interval-seconds: 5
wait-seconds-before-first-polling: '${{ matrix.wait-seconds-before-first-polling }}'
skip-same-workflow: 'false' # Intentionally set false to test skip list also can cover this use case
# Should specify jobName to test details
# But we should pattern likes regex for the jobName
skip-list: |
[
{
"workflowFile": "itself.yml"
},
{
"workflowFile": "GH-761-matrix.yml",
"jobName": "${{ github.job }}",
"jobMatchMode": "prefix"
},
{
"workflowFile": "merge-bot-pr.yml"
}
]
- name: Upload dumps as an artifact
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
with:
name: 'outputs-${{ github.job }}-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ matrix.os }}-${{ matrix.wait-seconds-before-first-polling }}'
path: '${{ steps.wait.outputs.dump }}'
2 changes: 0 additions & 2 deletions .github/workflows/GH-771-eventname.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ permissions: {}
jobs:
echo:
runs-on: ubuntu-24.04
if: ${{ github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' }}
timeout-minutes: 5
steps:
- name: Print note
Expand All @@ -27,7 +26,6 @@ jobs:
echo 'See https://github.com/kachick/wait-other-jobs/issues/771 for the detail'
wait:
runs-on: ubuntu-24.04
if: ${{ github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' }}
timeout-minutes: 5
steps:
- run: |
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
This file only records notable changes. Not synchronized with all releases and tags.

- main - not yet released
- Nothing
- Add `jobMatchMode` option for both wait and skip list
- Add `dump` outputs
- v3.3.0
- Add `startupGracePeriod` option in wait-list: [#820](https://github.com/kachick/wait-other-jobs/issues/820)
- Restrict `wait-seconds-before-first-polling` if it is too short as zero or shorter than `startupGracePeriod`
Expand Down
70 changes: 48 additions & 22 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,28 +104,54 @@ See the [docs](docs/examples.md) for further detail.

## Deadlocks

- If you use this action in multiple jobs on the same repository, you should avoid deadlocks.\
The `skip-list`, `wait-list` and `skip-same-workflow` options cover this use case.

- If you changed job name from the default, you should specify with `skip-list` or use `skip-same-workflow`
```yaml
jobs:
your_job: # This will be used default job name if you not specify below "name" field
name: "Changed at here"
runs-on: ubuntu-24.04
steps:
- uses: kachick/wait-other-jobs@v3
with:
skip-list: |
[
{
"workflowFile": "this_file_name_here.yml",
"jobName": "Changed at here"
}
]
timeout-minutes: 15
```
Similar problems should be considered in matrix jobs. See [#761](https://github.com/kachick/wait-other-jobs/issues/761) for further detail
If you use this action in multiple jobs on the same repository, you should avoid deadlocks.\
The `skip-list`, `wait-list` and `skip-same-workflow` options cover this use case.

If you changed job name from the default, you should set `skip-list` or roughly use `skip-same-workflow`

```yaml
jobs:
your_job: # This will be used default job name if you not specify below "name" field
name: "Changed at here"
runs-on: ubuntu-24.04
steps:
- uses: kachick/wait-other-jobs@v3.4.0
with:
skip-list: |
[
{
"workflowFile": "this_file_name.yml",
"jobName": "Changed at here"
}
]
timeout-minutes: 15
```

Similar problems should be considered in matrix use.\
Since v3.4.0, you can set `prefix` for `jobMatchMode` to make small list.

```yaml
jobs:
your_job:
strategy:
matrix:
os:
- ubuntu-24.04
- ubuntu-22.04
runs-on: ${{ matrix.os }}
steps:
- uses: kachick/wait-other-jobs@v3.4.0
with:
skip-list: |
[
{
"workflowFile": "this_file_name.yml",
"jobMatchMode": "prefix",
"jobName": "${{ github.job }}"
}
]
- run: make test
```

## Startup grace period

Expand Down
93 changes: 68 additions & 25 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31063,22 +31063,33 @@ function getDuration(durationable) {
}
throw new Error("unexpected value is specified in durations");
}
var FilterCondition = z2.object({
workflowFile: z2.string().endsWith(".yml"),
jobName: z2.string().min(1).optional()
});
var workflowFile = z2.string().endsWith(".yml");
var matchAllJobs = z2.object({
workflowFile,
jobName: z2.undefined(),
// Preferring undefined over null for backward compatibility
jobMatchMode: z2.literal("all").default("all")
}).strict();
var matchPartialJobs = z2.object({
workflowFile,
jobName: z2.string().min(1),
jobMatchMode: z2.enum(["exact", "prefix"]).default("exact")
}).strict();
var FilterCondition = z2.union([matchAllJobs, matchPartialJobs]);
var SkipFilterCondition = FilterCondition.readonly();
var WaitFilterCondition = FilterCondition.extend(
{
optional: z2.boolean().default(false).readonly(),
// - Intentionally avoided to use enum for now. Only GitHub knows whole eventNames and the adding plans
// - Intentionally omitted in skip-list, let me know if you have the use-case
eventName: z2.string().min(1).optional(),
// Do not raise validation errors for the reasonability of max value.
// Even in equal_intervals mode, we can't enforce the possibility of the whole running time
startupGracePeriod: Durationable.default(defaultGrace)
}
).readonly();
var waitOptions = {
optional: z2.boolean().default(false).readonly(),
// - Intentionally avoided to use enum for now. Only GitHub knows whole eventNames and the adding plans
// - Intentionally omitted in skip-list, let me know if you have the use-case
eventName: z2.string().min(1).optional(),
// Do not raise validation errors for the reasonability of max value.
// Even in equal_intervals mode, we can't enforce the possibility of the whole running time
startupGracePeriod: Durationable.default(defaultGrace)
};
var WaitFilterCondition = z2.union([
matchAllJobs.extend(waitOptions).strict(),
matchPartialJobs.extend(waitOptions).strict()
]).readonly();
var WaitList = z2.array(WaitFilterCondition).readonly();
var SkipList = z2.array(SkipFilterCondition).readonly();
var retryMethods = z2.enum(["exponential_backoff", "equal_intervals"]);
Expand Down Expand Up @@ -31117,7 +31128,10 @@ function parseInput() {
repo,
payload,
runId,
job,
// Not jobName, and GitHub does not provide the jobName
// https://github.com/orgs/community/discussions/8945
// https://github.com/orgs/community/discussions/16614
job: jobId,
sha,
eventName
} = import_github.context;
Expand Down Expand Up @@ -31160,7 +31174,7 @@ function parseInput() {
shouldSkipSameWorkflow,
isDryRun
});
const trigger = { ...repo, ref: commitSha, runId, jobName: job, eventName };
const trigger = { ...repo, ref: commitSha, runId, jobId, eventName };
const githubToken = (0, import_core.getInput)("github-token", { required: true, trimWhitespace: false });
(0, import_core.setSecret)(githubToken);
return { trigger, options, githubToken, tempDir };
Expand Down Expand Up @@ -32405,11 +32419,31 @@ function getSummaries(checks, trigger) {
(a2, b2) => join2(a2.workflowBasename, a2.jobName).localeCompare(join2(b2.workflowBasename, b2.jobName))
);
}
function matchPath({ workflowFile: workflowFile2, jobName, jobMatchMode }, summary) {
if (workflowFile2 !== summary.workflowBasename) {
return false;
}
if (!jobName) {
return true;
}
switch (jobMatchMode) {
case "exact": {
return jobName === summary.jobName;
}
case "prefix": {
return summary.jobName.startsWith(jobName);
}
default: {
const _exhaustiveCheck = jobMatchMode;
return false;
}
}
}
function seekWaitList(summaries, waitList, elapsed) {
const seeker = waitList.map((condition) => ({ ...condition, found: false }));
const filtered = summaries.filter(
(summary) => seeker.some((target) => {
const isMatchPath = target.workflowFile === summary.workflowBasename && (target.jobName ? target.jobName === summary.jobName : true);
const isMatchPath = matchPath(target, summary);
const isMatchEvent = target.eventName ? target.eventName === summary.eventName : true;
if (isMatchPath && isMatchEvent) {
target.found = true;
Expand Down Expand Up @@ -32452,7 +32486,20 @@ function judge(summaries) {
};
}
function generateReport(summaries, trigger, elapsed, { waitList, skipList, shouldSkipSameWorkflow }) {
const others = summaries.filter((summary) => !(summary.isSameWorkflow && trigger.jobName === summary.jobName));
const others = summaries.filter(
(summary) => !(summary.isSameWorkflow && // Ideally this logic should be...
//
// 1. `trigger(context).jobId === smmmary(checkRun).jobId`
// But GitHub does not provide the jobId for each checkRun: https://github.com/orgs/community/discussions/8945
//
// or second place as
// 2. `context.jobName === checkRun.jobName`
// But GitHub does not provide the jobName for each context: https://github.com/orgs/community/discussions/16614
//
// On the otherhand, the conxtext.jobId will be used for the default jobName
// Anyway, in matrix use, GitHub uses the default name for the prefix. It should be considered in list based solutions
trigger.jobId === summary.jobName)
);
const targets = others.filter((summary) => !(summary.isSameWorkflow && shouldSkipSameWorkflow));
if (waitList.length > 0) {
const { filtered, unmatches, unstarted } = seekWaitList(targets, waitList, elapsed);
Expand Down Expand Up @@ -32489,11 +32536,7 @@ function generateReport(summaries, trigger, elapsed, { waitList, skipList, shoul
return defaultReport;
}
if (skipList.length > 0) {
const filtered = targets.filter(
(summary) => !skipList.some(
(target) => target.workflowFile === summary.workflowBasename && (target.jobName ? target.jobName === summary.jobName : true)
)
);
const filtered = targets.filter((summary) => !skipList.some((target) => matchPath(target, summary)));
return { ...judge(filtered), summaries: filtered };
}
return { ...judge(targets), summaries: targets };
Expand Down Expand Up @@ -32551,7 +32594,7 @@ function colorize(severity, message) {
return message;
}
default: {
const _unexpectedSeverity = severity;
const _exhaustiveCheck = severity;
return message;
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ export function parseInput(): { trigger: Trigger; options: Options; githubToken:
repo,
payload,
runId,
job,
// Not jobName, and GitHub does not provide the jobName
// https://github.com/orgs/community/discussions/8945
// https://github.com/orgs/community/discussions/16614
job: jobId,
sha,
eventName,
} = context;
Expand Down Expand Up @@ -57,7 +60,7 @@ export function parseInput(): { trigger: Trigger; options: Options; githubToken:
isDryRun,
});

const trigger = { ...repo, ref: commitSha, runId, jobName: job, eventName } as const satisfies Trigger;
const trigger = { ...repo, ref: commitSha, runId, jobId, eventName } as const satisfies Trigger;

// `getIDToken` does not fit for this purpose. It is provided for OIDC Token
const githubToken = getInput('github-token', { required: true, trimWhitespace: false });
Expand Down
2 changes: 1 addition & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function colorize(severity: Severity, message: string): string {
return message;
}
default: {
const _unexpectedSeverity: never = severity;
const _exhaustiveCheck: never = severity;
return message;
}
}
Expand Down
Loading

0 comments on commit dcfa392

Please sign in to comment.