fix: require every chroot to succeed, and repair a stale timeout variable

Two things.

A build-level "succeeded" says the build finished, not that every chroot
produced an RPM. The job could go green while fedora-43, fedora-44 or rawhide
had failed or was still building. Poll build-chroot/list alongside the build
state, keep waiting while any chroot is unfinished, and fail if any finished in
anything other than succeeded or skipped. Per-chroot results are printed at the
end and included in the heartbeat, so a slow chroot is visible while it runs.

copr-cli has no per-chroot status subcommand, so this reads COPR's public API
directly with python3 — already a hard dependency of copr-cli, so nothing new
is required. COPR_API_BASE overrides the instance.

Separately, renaming BUILD_TIMEOUT to WAIT_BUDGET in v1.0.5 missed the
watch-build invocation, leaving `timeout "" copr-cli watch-build`. That failed
immediately, so the progress stream has not actually run since v1.0.5 —
harmless only because the verdict comes from polling, and caught here because
the new tests surfaced the error text.

The unfinished-versus-failed distinction is deliberate and load-bearing:
reporting a still-running chroot as a failure would be the same mistake this
action has now made in three different guises.
This commit is contained in:
2026-08-07 16:42:30 +03:00
parent 8fa9802d58
commit 95a84e4ad8
4 changed files with 128 additions and 9 deletions

View File

@@ -59,6 +59,20 @@ state transitions, and a silent step is killed by the runner's inactivity
timeout. The poll loop prints a heartbeat every interval; that is load-bearing, timeout. The poll loop prints a heartbeat every interval; that is load-bearing,
not decoration. not decoration.
## The verdict comes from the chroots, not just the build
A build-level `succeeded` says the build finished, not that every chroot
produced an RPM. The script polls `build-chroot/list` and requires each chroot
to reach `succeeded` (or `skipped`), so one release failing while the others
pass still fails the job. It also keeps waiting while any chroot is unfinished,
which the build-level state alone would let it skip past.
`copr-cli` has no per-chroot status subcommand, hence the direct API call via
`python3` — which `copr-cli` already depends on, so it is not a new
requirement. Keep the "unfinished is not failed" distinction: reporting a
still-running chroot as a failure is the same mistake this action has now made
in three different guises.
## Consumers should pin an immutable tag ## Consumers should pin an immutable tag
The runner caches actions by ref, so moving the floating `v1` does **not** The runner caches actions by ref, so moving the floating `v1` does **not**

View File

@@ -18,8 +18,11 @@ This action:
than from the health of a long-lived connection. than from the health of a long-lived connection.
- On completion, fetches each chroot's `builder-live.log` via - On completion, fetches each chroot's `builder-live.log` via
`copr-cli download-build` and emits them as `::group::` blocks. `copr-cli download-build` and emits them as `::group::` blocks.
- Fails CI if the build **fails**. A build merely still running when the wait - Waits for **every chroot** to finish, not just the build as a whole, and fails
budget expires is reported, not failed. CI if any of them failed. A build-level "succeeded" is not the same as every
release having produced an RPM.
- Fails CI if the build **fails**. A build or chroot merely still running when
the wait budget expires is reported, not failed.
- Returns within `COPR_WAIT_BUDGET` whatever happens, so the step finishes - Returns within `COPR_WAIT_BUDGET` whatever happens, so the step finishes
before a CI runner's step limit kills it. before a CI runner's step limit kills it.
@@ -29,6 +32,7 @@ This action:
|---|---|---| |---|---|---|
| `COPR_WAIT_BUDGET` | `2700` | Seconds to wait for the build before returning. A build still running at expiry is reported as such and the step exits 0 — CI runners commonly kill a step long before a large build finishes. | | `COPR_WAIT_BUDGET` | `2700` | Seconds to wait for the build before returning. A build still running at expiry is reported as such and the step exits 0 — CI runners commonly kill a step long before a large build finishes. |
| `COPR_POLL_INTERVAL` | `30` | Seconds between build-state polls. | | `COPR_POLL_INTERVAL` | `30` | Seconds between build-state polls. |
| `COPR_API_BASE` | `https://copr.fedorainfracloud.org` | COPR instance queried for per-chroot state. |
## Requirements ## Requirements
@@ -36,6 +40,8 @@ The runner must already have:
- `copr-cli` on `PATH` (provided by the `copr-cli` RPM on Fedora runners). - `copr-cli` on `PATH` (provided by the `copr-cli` RPM on Fedora runners).
- `grep -P` (PCRE, default on Fedora). - `grep -P` (PCRE, default on Fedora).
- `python3`, used to read per-chroot state from COPR's API. Already a hard
dependency of `copr-cli`, so this adds nothing in practice.
## Inputs ## Inputs

