From d690d5aaa8a4c642ad964df5b828b560ea13e9f1 Mon Sep 17 00:00:00 2001 From: rob thijssen Date: Fri, 7 Aug 2026 13:25:50 +0300 Subject: [PATCH] fix: bound every copr-cli call so a hung request cannot stall the job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1.0.3's heartbeat did its job: it showed exactly where the next failure was. In monsoon run 56 the heartbeats stopped at 14m29s while the build was still running, and the job was killed at 10:20:50 — five minutes after COPR had reported success at 10:15:20. The poll loop was blocked inside `$(build_state ...)`. copr-cli makes network requests with no internal timeout, so `copr-cli status` hung for over twelve minutes and took the loop with it. Same failure mode as watch-build in run 48, just relocated. Bound all of them: status (60s), submit (600s), download-build (600s), each overridable. A timed-out status yields empty output, which reads as non-terminal, so the loop simply heartbeats and retries on the next tick. Test reproduces it with a stub whose first status call hangs; it deadlocks against v1.0.3 and passes here. --- CLAUDE.md | 21 +++++++++++++++++++++ scripts/copr-build.sh | 18 +++++++++++++++--- tests/test-copr-build.sh | 16 +++++++++++++++- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4ecb533..466ee74 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,6 +33,27 @@ So the script polls `copr-cli status` for the authoritative state and treats `watch-build` purely as a progress stream whose exit code is ignored. If you are tempted to simplify this back into `if copr-cli watch-build; then`, don't. +## Two rules learned the hard way + +**Bound every `copr-cli` call.** They make network requests with no internal +timeout and will block forever. This bit us twice in different places — +`watch-build` in run 48, then `status` in run 56, where the poll loop sat +inside a command substitution for twelve minutes and the job was killed five +minutes after COPR had reported success. Any new `copr-cli` invocation gets a +`timeout`. + +**Never leave the step silent.** A long build produces no COPR output between +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, +not decoration. + +## Consumers should pin an immutable tag + +The runner caches actions by ref, so moving the floating `v1` does **not** +invalidate it — monsoon ran three releases against a stale cached copy after +v1 had been moved to the fix. `v1` is still maintained for convenience, but +consumers that need a specific fix must pin `vX.Y.Z`. + ## Tagging & release workflow We use a **floating major tag** alongside specific semver tags: diff --git a/scripts/copr-build.sh b/scripts/copr-build.sh index db8542c..6148736 100755 --- a/scripts/copr-build.sh +++ b/scripts/copr-build.sh @@ -13,6 +13,11 @@ set -o pipefail # how often to ask COPR where it has got to. BUILD_TIMEOUT="${COPR_BUILD_TIMEOUT:-7200}" POLL_INTERVAL="${COPR_POLL_INTERVAL:-30}" +# Every copr-cli invocation is bounded. They perform network calls with no +# internal timeout, and a hang in any of them stalls the whole job. +STATUS_TIMEOUT="${COPR_STATUS_TIMEOUT:-60}" +SUBMIT_TIMEOUT="${COPR_SUBMIT_TIMEOUT:-600}" +DOWNLOAD_TIMEOUT="${COPR_DOWNLOAD_TIMEOUT:-600}" PROJECT="$1" shift @@ -31,8 +36,15 @@ is_terminal_state() { esac } +# Ask COPR for a build's state. Empty output means "could not tell", which the +# caller treats as non-terminal and retries. +# +# Bounded, because copr-cli's network calls can block indefinitely. One release +# hung here for over twelve minutes: the poll loop sat inside this command +# substitution, heartbeats stopped, and the job was killed five minutes after +# COPR had already reported success. build_state() { - 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 @@ -64,7 +76,7 @@ wait_for_terminal_state() { } # Submit without waiting; capture the build ID from stdout. -SUBMIT_OUT=$(copr-cli build --nowait "$PROJECT" "$@") +SUBMIT_OUT=$(timeout "$SUBMIT_TIMEOUT" copr-cli build --nowait "$PROJECT" "$@") echo "$SUBMIT_OUT" BUILD_ID=$(echo "$SUBMIT_OUT" | grep -oP 'Created builds: \K[0-9]+' | head -n1) @@ -124,7 +136,7 @@ esac # Fetch per-chroot results (logs + rpms). Anonymous download — no auth needed. LOG_DIR="$(mktemp -d -t copr-logs.XXXXXX)" -copr-cli download-build --dest "$LOG_DIR" "$BUILD_ID" || { +timeout "$DOWNLOAD_TIMEOUT" copr-cli download-build --dest "$LOG_DIR" "$BUILD_ID" || { echo "warning: failed to download build artifacts" >&2 } diff --git a/tests/test-copr-build.sh b/tests/test-copr-build.sh index f2dfdb8..603b650 100755 --- a/tests/test-copr-build.sh +++ b/tests/test-copr-build.sh @@ -36,6 +36,13 @@ case "$1" in fi ;; status) + # Optionally hang, reproducing copr-cli blocking on a stalled network call. + if [ -n "${STUB_STATUS_HANGS_UNTIL:-}" ]; then + hidx_file="${STUB_IDX_FILE:-/tmp/stub_idx}.hang" + hidx=$(cat "$hidx_file" 2>/dev/null || echo 0) + echo $(( hidx + 1 )) > "$hidx_file" + if [ "$hidx" -lt "$STUB_STATUS_HANGS_UNTIL" ]; then sleep 3600; fi + fi # Pop the next state; the final one repeats forever. read -r -a states <<< "${STUB_STATES:-succeeded}" idx_file="${STUB_IDX_FILE:-/tmp/stub_idx}" @@ -59,7 +66,7 @@ run_case() { tmp="$(mktemp -d)" make_stub "$tmp/bin" out="$(env "$@" STUB_IDX_FILE="$tmp/idx" PATH="$tmp/bin:$PATH" \ - COPR_POLL_INTERVAL=1 COPR_BUILD_TIMEOUT="${CASE_TIMEOUT:-20}" \ + 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)" rc=$? if [ "$rc" -eq "$expected_rc" ] && grep -q "$expected_text" <<< "$out"; then @@ -97,6 +104,13 @@ run_case "skipped build exits 0" \ CASE_TIMEOUT=6 run_case "slow build emits a heartbeat while waiting" \ 0 "build 12345: running" STUB_WATCH_HANGS=1 STUB_STATES="running running running succeeded" +# The v1.0.3 failure: copr-cli status blocks forever, so the poll loop stalls +# inside the command substitution and heartbeats stop. Bounding the call lets +# the loop recover and still reach the right verdict. +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" \ + STUB_STATUS_HANGS_UNTIL=1 COPR_STATUS_TIMEOUT=2 + # Never settles: bounded, and reported as a timeout rather than a build failure. CASE_TIMEOUT=3 run_case "build that never settles times out" \ 1 "gave up after" STUB_WATCH_HANGS=1 STUB_STATES="running"