Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ jobs:
run: tests/storage-migration.sh
- name: set-count guards
run: tests/set-count-guards.sh
- name: watch-list staleness
run: tests/watch-list-staleness.sh
56 changes: 56 additions & 0 deletions lib/scheduler.sh
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ _rp_doctor() {

local gh_ok=1 pools=0 seen_orgs="" p running gh reg online
local tick clean i missing avail_kb cache_avail_kb free mode other phase hook_fails
local all_repos unwatched

_rp_doctor_fails=0
_rp_doctor_warns=0
Expand Down Expand Up @@ -356,6 +357,38 @@ _rp_doctor() {
"${p}: an org pool with no watched repositories never autoscales, because github reports queued runs per repository rather than per organisation" \
"fix: give it --watch OWNER/REPO,... in ~/.config/runpool/pools and 'runpool apply'"

# The same defect short of its limit: a watch list that is not empty and
# is no longer complete. The empty case above is caught locally; this one
# needs to know what the organisation holds, so it reports rather than
# fails and it names what it found rather than guessing intent.
#
# **It does not decide which repositories should be watched.** A
# repository at this scope may legitimately route every job to a managed
# runner, and nothing readable here distinguishes that from one that
# meant to reach the pool — the routing lives in a repository variable
# whose name is a convention of whoever set it up, not of RunPool. So
# the operator is told the difference and judges it.
#
# Reported here rather than polled at tick time on purpose. Waking on
# any queued run in the organisation would also wake for a public
# repository's, which the runner group refuses to serve, so the pool
# would come up for work it can never take and idle straight back down.
# Avoiding that needs the visibility of each repository, and re-deriving
# the public-repository answer is the one thing AGENTS.md says not to do.
if [ "${gh_ok}" = "1" ] && [ "${POOL_SCOPE}" = "org" ] && [ -n "${POOL_WATCH:-}" ]; then
all_repos=$(gh api "/orgs/${POOL_TARGET}/repos?per_page=100" --paginate --jq '.[].full_name' 2>/dev/null)
if [ -z "${all_repos}" ]; then
_rp_doctor_note "${p}: could not list ${POOL_TARGET}'s repositories, so the watch list was not checked for staleness"
else
unwatched=$(_rp_unwatched_repos "${POOL_WATCH}" "${all_repos}")
if [ -n "${unwatched}" ]; then
_rp_doctor_note "${p}: watching $(echo "${POOL_WATCH}" | tr ',' '\n' | grep -c .) of $(printf '%s\n' "${all_repos}" | grep -c .) repositories at ${POOL_TARGET}. A job queued by an unwatched one waits until a watched one happens to wake the pool: $(printf '%s' "${unwatched}" | tr '\n' ' ')"
else
_rp_doctor_ok "${p}: every repository at ${POOL_TARGET} is watched"
fi
fi
fi

if [ "${gh_ok}" = "0" ]; then
_rp_doctor_note "${p}: ${POOL_SCOPE} ${POOL_TARGET}, ${running}/${POOL_COUNT} running locally (github not checked)"
continue
Expand Down Expand Up @@ -497,6 +530,29 @@ _rp_doctor() {
return 0
}

# Repositories at an org pool's scope that its watch list does not name.
#
# `_rp_autoscale` polls POOL_WATCH because GitHub reports queued runs per
# repository and not per organisation, so the list IS the wake mechanism. A
# repository missing from it queues work nothing wakes the pool for, and
# that failure is silent in every direction: a queued job is not a failed
# job, so no alert fires, and the pool reports healthy because it is.
#
# Pure, and separate from the check that reports it, so the rule can be
# exercised without reaching GitHub. $1 is the watch list as stored, $2 is
# the repository list newline-separated.
_rp_unwatched_repos() {
local watch="$1" all="$2" r
watch=",$(echo "${watch}" | tr -d ' '),"
printf '%s\n' "${all}" | while IFS= read -r r; do
[ -n "${r}" ] || continue
case "${watch}" in
*",${r},"*) ;;
*) printf '%s\n' "${r}" ;;
esac
done
}