View File

@@ -58,7 +58,38 @@ build_state() {
timeout "$STATUS_TIMEOUT" copr-cli status "$1" 2>/dev/null | tail -n1 | tr -d '[:space:]' timeout "$STATUS_TIMEOUT" copr-cli status "$1" 2>/dev/null | tail -n1 | tr -d '[:space:]'
} }
# Poll COPR until the build settles, or until BUILD_TIMEOUT elapses. Echoes the # Per-chroot states, one "<name> <state>" per line.
#
# The build-level state is not enough on its own: it describes the build as a
# whole, and a green tick there is not the same as "every chroot produced an
# RPM". copr-cli has no per-chroot status subcommand, so this uses COPR's public
# API. python3 is a hard dependency of copr-cli itself, so it adds no new
# requirement, and the endpoint needs no authentication.
chroot_states() {
timeout "$STATUS_TIMEOUT" python3 -c '
import json, os, sys, urllib.request
base = os.environ.get("COPR_API_BASE", "https://copr.fedorainfracloud.org")
url = f"{base}/api_3/build-chroot/list?build_id={sys.argv[1]}"
with urllib.request.urlopen(url, timeout=20) as r:
for c in json.load(r).get("items", []):
print(c.get("name", "?"), c.get("state", "unknown"))
' "$1" 2>/dev/null
}
# True when every chroot has finished. Empty or unreadable output is treated as
# "not yet", so a transient API failure retries rather than passing the build.
all_chroots_finished() {
local out name state
out="$(chroot_states "$1")"
[ -n "$out" ] || return 1
while read -r name state; do
[ -n "$name" ] || continue
is_terminal_state "$state" || return 1
done <<< "$out"
return 0
}
# Poll COPR until the build settles, or until WAIT_BUDGET elapses. Echoes the
# terminal state, or "unknown" if we ran out of patience. # terminal state, or "unknown" if we ran out of patience.
# Poll until the build settles or the budget runs out. Echoes the last state # Poll until the build settles or the budget runs out. Echoes the last state
# seen, which the caller checks for terminality — a non-terminal state means we # seen, which the caller checks for terminality — a non-terminal state means we
@@ -69,7 +100,7 @@ wait_for_terminal_state() {
started=$(date +%s) started=$(date +%s)
deadline=$((started + WAIT_BUDGET)) deadline=$((started + WAIT_BUDGET))
state="$(build_state "$build_id")" state="$(build_state "$build_id")"
while ! is_terminal_state "$state"; do while ! is_terminal_state "$state" || ! all_chroots_finished "$build_id"; do
now=$(date +%s) now=$(date +%s)
if [ "$now" -ge "$deadline" ]; then if [ "$now" -ge "$deadline" ]; then
echo "${state:-unknown}" echo "${state:-unknown}"
@@ -81,8 +112,9 @@ wait_for_terminal_state() {
# inactivity timeout before it can finish. Two builds died this way after # inactivity timeout before it can finish. Two builds died this way after
# ~24-27 minutes of no output, one of them 66 seconds before COPR reported # ~24-27 minutes of no output, one of them 66 seconds before COPR reported
# success. Printing every poll keeps the step alive and shows progress. # success. Printing every poll keeps the step alive and shows progress.
printf ' [%3dm %3ds] build %s: %s\n' \ printf ' [%3dm %3ds] build %s: %s | %s\n' \
$(((now - started) / 60)) $(((now - started) % 60)) "$build_id" "${state:-unknown}" >&2 $(((now - started) / 60)) $(((now - started) % 60)) "$build_id" "${state:-unknown}" \
"$(chroot_states "$build_id" | awk '{printf "%s=%s ", $1, $2}')" >&2
sleep "$POLL_INTERVAL" sleep "$POLL_INTERVAL"
state="$(build_state "$build_id")" state="$(build_state "$build_id")"
done done
@@ -114,7 +146,7 @@ echo
# the runner killed the step at 08:35:50, which the job then reported as a # the runner killed the step at 08:35:50, which the job then reported as a
# build failure. Conflating "the watcher lost its connection" with "the build # build failure. Conflating "the watcher lost its connection" with "the build
# failed" turns a green release into a red one and blocks any dependent job. # failed" turns a green release into a red one and blocks any dependent job.
timeout "$BUILD_TIMEOUT" copr-cli watch-build "$BUILD_ID" & timeout "$WAIT_BUDGET" copr-cli watch-build "$BUILD_ID" &
WATCH_PID=$! WATCH_PID=$!
# The verdict comes from COPR itself. Polling also means we stop as soon as the # The verdict comes from COPR itself. Polling also means we stop as soon as the
@@ -127,9 +159,48 @@ wait "$WATCH_PID" 2>/dev/null || true
echo echo
echo "COPR build $BUILD_ID final state: $BUILD_STATE" echo "COPR build $BUILD_ID final state: $BUILD_STATE"
CHROOTS="$(chroot_states "$BUILD_ID")"
if [ -n "$CHROOTS" ]; then
echo "Per-chroot results:"
while read -r cname cstate; do
[ -n "$cname" ] || continue
echo " $cname: $cstate"
done <<< "$CHROOTS"
fi
# A chroot that failed while the build as a whole reads succeeded still means no
# RPM for that release, so the job must not go green on it. A chroot that simply
# has not finished is a different thing and must not be reported as a failure —
# that is the mistake this action has made in every other guise.
FAILED_CHROOTS="$(
while read -r cname cstate; do
[ -n "$cname" ] || continue
case "$cstate" in
succeeded | skipped | importing | pending | starting | running | waiting) ;;
*) printf '%s(%s) ' "$cname" "$cstate" ;;
esac
done <<< "$CHROOTS"
)"
UNFINISHED_CHROOTS="$(
while read -r cname cstate; do
[ -n "$cname" ] || continue
is_terminal_state "$cstate" || printf '%s(%s) ' "$cname" "$cstate"
done <<< "$CHROOTS"
)"
case "$BUILD_STATE" in case "$BUILD_STATE" in
succeeded) succeeded)
STATUS=0 if [ -n "$FAILED_CHROOTS" ]; then
echo "error: build $BUILD_ID reports succeeded but these chroots did not: $FAILED_CHROOTS" >&2
STATUS=1
elif [ -n "$UNFINISHED_CHROOTS" ]; then
echo "note: build $BUILD_ID has not finished within our ${WAIT_BUDGET}s wait budget"
echo " (chroots still going: $UNFINISHED_CHROOTS). COPR will finish it on its own."
echo " Follow: https://copr.fedorainfracloud.org/coprs/build/$BUILD_ID"
STATUS=0
else
STATUS=0
fi
;; ;;
skipped) skipped)
# COPR already had this exact build; nothing was rebuilt, but nothing is # COPR already had this exact build; nothing was rebuilt, but nothing is

