Let the upgrade tool find its own checks, and tell "cannot" from "failed"
Both fixes come from the sister project, where they were found on a real runner: `sudo ./bin/dejima-ci-upgrade mark-node 2` answered "cannot check - dejima-ci-upgrade check-lan is not installed here". The same two bugs were latent here, since every verify-node command names a sibling in bin/. VERIFY COMMANDS ARE NOW LOOKED UP ON $PATH FIRST, THEN BESIDE THIS PROGRAM. The notes tell an operator to run from a checkout on a box where nothing is installed yet, and a PATH-only lookup fails there on the very program doing the looking. $PATH still wins where both exist, because the installed copy is what the machine runs. AND EXIT 2 NOW MEANS "COULD NOT DETERMINE" RATHER THAN "NOT IN FORCE". Collapsing the two would have mark-node tell an operator their console work is missing on the strength of a check that never ran - wrong in the alarming direction, about the one thing they cannot see for themselves. reach and drift return 2 on INDETERMINATE to honour it, which is a contract rather than a detail and is now written down in three places. status also stops reporting console work as outstanding when the upgrade declares a check and that check passes. A snapshot restore takes the record and leaves the rules on the node, and reporting that as work to do sends an operator to redo something already done. One test had to change rather than being fixed: "a missing verify command is not reported as a failed check" relied on dejima-devbox-audit being unfindable, which the sibling fallback now makes false. It runs from a lonely copy instead, which is the condition it was always trying to describe. `bats test/` 155 passing, 4 new.
This commit is contained in:
@@ -358,7 +358,12 @@ cmd_reach() {
|
||||
say "INDETERMINATE: nothing answered, including the forge."
|
||||
say " A working firewall and a broken network look the same from here."
|
||||
say " Fix connectivity and run again. This is not a pass."
|
||||
rc=1
|
||||
# 2, not 1, and the difference is a contract rather than a detail.
|
||||
# dejima-devbox-upgrade reads a verify-node command's exit status as
|
||||
# 0 in force / 2 could not determine / anything else NOT in force.
|
||||
# Returning 1 here would have it record "your console work is
|
||||
# missing" on the strength of a check that could not see.
|
||||
rc=2
|
||||
;;
|
||||
esac
|
||||
|
||||
@@ -390,8 +395,8 @@ cmd_reach() {
|
||||
say " installs it; a host set up before it did will not have it."
|
||||
if [ "$verdict" = PASS ]; then
|
||||
verdict=INDETERMINATE
|
||||
rc=2
|
||||
fi
|
||||
rc=1
|
||||
;;
|
||||
esac
|
||||
|
||||
@@ -699,7 +704,7 @@ cmd_drift() {
|
||||
sed 's/^/ /' "$DRIFT_ERRS" | head -3 >&2
|
||||
printf 'INDETERMINATE\n'
|
||||
rm -rf "$work"
|
||||
return 1
|
||||
return 2
|
||||
fi
|
||||
|
||||
if [ "$accept" = 1 ]; then
|
||||
@@ -733,7 +738,7 @@ cmd_drift() {
|
||||
say " $PROG drift --accept"
|
||||
printf 'INDETERMINATE\n'
|
||||
rm -rf "$work"
|
||||
return 1
|
||||
return 2
|
||||
fi
|
||||
|
||||
[ -n "$missing" ] && {
|
||||
|
||||
+67
-15
@@ -118,25 +118,57 @@ state_set() { # state_set KEY VALUE
|
||||
|
||||
# Run an upgrade's verify-node command, if it has one.
|
||||
#
|
||||
# ok - the command passed, so the console work is in force
|
||||
# failed - it did not, whatever the state file says
|
||||
# none - the upgrade declares no check
|
||||
# missing - it declares one and the command is not installed here
|
||||
# Where a verify-node command's program actually is.
|
||||
#
|
||||
# `missing` is kept apart from `failed` on purpose. A check that could not run
|
||||
# has not found anything, and reporting that as a failure would train an
|
||||
# operator to ignore the word.
|
||||
# $PATH first, then beside this program. The fallback is the point: the upgrade
|
||||
# notes tell an operator to run this from a checkout, and a PATH-only lookup
|
||||
# fails on a box where nothing is installed yet - which is every box at the
|
||||
# moment it is being upgraded. The sister project shipped that bug and it found
|
||||
# a real runner.
|
||||
#
|
||||
# $PATH still wins where both exist, because the installed copy is the one this
|
||||
# machine actually runs.
|
||||
verify_cmd_path() { # verify_cmd_path NAME
|
||||
local self_dir
|
||||
command -v "$1" 2>/dev/null && return 0
|
||||
self_dir=$(cd "$(dirname "$(readlink -f "$0")")" && pwd) || return 1
|
||||
[ -x "$self_dir/$1" ] || return 1
|
||||
printf '%s\n' "$self_dir/$1"
|
||||
}
|
||||
|
||||
# THE EXIT-CODE CONTRACT, which is three states and not two:
|
||||
#
|
||||
# 0 the console work is in force
|
||||
# 2 could not be determined - the check did not run, or ran and could
|
||||
# not tell
|
||||
# other it is NOT in force, and the machine says so
|
||||
#
|
||||
# ok - in force
|
||||
# failed - not in force, whatever the state file says
|
||||
# unknown - it ran and could not tell
|
||||
# none - the upgrade declares no check
|
||||
# missing - it declares one and the command is nowhere to be found
|
||||
#
|
||||
# `unknown` and `missing` are both kept apart from `failed` on purpose. A check
|
||||
# that could not run has not found anything, and reporting that as a failure
|
||||
# would tell an operator their console work is missing on the strength of a
|
||||
# check that never ran.
|
||||
verify_node() { # verify_node VERSION
|
||||
local note cmd
|
||||
local note cmd prog rest path rc=0
|
||||
note=$(note_for "$1") || { printf 'none\n'; return 0; }
|
||||
cmd=$(note_field "$note" verify-node)
|
||||
[ -n "$cmd" ] || { printf 'none\n'; return 0; }
|
||||
command -v "${cmd%% *}" >/dev/null 2>&1 || { printf 'missing\n'; return 0; }
|
||||
if $cmd >/dev/null 2>&1; then
|
||||
printf 'ok\n'
|
||||
else
|
||||
printf 'failed\n'
|
||||
fi
|
||||
prog=${cmd%% *}
|
||||
rest=${cmd#"$prog"}
|
||||
path=$(verify_cmd_path "$prog") || { printf 'missing\n'; return 0; }
|
||||
# Unquoted on purpose: $rest carries the command's arguments.
|
||||
# shellcheck disable=SC2086
|
||||
"$path" $rest >/dev/null 2>&1 || rc=$?
|
||||
case $rc in
|
||||
0) printf 'ok\n' ;;
|
||||
2) printf 'unknown\n' ;;
|
||||
*) printf 'failed\n' ;;
|
||||
esac
|
||||
}
|
||||
|
||||
# --- status -----------------------------------------------------------------
|
||||
@@ -225,7 +257,17 @@ cmd_status() {
|
||||
outstanding=1
|
||||
rc=1
|
||||
say " v$v $title"
|
||||
[ "$need_node" = 1 ] && say " node: console work outstanding"
|
||||
if [ "$need_node" = 1 ]; then
|
||||
# Restoring a snapshot older than the console work leaves the rules
|
||||
# on the node and takes the record with the guest. Reporting that as
|
||||
# work to do sends an operator to redo something already done.
|
||||
if [ "$VERIFY" = 1 ] && [ "$(verify_node "$v")" = ok ]; then
|
||||
say " node: NOT RECORDED, but its check passes - the work"
|
||||
say " looks done. Re-record with: $PROG mark-node $v"
|
||||
else
|
||||
say " node: console work outstanding"
|
||||
fi
|
||||
fi
|
||||
[ "$need_guest" = 1 ] && say " guest: re-run dejima-devbox-bootstrap from a v$v checkout"
|
||||
[ "$(note_field "$note" snapshot)" = required ] \
|
||||
&& say " snapshot: REQUIRED after this one"
|
||||
@@ -248,6 +290,9 @@ cmd_status() {
|
||||
say " v$v FAILED. The check for this upgrade does not pass, so its"
|
||||
say " console work is not in force whatever the state file says."
|
||||
say " $PROG show $v" ;;
|
||||
unknown) any=1
|
||||
say " v$v could not tell. $(note_field "$(note_for "$v")" verify-node) ran and"
|
||||
say " could not determine the answer, which is not a failure." ;;
|
||||
missing) any=1
|
||||
say " v$v could not check - $(note_field "$(note_for "$v")" verify-node) is not installed here." ;;
|
||||
esac
|
||||
@@ -318,6 +363,13 @@ cmd_mark_node() {
|
||||
say " Recording as unverified rather than refusing, because a missing"
|
||||
say " program is not evidence about your firewall."
|
||||
;;
|
||||
unknown)
|
||||
say "$PROG: $(note_field "$note" verify-node) could not tell."
|
||||
say " It ran and reported that it could not determine the answer,"
|
||||
say " which is not the same as reporting the work is missing."
|
||||
say " Recording as unverified. Run it yourself to see what it needs:"
|
||||
say " $(note_field "$note" verify-node)"
|
||||
;;
|
||||
none)
|
||||
say "$PROG: v$v declares no check, so this is your word for it."
|
||||
;;
|
||||
|
||||
@@ -69,6 +69,16 @@ Two consequences worth stating plainly:
|
||||
- `mark-node` **runs the check first and refuses to record a claim the machine
|
||||
contradicts.** `--force` overrides it and records the result as unverified, so
|
||||
`status` keeps saying so rather than forgetting.
|
||||
- **A check that cannot tell is not a check that failed.** The contract is exit
|
||||
`0` for in force, `2` for could-not-determine, anything else for not in force.
|
||||
`reach` and `drift` both exit `2` on `INDETERMINATE`, and `mark-node` records
|
||||
the assertion as unverified rather than refusing it — telling an operator
|
||||
their console work is missing on the strength of a check that could not see
|
||||
would be wrong in the alarming direction.
|
||||
- **The command is looked up on `$PATH` first, then beside the tool.** The
|
||||
fallback is what makes a checkout work on a box where nothing is installed
|
||||
yet. The sister project shipped without it and the tool could not find
|
||||
itself.
|
||||
- `status` re-runs the checks for everything you have already recorded. An
|
||||
assertion that was true in June and is not true now is a rule somebody removed,
|
||||
and that is worth finding out from a tool rather than from an incident.
|
||||
|
||||
+7
-2
@@ -118,10 +118,15 @@ setup() {
|
||||
[[ "$output" == *FAIL* ]]
|
||||
}
|
||||
|
||||
@test "reach exits non-zero on INDETERMINATE, which must not read as a pass" {
|
||||
@test "reach exits 2 on INDETERMINATE, which must not read as a pass" {
|
||||
# 2 specifically, not merely non-zero. dejima-devbox-upgrade reads a
|
||||
# verify-node command's status as 0 in force / 2 could not determine /
|
||||
# anything else NOT in force, so returning 1 here would have it record
|
||||
# "your console work is missing" on the strength of a check that could not
|
||||
# see. Same verdict, different meaning, and only one of them is true.
|
||||
gateway_verdict() { printf 'INDETERMINATE\n'; }
|
||||
run cmd_reach
|
||||
[ "$status" -ne 0 ]
|
||||
[ "$status" -eq 2 ]
|
||||
[[ "$output" == *INDETERMINATE* ]]
|
||||
}
|
||||
|
||||
|
||||
+3
-1
@@ -46,7 +46,9 @@ stub_state() {
|
||||
# changed, and only one of those is safe to report.
|
||||
stub_state "FILE aaa /etc/one"
|
||||
run cmd_drift
|
||||
[ "$status" -ne 0 ]
|
||||
# 2, not merely non-zero: the upgrade tool reads this as "could not
|
||||
# determine" rather than "not in force".
|
||||
[ "$status" -eq 2 ]
|
||||
[[ "$output" == *INDETERMINATE* ]]
|
||||
[[ "$output" == *"no baseline"* ]]
|
||||
}
|
||||
|
||||
+47
-1
@@ -151,13 +151,59 @@ stub_audit() {
|
||||
@test "a missing verify command is not reported as a failed check" {
|
||||
# A check that could not run has not found anything. Calling that a failure
|
||||
# trains an operator to ignore the word.
|
||||
#
|
||||
# The tool has to be genuinely alone for this: run from the repository it
|
||||
# finds dejima-devbox-audit beside itself and uses it, which is the whole
|
||||
# point of the fallback and the reason this test cannot just rely on $PATH.
|
||||
lonely="$BATS_TEST_TMPDIR/alone"
|
||||
mkdir -p "$lonely/bin"
|
||||
cp "$TOOL" "$lonely/bin/dejima-devbox-upgrade"
|
||||
ln -s "$REPO/upgrades" "$lonely/upgrades"
|
||||
write_state guest_version=2
|
||||
run_tool mark-node 2
|
||||
CONF_DIR="$STATE_DIR" run "$lonely/bin/dejima-devbox-upgrade" mark-node 2
|
||||
[ "$status" -eq 0 ]
|
||||
[[ "$output" == *"cannot check"* ]]
|
||||
grep -q '^node_verified=no$' "$STATE_DIR/state"
|
||||
}
|
||||
|
||||
@test "the verify command is found beside the tool when it is not on PATH" {
|
||||
# The sister project shipped the inverse of this and it found a real
|
||||
# runner: the notes say to run from a checkout, and a PATH-only lookup
|
||||
# could not find the program doing the looking.
|
||||
write_state guest_version=2
|
||||
stub_audit 0
|
||||
# No stub on PATH this time - the sibling in bin/ must be what answers.
|
||||
PATH="$BATS_TEST_TMPDIR/empty:$PATH" CONF_DIR="$STATE_DIR" run "$TOOL" mark-node 2
|
||||
[[ "$output" != *"is not installed here"* ]]
|
||||
}
|
||||
|
||||
@test "a check that could not tell is recorded, not refused" {
|
||||
# Exit 2 means "could not determine". Collapsing it into "not in force"
|
||||
# would tell an operator their console work is missing on the strength of
|
||||
# a check that never ran.
|
||||
stub_audit 2
|
||||
write_state guest_version=2
|
||||
run_tool mark-node 2
|
||||
[ "$status" -eq 0 ]
|
||||
[[ "$output" == *"could not tell"* ]]
|
||||
[[ "$output" != *"refusing to record"* ]]
|
||||
grep -q '^node_verified=no$' "$STATE_DIR/state"
|
||||
}
|
||||
|
||||
@test "an unrecorded upgrade whose check passes is not reported as work to do" {
|
||||
# Restoring a snapshot older than the console work leaves the rules on the
|
||||
# node and takes the record with the guest. Reporting that as work to do
|
||||
# sends an operator to redo something already done.
|
||||
stub_audit 0
|
||||
write_state guest_version=2
|
||||
run_tool status
|
||||
# v2 declares a check and it passes, so it is not work to do.
|
||||
[[ "$output" == *"NOT RECORDED, but its check passes"* ]]
|
||||
# v3 declares no check, so it stays plainly outstanding. Both branches in
|
||||
# one run, which is also why this cannot assert the absence of that phrase.
|
||||
[[ "$output" == *"console work outstanding"* ]]
|
||||
}
|
||||
|
||||
@test "status contradicts a recorded claim the machine no longer supports" {
|
||||
# An assertion true in June and false now is a rule somebody removed. Better
|
||||
# found by a timer than by an incident.
|
||||
|
||||
Reference in New Issue
Block a user