From e058c9afd9020bbcfe327c403d38191a6b65b85f Mon Sep 17 00:00:00 2001
From: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Date: Tue, 15 Sep 2026 12:59:09 -0300
Subject: [PATCH] fix(xcat-core): otherpkgs does not log a failed rpm install
 or removal

A failed package install through the rpm or dpkg fallback leaves no
record in syslog. The same holds for the four package removal branches
that run after the install. The postscript sets its return code, but a
reader of the node log sees nothing, because the package manager output
goes to the console only when VERBOSE is set.

In xCAT/postscripts/otherpkgs the fallback install block, and the yum,
zypper, apt and rpm removal blocks, set RETURNVAL on a non-zero status
and stop there. Only the three repository install branches send a
message with local4.err.

This change adds the same local4.err message to the five branches that
have none. Each message names the command that ran, as the repository
install branches do.

xCAT-test/bats/postscripts_otherpkgs.bats drives each block with a
package manager that returns a failure. Two cases assert the message is
present, and both fail on the parent commit. Two more assert the
success path still logs "installed." and "removed." alone.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
---
 xCAT-test/bats/postscripts_otherpkgs.bats | 70 +++++++++++++++++++++++
 xCAT/postscripts/otherpkgs                |  5 ++
 2 files changed, 75 insertions(+)

diff --git a/xCAT-test/bats/postscripts_otherpkgs.bats b/xCAT-test/bats/postscripts_otherpkgs.bats
index 26495412e..dd0c2be05 100644
--- a/xCAT-test/bats/postscripts_otherpkgs.bats
+++ b/xCAT-test/bats/postscripts_otherpkgs.bats
@@ -358,3 +358,73 @@ run_pkglist_diagnostics()
     [[ "$output" == *'pkgsarray: foo bar, 2'* ]]
     [[ "$output" == *'yum/dnf: 1 (dnf), apt: 0, zypper: 0'* ]]
 }
+
+run_plain_install_block()
+{
+    local block
+    block="$(otherpkgs_block '#Handle the rest with rpm' 'if [ "$plain_pkgs" != "" -a -n "$OTHERPKGDIR" ]; then')" || return 99
+    local envlist="" VERBOSE= log_label=otherpkgs RETURNVAL=0 result=""
+    local supdatecommand=fake_pkg plain_pkgs="foo bar" mounted=1
+    local OTHERPKGDIR="$BATS_TEST_TMPDIR"
+    shadow_logger
+    shadow_pkg_manager
+    # The block changes directory. A subshell keeps the test in its own directory.
+    (
+        eval "$block"
+        printf 'RETURNVAL=%s\n' "$RETURNVAL"
+    )
+}
+
+run_plain_postremove_block()
+{
+    local block
+    block="$(otherpkgs_block '#remove more rpms if specified with' 'if [ "$plain_pkgs_postremove" != "" ]; then')" || return 99
+    local envlist="" VERBOSE= log_label=otherpkgs RETURNVAL=0 result=""
+    local sremovecommand=fake_pkg plain_pkgs_postremove="oldfoo"
+    shadow_logger
+    shadow_pkg_manager
+    eval "$block"
+    printf 'RETURNVAL=%s\n' "$RETURNVAL"
+}
+
+@test "a failed rpm fallback install is logged as failed" {
+    PKG_STATUS=1 run run_plain_install_block
+    [ "$status" -eq 0 ]
+    [[ "$output" == *'RETURNVAL=1'* ]]
+    refute_grep -q 'foo bar installed\.' "$LOGGER_LOG"
+    grep -q 'foo bar failed\.' "$LOGGER_LOG"
+}
+
+@test "a successful rpm fallback install is logged as installed" {
+    run run_plain_install_block
+    [ "$status" -eq 0 ]
+    [[ "$output" == *'RETURNVAL=0'* ]]
+    grep -q 'foo bar installed\.' "$LOGGER_LOG"
+    refute_grep -q 'failed\.' "$LOGGER_LOG"
+}
+
+@test "a failed package removal is logged as failed" {
+    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'* ]]
+        grep -q 'oldfoo failed\.' "$LOGGER_LOG"
+    done
+
+    : >"$LOGGER_LOG"
+    PKG_STATUS=1 run run_plain_postremove_block
+    [ "$status" -eq 0 ]
+    [[ "$output" == *'RETURNVAL=1'* ]]
+    refute_grep -q 'oldfoo removed\.' "$LOGGER_LOG"
+    grep -q 'oldfoo failed\.' "$LOGGER_LOG"
+}
+
+@test "a successful package removal is logged as removed only" {
+    run run_plain_postremove_block
+    [ "$status" -eq 0 ]
+    [[ "$output" == *'RETURNVAL=0'* ]]
+    grep -q 'oldfoo removed\.' "$LOGGER_LOG"
+    refute_grep -q 'failed\.' "$LOGGER_LOG"
+}
diff --git a/xCAT/postscripts/otherpkgs b/xCAT/postscripts/otherpkgs
index 70a90376a..e666dccb9 100755
--- a/xCAT/postscripts/otherpkgs
+++ b/xCAT/postscripts/otherpkgs
@@ -1071,6 +1071,7 @@ EOF`
         R=$?
         if [ $R -ne 0 ]; then
               RETURNVAL=$R
+              logger -p local4.err -t $log_label "$envlist $supdatecommand $plain_pkgs failed."
         else
               logger -p local4.info -t $log_label "$plain_pkgs installed."
         fi
@@ -1096,6 +1097,7 @@ EOF`
             R=$?
             if [ $R -ne 0 ]; then
               RETURNVAL=$R
+              logger -p local4.err -t $log_label "$envlist $yumcmd -y remove $repo_pkgs_postremove failed."
             else
               logger -p local4.info -t $log_label "$repo_pkgs_postremove removed."
             fi
@@ -1110,6 +1112,7 @@ EOF`
             R=$?
             if [ $R -ne 0 ]; then
               RETURNVAL=$R
+              logger -p local4.err -t $log_label "$envlist zypper remove -y $repo_pkgs_postremove failed."
             else
               logger -p local4.info -t $log_label "$repo_pkgs_postremove removed."
             fi
@@ -1125,6 +1128,7 @@ EOF`
             R=$?
             if [ $R -ne 0 ]; then
               RETURNVAL=$R
+              logger -p local4.err -t $log_label "$envlist apt-get -y remove $repo_pkgs_postremove failed."
             else
               logger -p local4.info  -t $log_label "$repo_pkgs_postremove removed."
             fi
@@ -1142,6 +1146,7 @@ EOF`
         R=$?
         if [ $R -ne 0 ]; then
               RETURNVAL=$R
+              logger -p local4.err -t $log_label "$envlist $sremovecommand $plain_pkgs_postremove failed."
         else
               logger -p local4.info -t $log_label "$plain_pkgs_postremove removed."
         fi