fix: bound every copr-cli call so a hung request cannot stall the job
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.
This commit is contained in:
21
CLAUDE.md
21
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:
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user