View File

@@ -19,6 +19,17 @@ FAIL=0
make_stub() { make_stub() {
local dir="$1" local dir="$1"
mkdir -p "$dir" mkdir -p "$dir"
# chroot_states shells out to python3; stub it so the per-chroot path is
# exercised without touching COPR. STUB_CHROOTS is "name=state,name=state".
cat > "$dir/python3" <<'PYSTUB'
#!/bin/bash
IFS=',' read -r -a pairs <<< "${STUB_CHROOTS:-}"
for p in "${pairs[@]}"; do
[ -n "$p" ] || continue
echo "${p%%=*} ${p##*=}"
done
PYSTUB
chmod +x "$dir/python3"
cat > "$dir/copr-cli" <<'STUB' cat > "$dir/copr-cli" <<'STUB'
#!/bin/bash #!/bin/bash
case "$1" in case "$1" in
@@ -65,7 +76,8 @@ run_case() {
local tmp out rc local tmp out rc
tmp="$(mktemp -d)" tmp="$(mktemp -d)"
make_stub "$tmp/bin" make_stub "$tmp/bin"
out="$(env "$@" STUB_IDX_FILE="$tmp/idx" PATH="$tmp/bin:$PATH" \ out="$(env STUB_CHROOTS="${STUB_CHROOTS:-f43=succeeded,f44=succeeded,rawhide=succeeded}" \
"$@" STUB_IDX_FILE="$tmp/idx" PATH="$tmp/bin:$PATH" \
COPR_POLL_INTERVAL=1 COPR_BUILD_TIMEOUT="${CASE_TIMEOUT:-20}" COPR_STATUS_TIMEOUT="${COPR_STATUS_TIMEOUT:-10}" \ COPR_POLL_INTERVAL=1 COPR_BUILD_TIMEOUT="${CASE_TIMEOUT:-20}" COPR_STATUS_TIMEOUT="${COPR_STATUS_TIMEOUT:-10}" \
bash "$SCRIPT" owner/project test.src.rpm 2>&1)" bash "$SCRIPT" owner/project test.src.rpm 2>&1)"
rc=$? rc=$?
@@ -111,6 +123,22 @@ CASE_TIMEOUT=30 run_case "a hung status call does not stall the poll loop" \
0 "final state: succeeded" STUB_WATCH_HANGS=1 STUB_STATES="running succeeded" \ 0 "final state: succeeded" STUB_WATCH_HANGS=1 STUB_STATES="running succeeded" \
STUB_STATUS_HANGS_UNTIL=1 COPR_STATUS_TIMEOUT=2 STUB_STATUS_HANGS_UNTIL=1 COPR_STATUS_TIMEOUT=2
# A chroot that failed while the build as a whole reads succeeded still means no
# RPM for that release. The job must not go green on it.
run_case "a failed chroot fails the job even when the build says succeeded" \
1 "these chroots did not" STUB_WATCH_HANGS=0 STUB_STATES="succeeded" \
STUB_CHROOTS="f43=succeeded,f44=failed,rawhide=succeeded"
# All three green is the ordinary pass, and each is reported.
run_case "all chroots succeeding passes and is reported" \
0 "rawhide: succeeded" STUB_WATCH_HANGS=0 STUB_STATES="succeeded" \
STUB_CHROOTS="f43=succeeded,f44=succeeded,rawhide=succeeded"
# The build settling before its chroots do must not end the wait early.
CASE_TIMEOUT=4 run_case "build succeeded but a chroot still running keeps waiting" \
0 "chroots still going: f44(running)" STUB_WATCH_HANGS=0 STUB_STATES="succeeded" \
STUB_CHROOTS="f43=succeeded,f44=running,rawhide=succeeded"
# The step limit case: the build is still running when our wait budget expires. # The step limit case: the build is still running when our wait budget expires.
# This must NOT be reported as a failure — that is what turned every green # This must NOT be reported as a failure — that is what turned every green
# release red — and it must not try to fetch logs that do not exist yet. # release red — and it must not try to fetch logs that do not exist yet.