[crmsh-4.6] Fix: report: crm report will hang if CIB contains invalid…

… configuraions (bsc#1229686) (#1518)

## Problem
Command `crm report` will hang if CIB contains invalid configurations,
like lack of `stonith-enabled=false` when there is no stonith device
configured.
## Root cause
When running `crm report`, crmsh tries to collect the output of
`crm_verify`, by calling `get_stdout_or_raise_error`.
`crm_verify` will return non 0 value, then `get_stdout_or_raise_error`
will raise custom exception `CommandFailure`, which will be wrongly
pickled between processes.
## Solution
1. Implementing the `__reduce__` method to provide custom pickling
    support. This method returns a tuple specifying how to reconstruct
instances of CommandFailure during the unpickling process, ensuring all
necessary arguments (`cmd`, `host`, `user`, `msg`) are correctly passed
    to the `__init__` method
2. When there is invalid configuration in cib as crm_verify complains,
the
    error output should be recorded in the report result instead of just
    raising an exception

crmsh/report/collect.py

@@ -375,8 +375,9 @@ def consume_cib_in_workdir(workdir: str) -> None:
         crmutils.str2file(out, os.path.join(workdir, constants.CONFIGURE_SHOW_F))
 
         cmd = f"crm_verify -V -x {cib_in_workdir}"
-        out = cluster_shell_inst.get_stdout_or_raise_error(cmd)
-        crmutils.str2file(out, os.path.join(workdir, constants.CRM_VERIFY_F))
+        _, _, err = cluster_shell_inst.get_rc_stdout_stderr_without_input(None, cmd)
+        if err:
+            crmutils.str2file(err, os.path.join(workdir, constants.CRM_VERIFY_F))
 
 
 def collect_config(context: core.Context) -> None:

crmsh/sh.py

@@ -87,6 +87,14 @@ def __init__(self, cmd: str, host: typing.Optional[str], user: typing.Optional[s
             super().__init__("Failed to run command as {}@{}: '{}': {}".format(user, host, cmd, msg), cmd)
         self.host = host
         self.user = user
+        self.cmd = cmd
+        self.msg = msg
+
+    def __reduce__(self):
+        '''
+        Return a tuple that Python will use for pickling this object
+        '''
+        return CommandFailure, (self.cmd, self.host, self.user, self.msg)
 
 
 class Utils:

test/unittests/test_report_collect.py

@@ -334,16 +334,17 @@ def test_consume_cib_in_workdir(self, mock_isfile, mock_run, mock_str2file):
         mock_isfile.return_value = True
         mock_run_inst = mock.Mock()
         mock_run.return_value = mock_run_inst
-        mock_run_inst.get_stdout_or_raise_error.side_effect = ["data1", "data2"]
+        mock_run_inst.get_stdout_or_raise_error.return_value = "data1"
+        mock_run_inst.get_rc_stdout_stderr_without_input.return_value = (0, "data2", "error")
         collect.consume_cib_in_workdir("/workdir")
         mock_isfile.assert_called_once_with(f"/workdir/{constants.CIB_F}")
-        mock_run_inst.get_stdout_or_raise_error.assert_has_calls([
-            mock.call('CIB_file=/workdir/cib.xml crm configure show'),
-            mock.call('crm_verify -V -x /workdir/cib.xml')
-        ])
+        cmd1 = "CIB_file=/workdir/cib.xml crm configure show"
+        mock_run_inst.get_stdout_or_raise_error.assert_called_once_with(cmd1)
+        cmd2 = f"crm_verify -V -x /workdir/cib.xml"
+        mock_run_inst.get_rc_stdout_stderr_without_input.assert_called_once_with(None, cmd2)
         mock_str2file.assert_has_calls([
             mock.call("data1", f"/workdir/{constants.CONFIGURE_SHOW_F}"),
-            mock.call("data2", f"/workdir/{constants.CRM_VERIFY_F}")
+            mock.call("error", f"/workdir/{constants.CRM_VERIFY_F}")
         ])
 
     @mock.patch('crmsh.report.utils.real_path')

test/unittests/test_sh.py

@@ -1,5 +1,6 @@
 import subprocess
 import unittest
+import pickle
 from unittest import mock
 
 import crmsh.sh
@@ -193,3 +194,33 @@ def test_subprocess_run_without_input_with_input_kwargs(self):
                 stdin=subprocess.PIPE,
             )
         self.cluster_shell.local_shell.su_subprocess_run.assert_not_called()
+
+
+class TestCommandFailurePickling(unittest.TestCase):
+    def test_pickling_unpickling(self):
+        # Create an instance of CommandFailure
+        original = crmsh.sh.CommandFailure(cmd="ls", host="localhost", user="root", msg="Permission denied")
+        # Pickle the object
+        pickled = pickle.dumps(original)
+        # Unpickle the object
+        unpickled = pickle.loads(pickled)
+
+        # Assert that the unpickled object retains the same attributes as the original
+        self.assertEqual(original.cmd, unpickled.cmd)
+        self.assertEqual(original.host, unpickled.host)
+        self.assertEqual(original.user, unpickled.user)
+        self.assertEqual(original.msg, unpickled.msg)
+
+    def test_pickling_unpickling_with_none_values(self):
+        # Create an instance of CommandFailure with None values for optional parameters
+        original = crmsh.sh.CommandFailure(cmd="ls", host=None, user=None, msg="No such file or directory")
+        # Pickle the object
+        pickled = pickle.dumps(original)
+        # Unpickle the object
+        unpickled = pickle.loads(pickled)
+
+        # Assert that the unpickled object retains the same attributes, including None values
+        self.assertEqual(original.cmd, unpickled.cmd)
+        self.assertEqual(original.host, unpickled.host)
+        self.assertEqual(original.user, unpickled.user)
+        self.assertEqual(original.msg, unpickled.msg)