diff --git a/bin/runpool b/bin/runpool index 40b8189..a54ab63 100755 --- a/bin/runpool +++ b/bin/runpool @@ -43,7 +43,7 @@ export RUNPOOL_INVOKED # The released version, and the only place it is written. The Homebrew formula # builds from a git tag, so a tag without a matching bump here ships a binary # that misreports itself. -RUNPOOL_VERSION="0.12.0" +RUNPOOL_VERSION="0.12.1" # shellcheck source=lib/common.sh . "${RUNPOOL_ROOT}/lib/common.sh" diff --git a/lib/lifecycle.sh b/lib/lifecycle.sh index dab2f1a..bf8793a 100644 --- a/lib/lifecycle.sh +++ b/lib/lifecycle.sh @@ -571,7 +571,15 @@ _rp_move_dir() { mv "${from}" "${to}" } -_rp_migrate_rewrite_runner_links() { +# GitHub's runner updater points `bin` and `externals` at its versioned +# directories with ABSOLUTE symlinks, so a runner directory that moves takes +# two dangling links with it and `./config.sh` dies on a missing +# Runner.Listener. Rewriting them relative makes the installation +# self-contained, and therefore safe to move again. +# +# Every caller that relocates a runner directory has to do this, and do it +# before running config.sh rather than after. +_rp_rewrite_runner_links() { local runner_dir="$1" name link target target_name for name in bin externals; do link="${runner_dir}/${name}" @@ -579,12 +587,9 @@ _rp_migrate_rewrite_runner_links() { target=$(readlink "${link}") || return 1 target_name="${target##*/}" [ -n "${target_name}" ] && [ -d "${runner_dir}/${target_name}" ] || { - _rp_err "migration copied an unresolved runner link: ${link} -> ${target}" + _rp_err "unresolved runner link: ${link} -> ${target}" return 1 } - # GitHub's runner updater creates absolute links to its versioned bin and - # externals directories. Make the copied installation self-contained so - # removing the legacy tree cannot break Runner.Listener afterwards. ln -sfn "${target_name}" "${link}" || return 1 done } @@ -697,7 +702,7 @@ _rp_migrate_storage() { while [ "${i}" -le "${POOL_COUNT}" ]; do runner_dir="${new_dir}/runner-${i}" cache_dir="${cache_pool}/runner-${i}" - _rp_migrate_rewrite_runner_links "${runner_dir}" || return 1 + _rp_rewrite_runner_links "${runner_dir}" || return 1 mkdir -p "${cache_dir}" || return 1 # A work tree is regenerable but not necessarily portable. Compilers can # bake its absolute path into build artifacts, and a remote cache can @@ -1074,6 +1079,10 @@ CONF _rp_log "${new} runner-${i}: past the pool's count of ${POOL_COUNT}, deregistered and left on disk" continue } + # Before config.sh, not after: the runner's bin and externals links are + # absolute and now point into the directory this pool used to live in, so + # ./config.sh cannot even start until they are made relative. + _rp_rewrite_runner_links "${runner_dir}" || return 1 _rp_migrate_update_work_folder "${runner_dir}" "$(_rp_runner_work_dir "${new}" "${i}")" || return 1 runner_name="$(hostname -s)-${new}-${i}" _rp_log "${new} runner-${i}: registering as '${runner_name}'" diff --git a/tests/pool-rename.sh b/tests/pool-rename.sh index 1d16082..2a7d384 100755 --- a/tests/pool-rename.sh +++ b/tests/pool-rename.sh @@ -81,12 +81,27 @@ build_pool() { >"${dir}/runner-${i}/.runner" # A config.sh that behaves like the real one: refuses nothing, writes a # .runner, and records exactly what it was asked for. + # Faithful to the real one in the way that matters here: it runs + # ./bin/Runner.Listener, so a dangling bin link makes it fail before it + # registers anything. A stub that ignored the links would pass whether or + # not they were rewritten, and prove nothing. cat >"${dir}/runner-${i}/config.sh" <&2; exit 1; } echo "\$*" >> "${cfg_log}" printf '{"agentId": 99, "workFolder": "x"}\n' > .runner STUB chmod +x "${dir}/runner-${i}/config.sh" + # GitHub's runner updater points bin and externals at versioned directories + # with ABSOLUTE links. A directory that moves therefore takes two dangling + # links with it, and config.sh dies on a missing Runner.Listener before it + # can register anything. The fixture has to carry them or the rename looks + # like it works. + mkdir -p "${dir}/runner-${i}/bin.2.337.0" "${dir}/runner-${i}/externals.2.337.0" + printf '#!/bin/bash\n' >"${dir}/runner-${i}/bin.2.337.0/Runner.Listener" + chmod +x "${dir}/runner-${i}/bin.2.337.0/Runner.Listener" + ln -sfn "${dir}/runner-${i}/bin.2.337.0" "${dir}/runner-${i}/bin" + ln -sfn "${dir}/runner-${i}/externals.2.337.0" "${dir}/runner-${i}/externals" printf 'plist for %s runner-%s\n' "${name}" "${i}" \ >"${RUNPOOL_AGENT_DIR}/runpool.${name}.${i}.plist" i=$(( i + 1 )) @@ -136,6 +151,21 @@ absent "${RUNPOOL_BASE}/runners/alpha" "the old runner tree" present "${RUNPOOL_CACHE_DIR}/pools/bravo" "the cache moved" absent "${RUNPOOL_CACHE_DIR}/pools/alpha" "the old cache" +# The links have to be relative afterwards, and they have to resolve. An +# absolute link left pointing into the old path is a runner that cannot start, +# and config.sh fails before registering, which leaves the pool half renamed. +for r in 1 2; do + link="${RUNPOOL_BASE}/runners/bravo/runner-${r}/bin" + [ -L "${link}" ] || fail "runner-${r}: bin is not a symlink" + case "$(readlink "${link}")" in + /*) fail "runner-${r}: bin is still an absolute link into the old path" ;; + esac + [ -d "${link}/" ] || fail "runner-${r}: bin does not resolve after the move" + ok + [ -d "${RUNPOOL_BASE}/runners/bravo/runner-${r}/externals/" ] || fail "runner-${r}: externals does not resolve" + ok +done + # Written by _rp_write_plist against the new label, and the old ones removed by # hand: _rp_rewrite_plists only ever writes. present "${RUNPOOL_AGENT_DIR}/runpool.bravo.1.plist" "the new launch agent"