diff --git a/CLAUDE.md b/CLAUDE.md index c668bdf..4ecb533 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,6 +15,24 @@ from a consumer repo (primarily `helexa/cortex`). When debugging, fetch job logs via the `gitea-mcp` tools against the consumer repo, not this one — e.g. `mcp__gitea-mcp__actions_run_read` with `owner=helexa`, `repo=cortex`. +`tests/test-copr-build.sh` covers the parts that don't need a real COPR: it +puts a stub `copr-cli` on `PATH` and drives `scripts/copr-build.sh` through +succeeded / failed / canceled / skipped / never-settles, plus the case where +`watch-build` hangs. Run it directly (`./tests/test-copr-build.sh`); it needs +nothing but bash and takes a few seconds. + +## Why the verdict does not come from `watch-build` + +`copr-cli watch-build` holds a long-lived connection and can stop responding +while the build carries on and finishes normally. Observed in `monsoon` run 48: +COPR build 10835047 succeeded at 08:30:30, the watcher went silent after +08:09:11, and the runner killed the step at 08:35:50 — reported as a failed +build, which also blocked the dependent version-bump job. + +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. + ## Tagging & release workflow We use a **floating major tag** alongside specific semver tags: diff --git a/README.md b/README.md index 5a50e6e..4947855 100644 --- a/README.md +++ b/README.md @@ -13,10 +13,21 @@ This action: - Submits with `--nowait` and captures the build ID. - Prints a clickable `https://copr.fedorainfracloud.org/coprs/build/...` link so you can follow live. -- Watches the build to completion (blocks, propagates exit status). +- Streams status transitions from `copr-cli watch-build` while polling COPR for + the build's actual state, so the job's verdict comes from the build rather + than from the health of a long-lived connection. - On completion, fetches each chroot's `builder-live.log` via `copr-cli download-build` and emits them as `::group::` blocks. - Fails CI if the build fails, but always dumps logs first. +- Gives up after `COPR_BUILD_TIMEOUT` (default 7200s) and says so, rather than + hanging until the runner kills the job. + +### Environment overrides + +| Variable | Default | Purpose | +|---|---|---| +| `COPR_BUILD_TIMEOUT` | `7200` | Seconds to wait for a build to reach a terminal state. | +| `COPR_POLL_INTERVAL` | `30` | Seconds between build-state polls. | ## Requirements diff --git a/scripts/copr-build.sh b/scripts/copr-build.sh index 0c36171..7d15fea 100755 --- a/scripts/copr-build.sh +++ b/scripts/copr-build.sh @@ -9,6 +9,11 @@ set -o pipefail +# How long to wait for a build to reach a terminal state before giving up, and +# how often to ask COPR where it has got to. +BUILD_TIMEOUT="${COPR_BUILD_TIMEOUT:-7200}" +POLL_INTERVAL="${COPR_POLL_INTERVAL:-30}" + PROJECT="$1" shift @@ -17,6 +22,37 @@ if [ -z "$PROJECT" ] || [ "$#" -eq 0 ]; then exit 2 fi +# COPR states that mean the build has finished, one way or another. Anything +# else (pending, starting, running, importing, waiting) is still in progress. +is_terminal_state() { + case "$1" in + succeeded | failed | canceled | skipped) return 0 ;; + *) return 1 ;; + esac +} + +build_state() { + 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 +# terminal state, or "unknown" if we ran out of patience. +wait_for_terminal_state() { + local build_id="$1" + local deadline=$(($(date +%s) + BUILD_TIMEOUT)) + local state + state="$(build_state "$build_id")" + while ! is_terminal_state "$state"; do + if [ "$(date +%s)" -ge "$deadline" ]; then + echo "unknown" + return + fi + sleep "$POLL_INTERVAL" + state="$(build_state "$build_id")" + done + echo "$state" +} + # Submit without waiting; capture the build ID from stdout. SUBMIT_OUT=$(copr-cli build --nowait "$PROJECT" "$@") echo "$SUBMIT_OUT" @@ -32,14 +68,49 @@ echo "Build $BUILD_ID submitted to $PROJECT" echo "Follow live: https://copr.fedorainfracloud.org/coprs/build/$BUILD_ID" echo -# Watch the build; captures status transitions to stdout. Exit non-zero -# on build failure, but defer propagating that until after we've fetched -# logs so the CI output contains diagnostics either way. -if copr-cli watch-build "$BUILD_ID"; then - STATUS=0 -else - STATUS=$? -fi +# Stream status transitions for the operator's benefit, in the background and +# bounded so it cannot hang the job. +# +# Its exit status is deliberately ignored, and it is NOT what decides the +# verdict. `watch-build` holds a long-lived connection to COPR, and it has been +# seen to stop responding while the build carries on and finishes normally: in +# one case the build succeeded at 08:30:30 while the watcher sat silent until +# 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 +# failed" turns a green release into a red one and blocks any dependent job. +timeout "$BUILD_TIMEOUT" copr-cli watch-build "$BUILD_ID" & +WATCH_PID=$! + +# The verdict comes from COPR itself. Polling also means we stop as soon as the +# build settles, rather than waiting on the watcher to notice. +BUILD_STATE="$(wait_for_terminal_state "$BUILD_ID")" + +kill "$WATCH_PID" 2>/dev/null || true +wait "$WATCH_PID" 2>/dev/null || true + +echo +echo "COPR build $BUILD_ID final state: $BUILD_STATE" + +case "$BUILD_STATE" in + succeeded) + STATUS=0 + ;; + skipped) + # COPR already had this exact build; nothing was rebuilt, but nothing is + # wrong either. + echo "note: build $BUILD_ID was skipped (already built)" + STATUS=0 + ;; + failed | canceled) + echo "error: COPR build $BUILD_ID finished with state '$BUILD_STATE'" >&2 + STATUS=1 + ;; + *) + echo "error: gave up after ${BUILD_TIMEOUT}s waiting for COPR build $BUILD_ID to finish" >&2 + echo " check https://copr.fedorainfracloud.org/coprs/build/$BUILD_ID" >&2 + STATUS=1 + ;; +esac # Fetch per-chroot results (logs + rpms). Anonymous download — no auth needed. LOG_DIR="$(mktemp -d -t copr-logs.XXXXXX)" diff --git a/tests/test-copr-build.sh b/tests/test-copr-build.sh new file mode 100755 index 0000000..a364fd5 --- /dev/null +++ b/tests/test-copr-build.sh @@ -0,0 +1,101 @@ +#!/bin/bash +# Tests for scripts/copr-build.sh, driven by a stub `copr-cli` on PATH. +# +# The case that matters most is a watcher that hangs while the build succeeds +# anyway: that is what made a green COPR release report as a failed CI job. +# +# Usage: tests/test-copr-build.sh + +set -o pipefail + +HERE="$(cd "$(dirname "$0")" && pwd)" +SCRIPT="$HERE/../scripts/copr-build.sh" +PASS=0 +FAIL=0 + +# Build a stub copr-cli whose behaviour is driven by env vars, and put it first +# on PATH. STUB_STATES is a space-separated list of states returned by +# successive `status` calls; the last one repeats. +make_stub() { + local dir="$1" + mkdir -p "$dir" + cat > "$dir/copr-cli" <<'STUB' +#!/bin/bash +case "$1" in + build) + echo "Uploading package test.src.rpm" + echo "Created builds: 12345" + ;; + watch-build) + if [ "${STUB_WATCH_HANGS:-0}" = "1" ]; then + # Mimic a watcher that streams a little then stops responding. + echo " Build 12345: running" + sleep 3600 + else + echo " Build 12345: succeeded" + fi + ;; + status) + # Pop the next state; the final one repeats forever. + read -r -a states <<< "${STUB_STATES:-succeeded}" + idx_file="${STUB_IDX_FILE:-/tmp/stub_idx}" + idx=$(cat "$idx_file" 2>/dev/null || echo 0) + if [ "$idx" -ge "${#states[@]}" ]; then idx=$(( ${#states[@]} - 1 )); fi + echo "${states[$idx]}" + echo $(( idx + 1 )) > "$idx_file" + ;; + download-build) + exit 0 + ;; +esac +STUB + chmod +x "$dir/copr-cli" +} + +run_case() { + local name="$1" expected_rc="$2" expected_text="$3" + shift 3 + local tmp out rc + 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}" \ + bash "$SCRIPT" owner/project test.src.rpm 2>&1)" + rc=$? + if [ "$rc" -eq "$expected_rc" ] && grep -q "$expected_text" <<< "$out"; then + echo "ok - $name" + PASS=$((PASS + 1)) + else + echo "FAIL - $name (rc=$rc want $expected_rc; wanted text: $expected_text)" + sed 's/^/ | /' <<< "$out" + FAIL=$((FAIL + 1)) + fi + rm -rf "$tmp" +} + +# The regression: watcher stops responding, build succeeded anyway. +run_case "hung watcher + succeeded build exits 0" \ + 0 "final state: succeeded" STUB_WATCH_HANGS=1 STUB_STATES="running succeeded" + +# A genuine failure must still fail, hung watcher or not. +run_case "hung watcher + failed build exits 1" \ + 1 "finished with state 'failed'" STUB_WATCH_HANGS=1 STUB_STATES="running failed" + +# Normal path. +run_case "healthy watcher + succeeded build exits 0" \ + 0 "final state: succeeded" STUB_WATCH_HANGS=0 STUB_STATES="succeeded" + +run_case "canceled build exits 1" \ + 1 "finished with state 'canceled'" STUB_WATCH_HANGS=0 STUB_STATES="canceled" + +# Already-built is not an error. +run_case "skipped build exits 0" \ + 0 "was skipped" STUB_WATCH_HANGS=0 STUB_STATES="skipped" + +# 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" + +echo +echo "passed: $PASS failed: $FAIL" +[ "$FAIL" -eq 0 ]