# ---------------------------------------------------------------------------
# autoscale — bring a pool up when it has queued work
# ---------------------------------------------------------------------------
Expand Down
81 changes: 81 additions & 0 deletions tests/watch-list-staleness.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#!/bin/bash
# The rule behind doctor's watch-list check, exercised without GitHub.
#
# `_rp_unwatched_repos` is what decides whether an org pool's watch list has
# gone stale, and the check that reports it needs the API. Separating the two
# is what lets the rule be tested at all.
set -uo pipefail

repo_dir=$(cd -P "$(dirname "$0")/.." && pwd)
scratch_dir=$(mktemp -d)
trap 'rm -rf "${scratch_dir}"' EXIT INT TERM

# Sourced rather than driven through the binary, because the rule under test
# is a pure function and the check that calls it needs GitHub. Every path the
# library reads at load time points into a scratch directory, so nothing here
# can see or touch a real installation.
export RUNPOOL_BASE="${scratch_dir}/base"
export RUNPOOL_STATE_DIR="${scratch_dir}/state"
export RUNPOOL_CACHE_DIR="${scratch_dir}/cache"
export RUNPOOL_CONFIG="${scratch_dir}/runpool.conf"
export RUNPOOL_POOLS_FILE="${scratch_dir}/pools"
export RUNPOOL_LOG_DIR="${scratch_dir}/logs"
export RUNPOOL_LOG="${scratch_dir}/logs/runpool.log"
export RUNPOOL_AGENT_DIR="${scratch_dir}/agents"
mkdir -p "${RUNPOOL_BASE}" "${RUNPOOL_STATE_DIR}" "${RUNPOOL_CACHE_DIR}" "${RUNPOOL_LOG_DIR}" "${RUNPOOL_AGENT_DIR}"

# shellcheck source=/dev/null
. "${repo_dir}/lib/common.sh" 2>/dev/null || true
# shellcheck source=/dev/null
. "${repo_dir}/lib/scheduler.sh"

fail() { echo "FAIL: $*" >&2; exit 1; }
pass=0
check() {
local label="$1" expected="$2" actual="$3"
if [ "${expected}" = "${actual}" ]; then
pass=$(( pass + 1 ))
else
fail "${label}: expected '${expected}', got '${actual}'"
fi
}

ALL=$'acme/one\nacme/two\nacme/three'

check "every repository watched" \
"" \
"$(_rp_unwatched_repos "acme/one,acme/two,acme/three" "${ALL}")"

check "one missing is named" \
"acme/three" \
"$(_rp_unwatched_repos "acme/one,acme/two" "${ALL}")"

check "several missing are all named" \
$'acme/two\nacme/three' \
"$(_rp_unwatched_repos "acme/one" "${ALL}")"

# The list is stored as one string and hand-edited, so spaces after commas
# are the likeliest way it is written. Treating "acme/two" and " acme/two"
# as different repositories would report a stale list that is not stale.
check "spaces around entries do not matter" \
"" \
"$(_rp_unwatched_repos "acme/one, acme/two , acme/three" "${ALL}")"

# A substring is not a match, and the direction matters. Without the comma
# fencing, a watch list naming `acme/one-more` would satisfy `acme/one`,
# because the shorter name occurs inside the longer one — so a genuinely
# unwatched repository would be reported as watched. That is the direction
# that fails silently: the check would say the list is complete when it is
# not, which is exactly the state it exists to catch.
check "a repository whose name occurs inside a watched one is still unwatched" \
"acme/one" \
"$(_rp_unwatched_repos "acme/one-more" $'acme/one\nacme/one-more')"

# A watch list naming something the organisation no longer holds is not this
# check's business: the failure it exists for is work nobody wakes for, and
# a repository that does not exist queues nothing.
check "a watched repository that no longer exists is not reported" \
"" \
"$(_rp_unwatched_repos "acme/one,acme/gone" "acme/one")"

echo "ok: ${pass} case(s)"