From 42746fae5ae850bcd0ed11362685bf460487f2bb Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:50:09 -0300 Subject: [PATCH] fix(xcat-core): otherpkgs logs a failed package install as installed In xCAT/postscripts/otherpkgs the success message runs after the branch that records a non-zero return, not inside an else. A node that fails to install its otherpkgs therefore reports both "... failed." and "... installed." to syslog, and a reader that greps for the success line sees a clean install. The same shape covers the four install sites and the four removal sites. Each success message now sits in the else branch of the status test. postscripts_otherpkgs.bats drives the install and postremove blocks with a package manager that fails, and uses refute_grep to require that no "installed."/"removed." message is sent. Against the unfixed script both tests find the success message. refute_grep is new in helpers/shell_source.bash, because bash ignores errexit for a command inverted with "!". Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/bats/helpers/shell_source.bash | 10 ++++ xCAT-test/bats/postscripts_otherpkgs.bats | 68 +++++++++++++++++++++++ xCAT/postscripts/otherpkgs | 38 ++++++++----- 3 files changed, 101 insertions(+), 15 deletions(-) diff --git a/xCAT-test/bats/helpers/shell_source.bash b/xCAT-test/bats/helpers/shell_source.bash index 61a0bea8d..4f4384fb2 100644 --- a/xCAT-test/bats/helpers/shell_source.bash +++ b/xCAT-test/bats/helpers/shell_source.bash @@ -130,3 +130,13 @@ extract_first_matching_line() } ' "$file" } + +# grep that fails when the pattern IS present. +# +# Do not write "! grep ..." for this. bash ignores errexit for a command inverted with "!", +# so such a line never fails a test unless it is the last line of one. +refute_grep() +{ + ! grep "$@" + return $? +} diff --git a/xCAT-test/bats/postscripts_otherpkgs.bats b/xCAT-test/bats/postscripts_otherpkgs.bats index 1a1d3a6f2..0883cacb7 100644 --- a/xCAT-test/bats/postscripts_otherpkgs.bats +++ b/xCAT-test/bats/postscripts_otherpkgs.bats @@ -153,3 +153,71 @@ run_plain_preremove_block() [ "$status" -eq 0 ] [ "$(logger_calls)" -eq 3 ] } + +run_install_block() +{ + local block + block="$(otherpkgs_block '#installation using yum/dnf or zypper' 'if [ "$repo_pkgs" != "" ]; then')" || return 99 + local hasyum=0 haszypper=0 hasapt=0 + eval "$1=1" + local envlist="" yumcmd=fake_pkg VERBOSE= log_label=otherpkgs RETURNVAL=0 REPOFILE=/dev/null result="" + local repo_pkgs="foo bar" + shadow_logger + shadow_pkg_manager + eval "$block" + printf 'RETURNVAL=%s\n' "$RETURNVAL" +} + +run_repo_postremove_block() +{ + local block + block="$(otherpkgs_block '#remove more rpms if specified with' 'if [ "$repo_pkgs_postremove" != "" ]; then')" || return 99 + local hasyum=0 haszypper=0 hasapt=0 + eval "$1=1" + local envlist="" yumcmd=fake_pkg VERBOSE= log_label=otherpkgs RETURNVAL=0 REPOFILE=/dev/null result="" + local repo_pkgs_postremove="oldfoo" + shadow_logger + shadow_pkg_manager + eval "$block" + printf 'RETURNVAL=%s\n' "$RETURNVAL" +} + +@test "a failed package install is not logged as installed" { + local manager + for manager in hasyum haszypper hasapt; do + : >"$LOGGER_LOG" + PKG_STATUS=1 run run_install_block "$manager" + [ "$status" -eq 0 ] + [[ "$output" == *'RETURNVAL=1'* ]] + refute_grep -q 'foo bar installed\.' "$LOGGER_LOG" + grep -q 'failed\.' "$LOGGER_LOG" + done +} + +@test "a successful package install is logged as installed" { + local manager + for manager in hasyum haszypper hasapt; do + : >"$LOGGER_LOG" + run run_install_block "$manager" + [ "$status" -eq 0 ] + [[ "$output" == *'RETURNVAL=0'* ]] + grep -q 'foo bar installed\.' "$LOGGER_LOG" + refute_grep -q 'failed\.' "$LOGGER_LOG" + done +} + +@test "a failed package removal is not logged as removed" { + local manager + for manager in hasyum haszypper hasapt; do + : >"$LOGGER_LOG" + PKG_STATUS=1 run run_repo_postremove_block "$manager" + [ "$status" -eq 0 ] + [[ "$output" == *'RETURNVAL=1'* ]] + refute_grep -q 'oldfoo removed\.' "$LOGGER_LOG" + done + + : >"$LOGGER_LOG" + run run_repo_postremove_block hasyum + [ "$status" -eq 0 ] + grep -q 'oldfoo removed\.' "$LOGGER_LOG" +} diff --git a/xCAT/postscripts/otherpkgs b/xCAT/postscripts/otherpkgs index 7e8549357..d3018eb7d 100755 --- a/xCAT/postscripts/otherpkgs +++ b/xCAT/postscripts/otherpkgs @@ -994,8 +994,9 @@ EOF` if [ $R -ne 0 ]; then RETURNVAL=$R logger -p local4.err -t $log_label "$envlist $yumcmd -y install $repo_pkgs failed." + else + logger -p local4.info -t $log_label "$repo_pkgs installed." fi - logger -p local4.info -t $log_label "$repo_pkgs installed." if [ $VERBOSE ]; then echo "$result" fi @@ -1008,8 +1009,9 @@ EOF` if [ $R -ne 0 ]; then RETURNVAL=$R logger -p local4.err -t $log_label "$envlist zypper install -y $repo_pkgs 2>&1 failed." - fi - logger -p local4.info -t $log_label "$repo_pkgs installed." + else + logger -p local4.info -t $log_label "$repo_pkgs installed." + fi if [ $VERBOSE ]; then echo "$result" fi @@ -1029,8 +1031,9 @@ EOF` if [ $R -ne 0 ]; then RETURNVAL=$R logger -p local4.err -t $log_label "install $repo_pkgs failed." - fi - logger -p local4.info -t $log_label "$repo_pkgs installed." + else + logger -p local4.info -t $log_label "$repo_pkgs installed." + fi if [ $VERBOSE ]; then echo "$result" fi @@ -1066,8 +1069,9 @@ EOF` R=$? if [ $R -ne 0 ]; then RETURNVAL=$R - fi - logger -p local4.info -t $log_label "$plain_pkgs installed." + else + logger -p local4.info -t $log_label "$plain_pkgs installed." + fi if [ $VERBOSE ]; then echo "$result" fi @@ -1090,8 +1094,9 @@ EOF` R=$? if [ $R -ne 0 ]; then RETURNVAL=$R - fi - logger -p local4.info -t $log_label "$repo_pkgs_postremove removed." + else + logger -p local4.info -t $log_label "$repo_pkgs_postremove removed." + fi if [ $VERBOSE ]; then echo "$result" fi @@ -1103,8 +1108,9 @@ EOF` R=$? if [ $R -ne 0 ]; then RETURNVAL=$R - fi - logger -p local4.info -t $log_label "$repo_pkgs_postremove removed." + else + logger -p local4.info -t $log_label "$repo_pkgs_postremove removed." + fi if [ $VERBOSE ]; then echo "$result" fi @@ -1117,8 +1123,9 @@ EOF` R=$? if [ $R -ne 0 ]; then RETURNVAL=$R - fi - logger -p local4.info -t $log_label "$repo_pkgs_postremove removed." + else + logger -p local4.info -t $log_label "$repo_pkgs_postremove removed." + fi if [ $VERBOSE ]; then echo "$result" fi @@ -1133,8 +1140,9 @@ EOF` R=$? if [ $R -ne 0 ]; then RETURNVAL=$R - fi - logger -p local4.info -t $log_label "$plain_pkgs_postremove removed." + else + logger -p local4.info -t $log_label "$plain_pkgs_postremove removed." + fi if [ $VERBOSE ]; then echo "$result" fi