Skip to content

NAS-144589 / 28.0.0-BETA.1 / truenas-appliance-runner: an HA mode, two appliances wired as an HA pair #23

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 1 commit into from
Oct 17, 2026
Merged

NAS-144589 / 28.0.0-BETA.1 / truenas-appliance-runner: an HA mode, two appliances wired as an HA pair #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 29 additions & 8 deletions .github/actions/truenas-appliance-runner/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,26 @@
# The TN_* variables are the e2e suite's contract, as the lab's claim
# action sets them; truenas-appliance (appliance.sh) is on PATH for
# snapshot, revert, probe, call and console.
#
# `ha: true` boots an HA pair instead (ha.sh): two appliances, from a pool
# that attaches the appliance disk twice,
#
# {"disks": ["truenas_appliance:nightly", "truenas_appliance:nightly"]}
#
# wired as middleware needs an HA pair's hardware, and handed over
# unconfigured: licensing, the VIP, a pool and failover are the suite's to
# set up. TN_HOST is node A, and TN_HA_* carry the addresses that setup
# needs. No baseline: a pair's shared disks cannot be snapshotted with it.
# truenas-appliance (ha.sh) is on PATH for call, ssh and console, each
# taking the node, a or b. Each node takes memory-mb and vcpus, and
# data-disk-count is the disks the two share.

inputs:
ha:
description: '`true` boots an HA pair instead of one appliance: see ha.sh.'
default: 'false'
baseline:
description: Snapshot taken once the appliance is up, to revert to between tests. Empty takes none.
description: Snapshot taken once the appliance is up, to revert to between tests. Empty takes none. Not taken for an HA pair.
default: fresh-install
memory-mb:
description: Appliance memory.
Expand Down Expand Up @@ -93,6 +109,7 @@
APPLIANCE_SSH: ${{ inputs.ssh }}
APPLIANCE_DISK_INPUT: ${{ inputs.disk }}
EXPORT_ENV: ${{ inputs.export-env }}
HA: ${{ inputs.ha }}
run: |
set -euo pipefail
lib="$GITHUB_ACTION_PATH"
Expand All @@ -106,15 +123,19 @@
# On PATH before it starts, so that a failed start's console is
# there for a later step to show.
echo "APPLIANCE_DIR=$APPLIANCE_DIR" >> "$GITHUB_ENV"
script=appliance.sh
[ "$HA" = true ] && script=ha.sh

Check notice on line 127 in .github/actions/truenas-appliance-runner/action.yml

View workflow job for this annotation

GitHub Actions / claude-review / Automatic PR review 

LOW: With ha: true the ssh and disk inputs are silently ignored; ssh: true yields empty outputs.ssh and no TN_SSH although ssh is enabled on both nodes.

@github-actions github-actions Bot Oct 16, 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 — With ha: true, the ssh and disk inputs are silently ignored. ha.sh never reads APPLIANCE_SSH or APPLIANCE_DISK. A caller passing ssh: true gets an empty outputs.ssh and no TN_SSH, even though ssh is in fact enabled on both nodes, at TN_HA_NODE_A:22 / TN_HA_NODE_B:22. This only affects the new opt-in path, but the header comment above could say so: ssh is always on for a pair and reached at the node addresses, and disk doesn't apply. Another option is for ha.sh to die when it is given APPLIANCE_DISK.

mkdir -p "$RUNNER_TEMP/truenas-appliance-bin"
ln -sf "$lib/appliance.sh" "$RUNNER_TEMP/truenas-appliance-bin/truenas-appliance"
ln -sf "$lib/$script" "$RUNNER_TEMP/truenas-appliance-bin/truenas-appliance"
echo "$RUNNER_TEMP/truenas-appliance-bin" >> "$GITHUB_PATH"
out="$("$lib/appliance.sh" start)"
# Folded: the boot as the appliance's console saw it, on the same
# clock as the milestones logged above.
echo "::group::The appliance's console during its boot (seconds since qemu started)"
cat "$APPLIANCE_DIR/console.ts.log" 2>/dev/null || echo "(no console)"
echo "::endgroup::"
out="$("$lib/$script" start)"
if [ "$HA" != true ]; then
# Folded: the boot as the appliance's console saw it, on the same
# clock as the milestones logged above.
echo "::group::The appliance's console during its boot (seconds since qemu started)"
cat "$APPLIANCE_DIR/console.ts.log" 2>/dev/null || echo "(no console)"
echo "::endgroup::"
fi
get() { sed -n "s/^$1=//p" <<<"$out"; }
{
echo "host=$(get TN_HOST)"
Expand All @@ -128,4 +149,4 @@
if [ "$EXPORT_ENV" = true ]; then
grep '^TN_' <<<"$out" >> "$GITHUB_ENV"
fi
echo "Appliance $(get TN_DOMAIN) at $(get TN_HOST): API up in $(get APPLIANCE_BOOT_SECONDS)s, baseline '${APPLIANCE_BASELINE:-none}' in $(get APPLIANCE_SNAPSHOT_SECONDS)s"

Check notice on line 152 in .github/actions/truenas-appliance-runner/action.yml

View workflow job for this annotation

GitHub Actions / claude-review / Automatic PR review 

LOW: For an HA pair the closing log line reports baseline 'fresh-install' with an empty duration although no baseline is taken.
Loading
Loading