Found while fixing #400, and deliberately left out of that PR: it is the same
class of bug, one step upstream, and closing it moves the rollback boundary
again.
Summary
Both update templates open with
mtui_patches=$(zypper -n patches | awk -F "|" '/$repa\>/ { print $2; }')
The command substitution's own status is discarded — in POSIX the assignment's
status is the substitution's, and nothing reads it. So when the pipeline
fails, the patch list comes back empty, the patch is skipped, mtui_status
stays 0, and mtui reports a successful update having installed nothing.
Failure modes that reach it:
zypper -n refresh failed earlier in the script (its status is likewise
discarded — no set -e);
- the update repo was never added, or was added under a different alias;
zypper -n patches itself fails — exit 6 (no repositories defined) or 7
(ZYpp library lock), among others.
In each case the operator is told the update installed cleanly.
Why it is not #400
#400 was "the patch ran and failed, and we discarded its status". This is "the
patch never ran, and we cannot tell that apart from having nothing to run". The
fix for #400 made the second case explicit — mtui_status=0 on the skip path
— where it used to be an accident, which is what makes it worth stating now.
Note the skip path is also legitimate: a host carrying none of the update's
products genuinely has no patches, and must keep passing. So this is not a
matter of failing on an empty list; it needs the substitution's own failure
distinguished from a genuinely empty result.
Suggested direction
Capture the substitution's status and route it separately from the patch's, e.g.
if ! mtui_patches=$(zypper -n patches | awk ...); then
mtui_status=<sentinel>
fi
with the sentinel classified as its own verdict rather than reusing "package not
found" or "Unknown Error" — the operator needs "could not determine what to
patch", which is a different action from "the patch failed".
Two things to weigh before implementing:
- The blast radius. A new failure class on this path fires the group-wide
rollback downgrade, which reverts every host in the group. Whether "could not
list patches" should roll back at all is a real question — arguably it should
fail the host without reverting peers, like the -1 never-ran gate does.
- A pipeline's status is the last stage's, so
awk's success would mask a
failing zypper. set -o pipefail is not POSIX; ${PIPESTATUS[0]} is bash.
The portable form needs thought, and the templates currently target plain
/bin/sh.
Related
The same shape exists in downgrade.rs's LIST_COMMAND, whose consumer already
documents that the pipeline's status is awk's and therefore only ever means "the
probe itself broke" (reports/update_flow.rs). Worth fixing together.
Found while fixing #400, and deliberately left out of that PR: it is the same
class of bug, one step upstream, and closing it moves the rollback boundary
again.
Summary
Both update templates open with
mtui_patches=$(zypper -n patches | awk -F "|" '/$repa\>/ { print $2; }')The command substitution's own status is discarded — in POSIX the assignment's
status is the substitution's, and nothing reads it. So when the pipeline
fails, the patch list comes back empty, the patch is skipped,
mtui_statusstays
0, and mtui reports a successful update having installed nothing.Failure modes that reach it:
zypper -n refreshfailed earlier in the script (its status is likewisediscarded — no
set -e);zypper -n patchesitself fails — exit6(no repositories defined) or7(ZYpp library lock), among others.
In each case the operator is told the update installed cleanly.
Why it is not #400
#400 was "the patch ran and failed, and we discarded its status". This is "the
patch never ran, and we cannot tell that apart from having nothing to run". The
fix for #400 made the second case explicit —
mtui_status=0on the skip path— where it used to be an accident, which is what makes it worth stating now.
Note the skip path is also legitimate: a host carrying none of the update's
products genuinely has no patches, and must keep passing. So this is not a
matter of failing on an empty list; it needs the substitution's own failure
distinguished from a genuinely empty result.
Suggested direction
Capture the substitution's status and route it separately from the patch's, e.g.
with the sentinel classified as its own verdict rather than reusing "package not
found" or "Unknown Error" — the operator needs "could not determine what to
patch", which is a different action from "the patch failed".
Two things to weigh before implementing:
rollback downgrade, which reverts every host in the group. Whether "could not
list patches" should roll back at all is a real question — arguably it should
fail the host without reverting peers, like the
-1never-ran gate does.awk's success would mask afailing
zypper.set -o pipefailis not POSIX;${PIPESTATUS[0]}is bash.The portable form needs thought, and the templates currently target plain
/bin/sh.Related
The same shape exists in
downgrade.rs'sLIST_COMMAND, whose consumer alreadydocuments that the pipeline's status is awk's and therefore only ever means "the
probe itself broke" (
reports/update_flow.rs). Worth fixing together.