Archived
Fix nix_extra_opts falsely reporting nix-cache's SSH remote builder down
The reachability check used `cat < /dev/tcp/${NIX_CACHE_HOST}/22`, which
blocks forever reading for EOF that never comes -- sshd sends its banner
and then holds the connection open waiting for the client to speak next.
Every single check hit the 3s timeout and reported "unreachable"
unconditionally, regardless of whether the remote builder was actually up.
Confirmed live: a plain TCP connect (`exec 3<>/dev/tcp/...`, no read)
returns in ~60ms against a healthy nix-cache instead of always timing out.
Fixing that exposed a second, previously-dormant bug: `printf -v
NIX_EXTRA_OPTS '%q ' "${NIX_OPTS[@]}"` on a genuinely empty NIX_OPTS array
still runs one format pass and yields the literal `'' ` rather than an
empty string. A subprocess (e.g. sync-host-keys.sh) reusing this
process's decision via `eval "NIX_OPTS=(${NIX_EXTRA_OPTS})"` then rebuilt
a 1-element array holding an empty string instead of a 0-element array,
which broke `nix-shell "${NIX_OPTS[@]}" -p <pkg>` with a bogus positional
argument the moment NIX_OPTS was legitimately empty (nix-cache reachable)
-- something the first bug had made impossible to ever hit before.
Also adds a couple of retries (1s apart) to both checks as a secondary
safety net against genuine multi-second blips, on top of fixing the
checks themselves.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+52
-5
@@ -88,13 +88,60 @@ nix_extra_opts() {
|
||||
fi
|
||||
export NIX_EXTRA_OPTS_DECIDED=1
|
||||
NIX_OPTS=()
|
||||
if ! curl --silent --fail --max-time 3 "http://${NIX_CACHE_HOST}/nix-cache-info" >/dev/null 2>&1; then
|
||||
|
||||
# Retry a couple of times, 1s apart, before believing either check --
|
||||
# belt-and-suspenders against a genuine multi-second blip (nix-cache
|
||||
# restarting), on top of the fix below. Worst case (~11s total, host
|
||||
# genuinely gone) is still nowhere near the 15s+ *per lookup* nix's own
|
||||
# substituter retries would cost if this check didn't exist at all.
|
||||
local attempt cache_up=0 builder_up=0
|
||||
for attempt in 1 2 3; do
|
||||
if curl --silent --fail --max-time 3 "http://${NIX_CACHE_HOST}/nix-cache-info" >/dev/null 2>&1; then
|
||||
cache_up=1
|
||||
break
|
||||
fi
|
||||
[[ "$attempt" -lt 3 ]] && sleep 1
|
||||
done
|
||||
|
||||
if [[ "$cache_up" -eq 0 ]]; then
|
||||
echo "nix-cache (http://${NIX_CACHE_HOST}) is unreachable -- skipping it (substituter + remote builder) for the rest of this run." >&2
|
||||
NIX_OPTS=(--option substituters "https://cache.nixos.org/" --builders "")
|
||||
elif ! timeout 3 bash -c "cat < /dev/tcp/${NIX_CACHE_HOST}/22" >/dev/null 2>&1; then
|
||||
echo "nix-cache's SSH remote builder (nixremote@${NIX_CACHE_HOST}:22) is unreachable -- disabling remote builds for the rest of this run." >&2
|
||||
NIX_OPTS=(--builders "")
|
||||
else
|
||||
for attempt in 1 2 3; do
|
||||
# `exec 3<>/dev/tcp/...` just opens the fd and returns -- it does NOT
|
||||
# read from it. Confirmed live this is load-bearing, not stylistic:
|
||||
# the previous `cat < /dev/tcp/.../22` blocked forever and always hit
|
||||
# the timeout even against a perfectly healthy nix-cache, because
|
||||
# sshd sends its banner and then holds the connection open waiting
|
||||
# for the client to speak next -- `cat` never sees EOF, so this
|
||||
# check reported "unreachable" unconditionally, 100% of the time,
|
||||
# regardless of whether the remote builder was actually up.
|
||||
if timeout 3 bash -c "exec 3<>/dev/tcp/${NIX_CACHE_HOST}/22" 2>/dev/null; then
|
||||
builder_up=1
|
||||
break
|
||||
fi
|
||||
[[ "$attempt" -lt 3 ]] && sleep 1
|
||||
done
|
||||
if [[ "$builder_up" -eq 0 ]]; then
|
||||
echo "nix-cache's SSH remote builder (nixremote@${NIX_CACHE_HOST}:22) is unreachable -- disabling remote builds for the rest of this run." >&2
|
||||
NIX_OPTS=(--builders "")
|
||||
fi
|
||||
fi
|
||||
|
||||
# `printf '%q '` with a genuinely empty NIX_OPTS still runs one format
|
||||
# pass over a missing argument and yields the literal `'' ` rather than
|
||||
# an empty string (confirmed live) -- a subprocess that later does
|
||||
# `eval "NIX_OPTS=(${NIX_EXTRA_OPTS})"` (the branch above, for e.g.
|
||||
# sync-host-keys.sh reusing this process's decision) would then rebuild
|
||||
# a 1-element array holding an empty string instead of a 0-element
|
||||
# array, and `nix-shell "${NIX_OPTS[@]}" -p <pkg>` chokes on that stray
|
||||
# element as a bogus positional argument. Guard the empty case
|
||||
# explicitly so nix-cache being reachable (NIX_OPTS legitimately empty)
|
||||
# round-trips as truly empty instead.
|
||||
if [[ ${#NIX_OPTS[@]} -gt 0 ]]; then
|
||||
printf -v NIX_EXTRA_OPTS '%q ' "${NIX_OPTS[@]}"
|
||||
else
|
||||
NIX_EXTRA_OPTS=""
|
||||
fi
|
||||
printf -v NIX_EXTRA_OPTS '%q ' "${NIX_OPTS[@]}"
|
||||
export NIX_EXTRA_OPTS
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user