Skip to content

Commit 6a4a72f

Browse files
authored
ci(experimental): skip or stop superseded runs instead of cancelling them (#1938) (#1942)
* ci(experimental): skip or stop superseded runs instead of cancelling them (#1938) - Drop experimental.yml's concurrency group: a cancelled check marks its commit red, and a group without cancel-in-progress still cancels its pending run. - Add util/superseded.py: a commit is current while it heads main, or its open pull request; any other event, or an API error, is current. - Add a `current` gate job that every other job needs and skips on, so a run superseded before it starts ends with skipped checks and a notice. - Run the capped pytest command under the script's watcher, which checks every 60 s and, once superseded, stops the session (TERM, then KILL) and exits 0 with a notice and a summary line; otherwise it keeps the tests' status. - Describe it in workflows.rst; pin it in tests/project and update the pins the gate and watcher move. Closes #1938 * ci(experimental): bound the superseded stop and reach the test scope (#1938) - Run the watcher outside the scope and, where the tests run in one, stop pcapkit-tests.scope by name with `sudo -n systemctl kill`: the scope's processes are out of reach of a signal to the session's root-owned sudo. The session's process group stays the path for an unscoped run. - Bound every wait. A command that outlives SIGKILL gets a `Superseded run not stopped` warning and exit 70, not 0, since its tests may run on. - Open each downstream `if:` with `!cancelled()`, so a failed `current` job runs the rest; the legs still need `changes` and `python-versions`. - Test the scoped stop, the bounded stop and the gate's failure semantics through an injectable command runner and signaller, and in the step. Refs #1938 * ci(experimental): fetch superseded state with requests (#1938) - Back util/superseded.py's transport with a requests.Session, keeping the injectable transport, ETag/If-None-Match, the bounded timeout, and any error, a missing requests included, counting as current. - Gate: read `latest`, set it up and install requests before the check, every step continue-on-error. - Legs: set up `latest` apart from the leg (`update-environment: false`), install requests into it alone, and run the watcher on it; where either step fails, the tests run unwatched. Each step is capped at 1 minute, leaving 3 of the job's 140 uncapped. - util/update_python_versions.py reads a matrix job's fallback label for `latest`, as it does for the tiers. - Update the docs, the caps pins and the new tests. Refs #1938 * test(experimental): drop review-round labels from the #1938 pins (#1938) - Reword the #1938 tests' docstrings and comments without the review-round prefixes, each still stating what its test checks. - State the experimental leg's uncapped margin as what it covers and the measured maximum, not as what it was. Refs #1938
1 parent 9985060 commit 6a4a72f

9 files changed

Lines changed: 1502 additions & 91 deletions

‎.github/workflows/experimental.yml‎

Lines changed: 156 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,88 @@ on:
2525
permissions:
2626
contents: read
2727

28-
# #1903, the owner's option 1: a run is superseded by the next one on its ref,
29-
# whether a pull request's or a push to main's. Nothing waits on these legs, so
30-
# main's newest commit is the one worth testing, and a merge wave no longer
31-
# queues a two-hour GraalPy leg per push. unit-tests.yml still gives each push
32-
# a run of its own, since its `unittest-ordering` leg runs only on main. Any
33-
# other event -- a push to another branch, should `on:` ever allow one, or a
34-
# trigger added later -- keeps a group of its own, by run id, and is never
35-
# cancelled.
36-
concurrency:
37-
group: experimental-tests-${{ (github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main')) && github.ref || github.run_id }}
38-
cancel-in-progress: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main') }}
28+
# #1903 has a newer commit supersede a run, a pull request's or a push to
29+
# main's: nothing waits on these legs, so the newest commit is the one worth
30+
# testing. #1938, the owner's choice: a superseded run is skipped or stopped,
31+
# never cancelled, since GitHub counts a cancelled check against its commit and
32+
# marks it red. So there is no `concurrency:` block at all: without
33+
# `cancel-in-progress` a group still holds one pending run and cancels it once
34+
# a newer one arrives (docs.github.com, "Control the concurrency of workflows
35+
# and jobs"), and `queue: max` would hold a new commit's run behind the
36+
# superseded one until that one stops. Instead `current` skips every other job
37+
# of a run whose commit is superseded before it starts, and `Run unit tests`
38+
# stops one superseded while its tests run; util/superseded.py holds the rule
39+
# for both. A run whose commit stays current is unaffected.
3940

4041
jobs:
42+
# #1938: the gate. util/superseded.py's `check` asks the API whether this
43+
# run's commit is still the head of main or of its open pull request, and
44+
# writes `current`; any other event, or an API it cannot read, is current.
45+
# Every other job needs this one and skips where `current` is `false`, so a
46+
# superseded run's checks read `skipped`, which marks no commit red. Every
47+
# step is `continue-on-error`: without `latest`, setup-python or requests the
48+
# check fails or answers current, so a step's failure leaves `current` empty
49+
# or `true`, which runs the rest; each job's `if:` opens with `!cancelled()`,
50+
# so a failure of the job itself, which replaces the implicit `success()`,
51+
# runs the rest too.
52+
current:
53+
name: Check the commit is still current
54+
runs-on: ubuntu-latest
55+
timeout-minutes: 5
56+
permissions:
57+
contents: read
58+
pull-requests: read
59+
outputs:
60+
current: ${{ steps.check.outputs.current }}
61+
62+
steps:
63+
- uses: actions/checkout@v7
64+
continue-on-error: true
65+
with:
66+
sparse-checkout: |
67+
.github/python-versions.toml
68+
util/python_versions.py
69+
util/superseded.py
70+
sparse-checkout-cone-mode: false
71+
persist-credentials: false
72+
73+
# #1660: `latest` from .github/python-versions.toml, read with the
74+
# runner's own python3, for the interpreter util/superseded.py fetches
75+
# with requests on.
76+
- name: Read the Python versions
77+
id: versions
78+
continue-on-error: true
79+
run: python3 util/python_versions.py --github-output
80+
81+
- uses: actions/setup-python@v7
82+
id: python
83+
continue-on-error: true
84+
with:
85+
python-version: ${{ steps.versions.outputs.latest }}
86+
87+
- name: Install requests
88+
id: requests
89+
continue-on-error: true
90+
run: python -m pip install requests
91+
92+
- name: Ask whether this run's commit is still current
93+
id: check
94+
continue-on-error: true
95+
env:
96+
GH_TOKEN: ${{ github.token }}
97+
run: python util/superseded.py check
98+
4199
# unit-tests.yml's job of the same name, reading the one output this file needs.
42100
python-versions:
43101
name: Read the Python versions
102+
needs: current
103+
if: ${{ !cancelled() && needs.current.outputs.current != 'false' }}
44104
runs-on: ubuntu-latest
45105
timeout-minutes: 5
46106
outputs:
47107
experimental: ${{ steps.read.outputs.experimental }}
108+
# #1938: the watcher's own interpreter, apart from the leg's.
109+
latest: ${{ steps.read.outputs.latest }}
48110

49111
steps:
50112
- uses: actions/checkout@v7
@@ -65,6 +127,8 @@ jobs:
65127
# pull_request run.
66128
changes:
67129
name: Classify changed files
130+
needs: current
131+
if: ${{ !cancelled() && needs.current.outputs.current != 'false' }}
68132
runs-on: ubuntu-latest
69133
timeout-minutes: 5
70134
outputs:
@@ -127,11 +191,12 @@ jobs:
127191
#
128192
# A job past its `timeout-minutes` is cancelled (same page,
129193
# `jobs.<job_id>.timeout-minutes`), and the page does not say whether the
130-
# job-level guard covers that. So setup-python, the install and the tests
131-
# carry caps of their own, 5, 10 and 120 minutes, under the job's 140: a step
132-
# past its cap fails rather than cancels, and `continue-on-error` then applies
133-
# as to any failure (actions/runner src/Runner.Worker/StepsRunner.cs:322-353,
134-
# at 5f0b3ddcc). That leaves 5 minutes for the rest, which took at most 0.32,
194+
# job-level guard covers that. So setup-python, the install, the watcher's
195+
# setup-python and requests (#1938) and the tests carry caps of their own,
196+
# 5, 10, 1, 1 and 120 minutes, under the job's 140: a step past its cap
197+
# fails rather than cancels, and `continue-on-error` then applies as to any
198+
# failure (actions/runner src/Runner.Worker/StepsRunner.cs:322-353, at
199+
# 5f0b3ddcc). That leaves 3 minutes for the rest, which took at most 0.32,
135200
# setup-python included, on every uncovered leg of runs
136201
# 38569321543-38569522442.
137202
#
@@ -151,11 +216,20 @@ jobs:
151216
# expected, blocks nothing, and leaves the caps as they are.
152217
experimental:
153218
name: Python ${{ matrix.python-version }} (experimental)
154-
if: ${{ needs.changes.outputs.code != 'false' }}
155-
needs: [changes, python-versions]
219+
# #1938: `!cancelled()` so a failed `current` skips nothing, and then the
220+
# two results stand in for the implicit `success()` it replaces: a failed
221+
# `changes` or `python-versions` still skips the legs.
222+
if: >-
223+
${{ !cancelled() && needs.changes.result == 'success' && needs.python-versions.result == 'success'
224+
&& needs.current.outputs.current != 'false' && needs.changes.outputs.code != 'false' }}
225+
needs: [current, changes, python-versions]
156226
runs-on: ${{ matrix.os || 'ubuntu-latest' }}
157227
timeout-minutes: 140
158228
continue-on-error: true
229+
# #1938: `Run unit tests` reads the pull request, as `current` does.
230+
permissions:
231+
contents: read
232+
pull-requests: read
159233
# #1672: every annotation a failing step writes is a warning.
160234
env:
161235
ANNOTATION_LEVEL: warning
@@ -202,6 +276,35 @@ jobs:
202276
nproc
203277
python -c "import os; print('cpu_count', os.cpu_count())"
204278
279+
# #1938: the interpreter `Run unit tests` runs util/superseded.py on, a
280+
# CPython of its own: `latest`, never the leg's GraalPy or free-threaded
281+
# build, which would also have to take requests. `update-environment:
282+
# false` leaves PATH, `pythonLocation` and `LD_LIBRARY_PATH` as the leg's
283+
# setup-python set them, so the tests see the leg's interpreter alone;
284+
# this one is run by its `python-path`, and finds its own libpython by
285+
# the rpath actions/python-versions builds with
286+
# (builders/ubuntu-python-builder.psm1), since its soname is not the
287+
# leg's. No `cache:` and `--no-cache-dir`, so the leg's pip cache is
288+
# untouched. Each step has a cap of 1 minute (see the job's comment),
289+
# and a failure leaves the tests to run unwatched.
290+
- uses: actions/setup-python@v7
291+
id: watcher-python
292+
if: ${{ steps.install.outcome == 'success' }}
293+
continue-on-error: true
294+
timeout-minutes: 1
295+
with:
296+
python-version: ${{ needs.python-versions.outputs.latest }}
297+
update-environment: false
298+
299+
- name: Install the watcher's requests
300+
id: watcher
301+
if: ${{ steps.install.outcome == 'success' && steps.watcher-python.outcome == 'success' }}
302+
continue-on-error: true
303+
timeout-minutes: 1
304+
env:
305+
WATCHER_PYTHON: ${{ steps.watcher-python.outputs.python-path }}
306+
run: '"$WATCHER_PYTHON" -m pip install --no-cache-dir requests'
307+
205308
# #1740: GraalPy's leg loses its runner 18-30 minutes into its tests ("The
206309
# runner has received a shutdown signal"), under a 120-minute cap. So every
207310
# 30 s this samples `free -m`, `df -h` of `/` and the workspace's volume,
@@ -313,6 +416,26 @@ jobs:
313416
# crashed, by xdist's `[gwN] node down:` lines (under `-q` it prints no
314417
# `replacing crashed worker`, xdist/dsession.py:78-80 at v3.8.0), for
315418
# `Containment summary`.
419+
#
420+
# #1938: util/superseded.py's `watch` runs the whole command -- the scope's
421+
# sudo where there is one, then prlimit, timeout and pytest -- in a
422+
# session of its own, outside the scope, and asks `current`'s question
423+
# every 60 s. Once the commit is superseded it sends SIGTERM, SIGKILL 30 s
424+
# on, to that session and, with `--scope`, to every process of
425+
# pcapkit-tests.scope through `systemctl kill`: the scope's processes run
426+
# as the runner's user under a root-owned sudo, which a signal to the
427+
# session cannot pass. Ubuntu's own /etc/sudoers sets `use_pty` (sudo
428+
# 1.9.9-1ubuntu2.7 and 1.9.15p5-3ubuntu5.24.04.4), and sudoers(5) says it
429+
# has no effect where sudo has no terminal, as in a step; the scope is
430+
# reached by name either way. Once the command exits it writes a
431+
# `Superseded` notice and a job-summary line and exits 0, so the step
432+
# succeeds and frees the runner; where it outlives SIGKILL by 15 s, a
433+
# warning and exit 70, since the tests may run on until `timeout` ends
434+
# them. Otherwise it exits with the command's own status, so a failure on
435+
# a current commit reads as it would unwatched, and an API it cannot read
436+
# counts as current. It runs on `watcher-python`, with requests; where
437+
# that interpreter or its requests did not install, the tests run
438+
# unwatched, as they would without #1938.
316439
- name: Run unit tests
317440
id: tests
318441
if: ${{ steps.install.outcome == 'success' }}
@@ -321,6 +444,9 @@ jobs:
321444
env:
322445
GRAAL_PYTHON_VM_ARGS: ${{ startsWith(matrix.python-version, 'graalpy') && '--vm.Xmx2g --vm.XX:+ExitOnOutOfMemoryError' || '' }}
323446
WORKERS: ${{ startsWith(matrix.python-version, 'graalpy') && '3' || 'auto' }}
447+
GH_TOKEN: ${{ github.token }}
448+
WATCHER: ${{ steps.watcher.outcome }}
449+
WATCHER_PYTHON: ${{ steps.watcher-python.outputs.python-path }}
324450
run: |
325451
tail -n +1 -F "$RUNNER_TEMP/resource-monitor.log" 2>/dev/null &
326452
monitor=$!
@@ -350,6 +476,18 @@ jobs:
350476
# A log line, not an annotation: on a lost runner this log is all that is left.
351477
echo "No 12G scope: the probe failed (${probe}), so the tests run uncapped."
352478
fi
479+
if [ "$WATCHER" = success ] && [ -x "$WATCHER_PYTHON" ]; then
480+
watched=
481+
if [ "$scope" = 12G ]; then
482+
# `scoped` is a function, which the watcher runs through a shell.
483+
export -f scoped
484+
set -- bash -c '"$@"' bash "$@"
485+
watched=pcapkit-tests.scope
486+
fi
487+
set -- "$WATCHER_PYTHON" util/superseded.py watch --interval 60 --grace 30 --scope "$watched" -- "$@"
488+
else
489+
echo "No watcher: its interpreter or requests did not install (${WATCHER:-skipped}), so the tests run unwatched."
490+
fi
353491
oom_kills() { awk '$1 == "oom_kill" { print $2 }' /proc/vmstat; }
354492
before=$(oom_kills)
355493
started=$SECONDS

‎docs/source/contributing/workflows.rst‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -758,13 +758,50 @@ Tests run completes without it, and with it ``Required checks passed``,
758758
``unittest-ordering`` and the workflows that wait on Unit Tests completing
759759
(``coverage-comment.yml``, ``deploy-pages.yml``, ``cron-vendor.yml`` and
760760
``cron-conda.yml``). It runs on Unit Tests' ``push`` and ``pull_request`` events,
761-
skips a docs-only pull request by the same ``changes`` script, and cancels a
762-
superseded run, a pull request's or, unlike Unit Tests, a push to ``main``'s
761+
skips a docs-only pull request by the same ``changes`` script, and lets a newer
762+
commit supersede a run, a pull request's or, unlike Unit Tests, a push to ``main``'s
763763
(`#1903 <https://github.com/JarryShaw/PyPCAPKit/issues/1903>`__): since nothing
764764
waits on the legs, ``main``'s newest commit is the one worth testing, whereas Unit
765-
Tests keeps a run per push for ``unittest-ordering``, which runs only there. Any
766-
other event keeps a concurrency group of its own, by run id. The legs never
767-
block, by the KTLO mechanics above: every step that can fail has
765+
Tests keeps a run per push for ``unittest-ordering``, which runs only there.
766+
767+
A superseded run is skipped or stopped, never cancelled (`#1938
768+
<https://github.com/JarryShaw/PyPCAPKit/issues/1938>`__): GitHub counts a
769+
``cancelled`` check against its commit, so a cancelled run marks the commit red.
770+
The workflow has no ``concurrency`` group either, since a group cancels its pending
771+
run once a newer one queues. Its first job, ``current`` ("Check the commit is still
772+
current"), sets up ``latest`` and installs ``requests``, which the script fetches with,
773+
then runs :file:`util/superseded.py`'s ``check``. A push's commit is current
774+
while it is the head of ``main``; a pull request's while the pull request is open and
775+
its head is still the event's ``pull_request.head.sha``; any other event's always.
776+
Every other job needs ``current`` and skips where its output is ``false``, so a run
777+
superseded before it starts ends with ``skipped`` checks and a ``Superseded`` notice.
778+
Each of those ``if:`` conditions opens with ``!cancelled()``, which drops the implicit
779+
``success()``, so a failed ``current`` step, which leaves the output empty, or a failed
780+
``current`` job runs the rest; the legs still need ``changes`` and ``python-versions``
781+
to succeed. ``Run unit tests`` runs its whole command -- the scope's ``sudo`` where
782+
there is one, then ``prlimit``, ``timeout`` and pytest -- under the script's ``watch``,
783+
outside the scope, which asks again every 60 seconds. The watcher runs on a CPython of
784+
its own, ``latest`` from a second setup-python step, ``watcher-python``, with
785+
``update-environment: false``, so the leg's ``PATH``, ``LD_LIBRARY_PATH`` and pip
786+
cache are as its own setup-python left them, and ``requests`` goes into that
787+
interpreter, never the leg's GraalPy or free-threaded build. The two steps are capped
788+
at 1 minute each, and where either fails the tests run unwatched. Once the commit is superseded,
789+
or the pull request merged or closed, it sends SIGTERM, then SIGKILL 30 seconds on:
790+
to ``pcapkit-tests.scope`` by name (``sudo -n systemctl kill``) where the tests run
791+
in it, since they run as the runner's user under a root-owned ``sudo`` and in
792+
``timeout``'s own process group, and to the command's session otherwise. Ubuntu's
793+
``/etc/sudoers`` sets ``use_pty``, which sudoers(5) says has no effect where ``sudo``
794+
has no terminal, as in a step; the scope is reached by name either way. Once the
795+
command exits, the watcher writes a ``Superseded`` notice and a job-summary line and
796+
exits 0, so the leg ends green and frees its runner. Every wait is bounded: where the
797+
command outlives SIGKILL by 15 seconds, it writes a ``Superseded run not stopped``
798+
warning and exits 70, not 0, since the tests may run on, holding the step's output,
799+
until ``timeout`` or the step's cap ends them, and the failed step is then a warning
800+
like any other on these legs. Otherwise the step ends with the tests' own status. An
801+
API the script cannot read counts as current, so it never stops the tests, and it
802+
asks with ``If-None-Match``, as a ``304`` answer costs no rate limit.
803+
804+
The legs never block, by the KTLO mechanics above: every step that can fail has
768805
``continue-on-error: true``, their annotations are warnings, and ``Experimental
769806
summary``, the job's last step, reports each failed step. A failure no step
770807
contains, such as a lost runner, fails the job, and the job's own
@@ -773,8 +810,10 @@ run from failing when a job fails" (`Workflow syntax
773810
<https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idcontinue-on-error>`__),
774811
though the leg's check run still reads ``failure``. A job past its
775812
``timeout-minutes`` is cancelled, and that page does not say whether the job-level
776-
guard covers a cancellation. So setup-python, the install and the tests have caps
777-
of their own, of 5, 10 and 120 minutes, under the job's 140: a step at its cap
813+
guard covers a cancellation. So setup-python, the install, the watcher's setup-python
814+
and ``requests`` (`#1938 <https://github.com/JarryShaw/PyPCAPKit/issues/1938>`__,
815+
below) and the tests have caps of their own, of 5, 10, 1, 1 and 120 minutes, under
816+
the job's 140, which leaves 3 for the rest: a step at its cap
778817
fails rather than cancels, and the guard holds (actions/runner
779818
``src/Runner.Worker/StepsRunner.cs:322-353``). The runner's ``has timed out`` error
780819
stays, changing no conclusion, as ``Process completed with exit code N.`` does.

0 commit comments

Comments
 (0)