This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-4-test by this push:
new 1409c90ea1a [v3-4-test] Keep cancelled CI runs from showing a failed
CI passed check (#75720) (#75738)
1409c90ea1a is described below
commit 1409c90ea1a17f2177bb0666b79ef84a2de6eb64
Author: Shahar Epstein <[email protected]>
AuthorDate: Tue Oct 27 11:48:01 2026 +0200
[v3-4-test] Keep cancelled CI runs from showing a failed CI passed check
(#75720) (#75738)
* Keep cancelled CI runs from showing a failed CI passed check
The step meant to fail CI passed when a job hit its timeout tested
`!cancelled()`. Inside the `always()` gate job, a step's `cancelled()` is
false even when the run itself was cancelled, because the job starts after
the cancellation. So every duplicate run that concurrency cancelled, and
every run cancelled by hand, showed a failed "CI passed (run cancelled)"
check next to the passing run of the same commit (runs 40442992585 and
40453574496).
Only a job-level `if` sees that the run was cancelled, so the timeout check
moves into a job of its own. A job that hits its timeout while the run goes
on still fails the gate, which the scheduled canary's Slack report relies
on, and the failed check is now the required CI passed instead of
"CI passed (run cancelled)".
* Check the timed-out-jobs needs only when that job exists
Removing check-timed-out-jobs alone already fails the hook, because the
gate still needs it. Requiring the job as well forced every existing test
case to carry it.
(cherry picked from commit 8eb5357fb136263967c06493323ef026018d7bab)
---
.github/workflows/ci-amd.yml | 68 +++++++++++++++++++----
.github/workflows/ci-arm.yml | 67 ++++++++++++++++++----
dev/README_AIRFLOW3_DEV.md | 5 +-
scripts/ci/prek/check_ci_gate_needs.py | 15 +++++
scripts/ci/prek/check_ci_workflows_in_sync.py | 17 ++++--
scripts/tests/ci/prek/test_check_ci_gate_needs.py | 23 ++++++++
6 files changed, 165 insertions(+), 30 deletions(-)
diff --git a/.github/workflows/ci-amd.yml b/.github/workflows/ci-amd.yml
index 4e6de3a5a8d..b79663bbb9a 100644
--- a/.github/workflows/ci-amd.yml
+++ b/.github/workflows/ci-amd.yml
@@ -1501,6 +1501,58 @@ jobs:
use-uv: ${{ needs.build-info.outputs.use-uv }}
debug-resources: ${{ needs.build-info.outputs.debug-resources }}
+ check-timed-out-jobs:
+ # A job-level `if` is the only place `cancelled()` tells a cancelled run
from a job cancelled by
+ # its timeout: the steps of `ci-passed` start after the cancellation. Its
`needs` must match
+ # those of `ci-passed`, so a timeout in any job fails the gate.
+ name: "Check for timed-out CI jobs"
+ if: always() && !cancelled() && contains(needs.*.result, 'cancelled')
+ needs:
+ - additional-ci-image-checks
+ - additional-prod-image-tests
+ - basic-tests
+ - build-ci-images
+ - build-info
+ - build-java-sdk-docs
+ - build-prod-images
+ - build-ts-sdk-docs
+ - check-run-needed
+ - ci-image-checks
+ - finalize-tests
+ - gate
+ - generate-constraints
+ - migration-round-trip
+ - mypy-providers
+ - print-platform
+ - providers
+ - tests-agent-frameworks
+ - tests-airflow-ctl
+ - tests-go-sdk
+ - tests-helm
+ - tests-integration-system
+ - tests-java-sdk
+ - tests-kubernetes
+ - tests-kustomize-overlays
+ - tests-mysql-core
+ - tests-mysql-providers
+ - tests-non-db-core
+ - tests-non-db-providers
+ - tests-postgres-core
+ - tests-postgres-providers
+ - tests-special
+ - tests-sqlite-core
+ - tests-sqlite-providers
+ - tests-task-sdk
+ - tests-ts-sdk
+ - tests-with-lowest-direct-resolution-core
+ - tests-with-lowest-direct-resolution-providers
+ runs-on: ["ubuntu-slim"]
+ steps:
+ - name: "Fail because a CI job was cancelled without the run being
cancelled"
+ run: |
+ echo "A CI job was cancelled, likely by its timeout, while the run
was not cancelled."
+ exit 1
+
ci-passed:
# A skipped build-info means project CI did not run at all: the fork CI
gate (AIP-120) left the
# pull request to CI in the contributor's fork, and every job here is
skipped. Reporting
@@ -1511,8 +1563,8 @@ jobs:
# request's base and head are force-pushed together -- reports under
another name too, so it
# never puts a failed "CI passed" next to the passing one of its twin. "CI
passed" is then
# missing for that run, which keeps the merge blocked until another run of
the commit reports it.
- # A job cancelled by its timeout gets that name too, since `name` cannot
test whether the run
- # itself was cancelled, but the job still fails.
+ # A job cancelled by its timeout, while the run is not, makes
`check-timed-out-jobs` fail, and
+ # so this job fails.
name: >-
${{ (!contains(needs.*.result, 'failure') && contains(needs.*.result,
'cancelled') &&
'CI passed (run cancelled)') ||
@@ -1531,6 +1583,7 @@ jobs:
- build-prod-images
- build-ts-sdk-docs
- check-run-needed
+ - check-timed-out-jobs
- ci-image-checks
- finalize-tests
- gate
@@ -1567,15 +1620,8 @@ jobs:
run: |
echo "At least one CI job failed."
exit 1
- # A job that hits its timeout is cancelled while the run is not, and
must fail this check, as
- # the scheduled canary's Slack report lists only failed jobs.
- - name: "Fail if a CI job was cancelled without the run being cancelled"
- if: contains(needs.*.result, 'cancelled') && !cancelled()
- run: |
- echo "A CI job was cancelled, likely by its timeout, while the run
was not cancelled."
- exit 1
- name: "Report that CI jobs were cancelled"
- if: contains(needs.*.result, 'cancelled') && cancelled()
+ if: contains(needs.*.result, 'cancelled')
run: |
echo "At least one CI job was cancelled, so this run does not report
CI passed."
echo 'CI jobs were cancelled: this run does not report `CI passed`.'
>> "${GITHUB_STEP_SUMMARY}"
@@ -1588,7 +1634,7 @@ jobs:
needs:
- build-info
- check-run-needed
- # Waits for the gate job, which is the only failed job when a CI job
timed out.
+ # Waits for the gate job, which also fails when a CI job timed out.
- ci-passed
- finalize-tests
- tests-agent-frameworks
diff --git a/.github/workflows/ci-arm.yml b/.github/workflows/ci-arm.yml
index 47c59be2367..9ce7669b669 100644
--- a/.github/workflows/ci-arm.yml
+++ b/.github/workflows/ci-arm.yml
@@ -1384,6 +1384,57 @@ jobs:
use-uv: ${{ needs.build-info.outputs.use-uv }}
debug-resources: ${{ needs.build-info.outputs.debug-resources }}
+ check-timed-out-jobs:
+ # A job-level `if` is the only place `cancelled()` tells a cancelled run
from a job cancelled by
+ # its timeout: the steps of `ci-passed` start after the cancellation. Its
`needs` must match
+ # those of `ci-passed`, so a timeout in any job fails the gate.
+ name: "Check for timed-out CI jobs"
+ if: always() && !cancelled() && contains(needs.*.result, 'cancelled')
+ needs:
+ - additional-ci-image-checks
+ - additional-prod-image-tests
+ - basic-tests
+ - build-ci-images
+ - build-info
+ - build-java-sdk-docs
+ - build-prod-images
+ - build-ts-sdk-docs
+ - check-run-needed
+ - ci-image-checks
+ - finalize-tests
+ - generate-constraints
+ - migration-round-trip
+ - mypy-providers
+ - print-platform
+ - providers
+ - tests-agent-frameworks
+ - tests-airflow-ctl
+ - tests-go-sdk
+ - tests-helm
+ - tests-integration-system
+ - tests-java-sdk
+ - tests-kubernetes
+ - tests-kustomize-overlays
+ - tests-mysql-core
+ - tests-mysql-providers
+ - tests-non-db-core
+ - tests-non-db-providers
+ - tests-postgres-core
+ - tests-postgres-providers
+ - tests-special
+ - tests-sqlite-core
+ - tests-sqlite-providers
+ - tests-task-sdk
+ - tests-ts-sdk
+ - tests-with-lowest-direct-resolution-core
+ - tests-with-lowest-direct-resolution-providers
+ runs-on: ["ubuntu-slim"]
+ steps:
+ - name: "Fail because a CI job was cancelled without the run being
cancelled"
+ run: |
+ echo "A CI job was cancelled, likely by its timeout, while the run
was not cancelled."
+ exit 1
+
ci-passed:
# A skipped build-info means project CI did not run at all: the fork CI
gate (AIP-120) left the
# pull request to CI in the contributor's fork, and every job here is
skipped. Reporting
@@ -1394,8 +1445,8 @@ jobs:
# request's base and head are force-pushed together -- reports under
another name too, so it
# never puts a failed "CI passed" next to the passing one of its twin. "CI
passed" is then
# missing for that run, which keeps the merge blocked until another run of
the commit reports it.
- # A job cancelled by its timeout gets that name too, since `name` cannot
test whether the run
- # itself was cancelled, but the job still fails.
+ # A job cancelled by its timeout, while the run is not, makes
`check-timed-out-jobs` fail, and
+ # so this job fails.
name: >-
${{ (!contains(needs.*.result, 'failure') && contains(needs.*.result,
'cancelled') &&
'CI passed (run cancelled)') ||
@@ -1414,6 +1465,7 @@ jobs:
- build-prod-images
- build-ts-sdk-docs
- check-run-needed
+ - check-timed-out-jobs
- ci-image-checks
- finalize-tests
- generate-constraints
@@ -1449,15 +1501,8 @@ jobs:
run: |
echo "At least one CI job failed."
exit 1
- # A job that hits its timeout is cancelled while the run is not, and
must fail this check, as
- # the scheduled canary's Slack report lists only failed jobs.
- - name: "Fail if a CI job was cancelled without the run being cancelled"
- if: contains(needs.*.result, 'cancelled') && !cancelled()
- run: |
- echo "A CI job was cancelled, likely by its timeout, while the run
was not cancelled."
- exit 1
- name: "Report that CI jobs were cancelled"
- if: contains(needs.*.result, 'cancelled') && cancelled()
+ if: contains(needs.*.result, 'cancelled')
run: |
echo "At least one CI job was cancelled, so this run does not report
CI passed."
echo 'CI jobs were cancelled: this run does not report `CI passed`.'
>> "${GITHUB_STEP_SUMMARY}"
@@ -1470,7 +1515,7 @@ jobs:
needs:
- build-info
- check-run-needed
- # Waits for the gate job, which is the only failed job when a CI job
timed out.
+ # Waits for the gate job, which also fails when a CI job timed out.
- ci-passed
- finalize-tests
- tests-agent-frameworks
diff --git a/dev/README_AIRFLOW3_DEV.md b/dev/README_AIRFLOW3_DEV.md
index c13904dbbec..faec7d728b7 100644
--- a/dev/README_AIRFLOW3_DEV.md
+++ b/dev/README_AIRFLOW3_DEV.md
@@ -201,8 +201,9 @@ run only part of the tests still get a result. A run with
cancelled jobs and no
**CI passed (run cancelled)** instead, so a duplicate run of the same commit
that concurrency cancelled
does not show a failed **CI passed** next to its twin's result. Such a run
does not report **CI passed**
at all, so the PR stays blocked until another run of the commit, or a re-run,
reports it. A job that
-hits its timeout is cancelled while the run is not, so that run's check fails,
under the same name. The
-`check-ci-gate-needs` prek hook makes sure every new CI job is added to it.
`Tests (ARM)`, which runs on pushes and on a schedule rather
+hits its timeout is cancelled while the run is not, so the `Check for
timed-out CI jobs` job fails, and
+that run reports a failed **CI passed**. The `check-ci-gate-needs` prek hook
makes sure every new CI job
+is added to it. `Tests (ARM)`, which runs on pushes and on a schedule rather
than on PRs, has the same job, kept identical by the
`check-ci-workflows-in-sync` hook. For a PR that the
fork CI gate (AIP-120) leaves to CI in the contributor's fork, project CI does
not run, so the job reports
as **CI passed (project CI not run)** instead, and the fork CI reconciler
posts **CI passed** once the
diff --git a/scripts/ci/prek/check_ci_gate_needs.py
b/scripts/ci/prek/check_ci_gate_needs.py
index 63e4207cc1e..810e52295f6 100755
--- a/scripts/ci/prek/check_ci_gate_needs.py
+++ b/scripts/ci/prek/check_ci_gate_needs.py
@@ -26,6 +26,9 @@
``CI passed`` is the check main's ruleset requires, and auto-merge waits for
it. It only fails
when a job it needs failed, so a job missing from its ``needs`` could fail and
the PR would still
auto-merge. Reporting-only jobs that cannot make a PR wrong are exempt.
+
+``Check for timed-out CI jobs`` fails the gate when a job hit its timeout. It
must need the same jobs
+as the gate, or a timeout in a job it misses would pass.
"""
from __future__ import annotations
@@ -38,6 +41,7 @@ import yaml
AIRFLOW_ROOT = Path(__file__).parents[3]
CI_WORKFLOW = AIRFLOW_ROOT / ".github/workflows/ci-amd.yml"
GATE_JOB = "ci-passed"
+TIMEOUT_JOB = "check-timed-out-jobs"
EXEMPT_JOBS = {"notify-slack", "summarize-warnings"}
@@ -47,6 +51,8 @@ def find_problems(jobs: dict) -> list[str]:
needs = jobs[GATE_JOB].get("needs") or []
needs = {needs} if isinstance(needs, str) else set(needs)
expected = set(jobs) - EXEMPT_JOBS - {GATE_JOB}
+ timeout_needs = jobs.get(TIMEOUT_JOB, {}).get("needs") or []
+ timeout_needs = {timeout_needs} if isinstance(timeout_needs, str) else
set(timeout_needs)
problems = [
f"job {job!r} is not in {GATE_JOB}.needs, so its failure would not
stop auto-merge"
for job in sorted(expected - needs)
@@ -55,6 +61,15 @@ def find_problems(jobs: dict) -> list[str]:
problems += [
f"{GATE_JOB}.needs lists exempt reporting job {job!r}" for job in
sorted(needs & EXEMPT_JOBS)
]
+ if TIMEOUT_JOB in jobs:
+ problems += [
+ f"{TIMEOUT_JOB}.needs is missing {job!r}, which {GATE_JOB} needs"
+ for job in sorted(needs - {TIMEOUT_JOB} - timeout_needs)
+ ]
+ problems += [
+ f"{TIMEOUT_JOB}.needs lists {job!r}, which {GATE_JOB} does not
need"
+ for job in sorted(timeout_needs - needs)
+ ]
return problems
diff --git a/scripts/ci/prek/check_ci_workflows_in_sync.py
b/scripts/ci/prek/check_ci_workflows_in_sync.py
index edee96e3a9b..047c3f0dd7c 100755
--- a/scripts/ci/prek/check_ci_workflows_in_sync.py
+++ b/scripts/ci/prek/check_ci_workflows_in_sync.py
@@ -48,9 +48,10 @@ Documented divergences:
``slack_notification_state.py`` keeps independent state for each
platform on the same branch
9. ``gate`` job, the ``needs: [gate]`` / ``if:`` lines of ``build-info``
- and ``gate`` in the needs of ``ci-passed`` — AMD only. The fork CI gate
(AIP-120) decides whether project CI runs
- for pull requests; ARM has no pull_request or push trigger, and the
- ``check-run-needed`` step both share skips a fork's scheduled run.
+ and ``gate`` in the needs of ``ci-passed`` and ``check-timed-out-jobs``
+ — AMD only. The fork CI gate (AIP-120) decides whether project CI
+ runs for pull requests; ARM has no pull_request or push trigger, and
+ the ``check-run-needed`` step both share skips a fork's scheduled run.
Anything else differing between the two files is a drift bug. To
intentionally introduce a new divergence, update the rules in this script
@@ -258,7 +259,8 @@ AMD_ONLY_GATE_BLOCK = """ gate:
needs.gate.outputs.project-ci == 'true' &&
needs.check-run-needed.outputs.run-needed == 'true'
"""
-# The gate is also one of the jobs the "CI passed" gate job of the merge
ruleset needs.
+# The gate is also one of the jobs the "CI passed" gate job of the merge
ruleset needs, and so is
+# in the needs of `check-timed-out-jobs`, which must match them.
AMD_ONLY_CI_PASSED_GATE_NEED = """ - finalize-tests
- gate
- generate-constraints
@@ -334,8 +336,11 @@ def normalize(content: str, *, side: str) -> str:
raise SystemExit("ci-amd.yml contains the ARM-only trigger block —
should not.")
if AMD_ONLY_GATE_BLOCK not in content:
raise SystemExit(f"ci-amd.yml is missing its expected fork CI gate
block:\n{AMD_ONLY_GATE_BLOCK}")
- if content.count(AMD_ONLY_CI_PASSED_GATE_NEED) != 1:
- raise SystemExit("ci-amd.yml must list `gate` once in the needs of
the CI passed job.")
+ if content.count(AMD_ONLY_CI_PASSED_GATE_NEED) != 2:
+ raise SystemExit(
+ "ci-amd.yml must list `gate` once in the needs of the CI
passed job"
+ " and once in those of check-timed-out-jobs."
+ )
content = (
content.replace(AMD_ONLY_BLOCK, "")
.replace(AMD_ONLY_GATE_BLOCK, AMD_ONLY_GATE_REPLACEMENT)
diff --git a/scripts/tests/ci/prek/test_check_ci_gate_needs.py
b/scripts/tests/ci/prek/test_check_ci_gate_needs.py
index 77248def0ab..d1e8f18acf9 100644
--- a/scripts/tests/ci/prek/test_check_ci_gate_needs.py
+++ b/scripts/tests/ci/prek/test_check_ci_gate_needs.py
@@ -54,6 +54,29 @@ def test_real_workflow_gate_needs_every_job():
["ci-passed.needs lists exempt reporting job
'summarize-warnings'"],
id="exempt-job-listed",
),
+ pytest.param(
+ {
+ "build": {},
+ "test": {},
+ "check-timed-out-jobs": {"needs": ["build"]},
+ "ci-passed": {"needs": ["build", "test",
"check-timed-out-jobs"]},
+ },
+ ["check-timed-out-jobs.needs is missing 'test', which ci-passed
needs"],
+ id="timeout-check-missing-job",
+ ),
+ pytest.param(
+ {
+ "build": {},
+ "test": {},
+ "check-timed-out-jobs": {"needs": ["build", "test"]},
+ "ci-passed": {"needs": ["build", "check-timed-out-jobs"]},
+ },
+ [
+ "job 'test' is not in ci-passed.needs, so its failure would
not stop auto-merge",
+ "check-timed-out-jobs.needs lists 'test', which ci-passed does
not need",
+ ],
+ id="timeout-check-extra-job",
+ ),
],
)
def test_find_problems(jobs, expected):
