Skip to content

NAS-144733 / 28.0.0-BETA.1 / truenas-appliance-runner: wait for node B to answer ssh from a new boot after giving it its own ids #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
william-gr merged 2 commits into from
Oct 19, 2026
Merged

NAS-144733 / 28.0.0-BETA.1 / truenas-appliance-runner: wait for node B to answer ssh from a new boot after giving it its own ids #24

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59fd874
truenas-appliance-runner: wait for node B to answer ssh from a new bo…
william-gr Oct 19, 2026
10b12af
truenas-appliance-runner: give node B ten minutes by the clock, retry…
william-gr Oct 19, 2026
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
22 changes: 16 additions & 6 deletions .github/actions/truenas-appliance-runner/ha.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,24 @@
head -c 16 /dev/urandom | od -An -tx1 | tr -d " \n" | sudo tee /etc/machine-id >/dev/null
sudo rm -f /var/lib/dbus/machine-id
sudo zgenhostid -f'
local boot now deadline
boot="$(ssh_ b cat /proc/sys/kernel/random/boot_id)"
ssh_ b 'sudo systemctl reboot' || true
# Down before it is waited for: the API answers for a while after the
# reboot is asked for.
local i
for i in $(seq 1 60); do
ssh_ b true 2>/dev/null || break
sleep 2
# Back once B answers ssh from a new boot. Neither "ssh stopped answering"
# nor "the API answers" proves that: the API answers for a while after the
# reboot is asked for, and on the new boot it can answer before sshd does
# (run 39251437564 read B's ids over ssh and was refused).
# A refused or timed-out connection (sshd not started yet, the network
# not up) is just tried again.
deadline=$((SECONDS + 600))
while [ "$SECONDS" -lt "$deadline" ]; do
now="$(ssh_ b cat /proc/sys/kernel/random/boot_id 2>/dev/null)" || now=

Check notice on line 246 in .github/actions/truenas-appliance-runner/ha.sh

View workflow job for this annotation

GitHub Actions / claude-review / Automatic PR review 

LOW: The 10-minute deadline is only checked between polls; ssh_ has no ServerAliveInterval, so one poll that connects just before B's guest resets can hang past it

@github-actions github-actions Bot Oct 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — The 10-minute deadline is only checked between polls, so it doesn't cap a single poll. ConnectTimeout=10 only covers connection setup. ssh_ sets no ServerAliveInterval/ServerAliveCountMax, so if a poll connects just before B's guest resets (the old boot's sshd is still up during shutdown), the reset sends no RST. That ssh call can then block on a dead TCP connection far past the deadline, and the die never fires. The window is small because cat returns at once, so this is unlikely. If you want "by the clock" to actually hold, add -o ServerAliveInterval=5 -o ServerAliveCountMax=2 here (or in ssh_), or wrap the poll in timeout 20.

[ -n "$now" ] && [ "$now" != "$boot" ] && break
sleep 5
done
if [ -z "$now" ] || [ "$now" = "$boot" ]; then
die "node B did not come back over ssh from its reboot within 10 minutes"

@github-actions github-actions Bot Oct 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — "within 10 minutes" counts only the sleep 5. Each attempt also runs ssh_, and ssh_ sets ConnectTimeout=10. While B is down, an attempt costs anywhere from about 3 s (no ARP reply, so EHOSTUNREACH) to the full 10 s. So 120 attempts take roughly 16 to 30 minutes, not 10. The loop does still terminate, so it can't hang. But the message understates how long the job waited, and it is easy to misread next to a step timeout. You could bound the loop by wall-clock time instead, e.g. deadline=$((SECONDS + 600)) and while [ "$SECONDS" -lt "$deadline" ]. Or the message could just say "after 120 tries".

fi
"${TNCTL[@]}" wait "ws://${IP[b]}" "$APPLIANCE_USER" "$password" --timeout 600 >/dev/null
b="$(ids b)"
[ "$a" != "$b" ] || die "node B still has A's ids after its reboot: $b"
Expand Down
Loading