Repository: ClusterLabs/crmsh

Issue:
No need to add 'devices=' parameter for fence_sbd
After https://github.com/ClusterLabs/fence-agents/pull/591 was merged

Discussion:

Solution patch:
diff --git a/crmsh/bootstrap.py b/crmsh/bootstrap.py
index d83a1a67..441aab1f 100644
--- a/crmsh/bootstrap.py
+++ b/crmsh/bootstrap.py
@@ -238,6 +238,8 @@ class Context(object):
         """
         Validate -N/--nodes option
         """
+        if self.user_at_node_list and not self.yes_to_all:
+            utils.fatal("Can't use -N/--nodes option without -y/--yes option")
         if self.user_at_node_list and self.stage:
             utils.fatal("Can't use -N/--nodes option and stage({}) together".format(self.stage))
         me = utils.this_node()
diff --git a/crmsh/report/collect.py b/crmsh/report/collect.py
index f01245b8..002a8eea 100644
--- a/crmsh/report/collect.py
+++ b/crmsh/report/collect.py
@@ -536,10 +536,13 @@ def collect_qdevice_info(context: core.Context) -> None:
     """
     Collect quorum/qdevice/qnetd information
     """
+    service_manager = ServiceManager()
+    if not service_manager.service_is_active("corosync.service"):
+        return
     out_string = "##### Quorum status #####\n"
     out_string += corosync.query_quorum_status() + "\n"
 
-    if ServiceManager().service_is_active("corosync-qdevice.service"):
+    if service_manager.service_is_active("corosync-qdevice.service"):
         out_string += "\n##### Qdevice status #####\n"
         out_string += corosync.query_qdevice_status() + "\n"
         out_string += "\n##### Qnetd status #####\n"
diff --git a/crmsh/sbd.py b/crmsh/sbd.py
index a702fc99..88c67c34 100644
--- a/crmsh/sbd.py
+++ b/crmsh/sbd.py
@@ -559,9 +559,7 @@ class SBDManager:
             if utils.get_property("stonith-watchdog-timeout", get_default=False):
                 utils.delete_property("stonith-watchdog-timeout")
             if not xmlutil.CrmMonXmlParser().is_resource_configured(self.SBD_RA):
-                all_device_list = SBDUtils.get_sbd_device_from_config()
-                devices_param_str = f"params devices=\"{','.join(all_device_list)}\""
-                cmd = f"crm configure primitive {self.SBD_RA_ID} {self.SBD_RA} {devices_param_str}"
+                cmd = f"crm configure primitive {self.SBD_RA_ID} {self.SBD_RA}"
                 sh.cluster_shell().get_stdout_or_raise_error(cmd)
         utils.set_property("stonith-enabled", "true")
 
diff --git a/crmsh/ui_context.py b/crmsh/ui_context.py
index 56088d4e..c8479648 100644
--- a/crmsh/ui_context.py
+++ b/crmsh/ui_context.py
@@ -114,8 +114,6 @@ class Context(object):
         A space at the end of the line is significant.
         '''
         complete_next = line.endswith(' ')
-        # if complete_next:
-        #    print >>sys.stderr, "complete_next is on"
 
         # copy current state
         prev_stack = list(self.stack)
@@ -137,7 +135,7 @@ class Context(object):
                     self.command_args = tokens
                     self.command_info = self.current_level().get_child(token)
 
-                    if not self.command_info:
+                    if not self.command_args and not self.command_info:
                         return self.current_level().get_completions()
                     if self.command_info.type == 'level':
                         self.enter_level(self.command_info.level)
@@ -145,7 +143,12 @@ class Context(object):
                         # use the completer for the command
                         ret = self.command_info.complete(self, tokens)
                         if tokens:
-                            ret = [t for t in ret if t.startswith(tokens[-1])]
+                            last_token = tokens[-1]
+                            if last_token:
+                                ret = [t for t in ret if t.startswith(last_token)]
+                            elif len(tokens) > 1:
+                                # don't complete for the unknown token
+                                ret = [t for t in ret if t in tokens[:-1]]
 
                         if not ret or self.command_info.aliases:
                             if not token in self.current_level().get_completions():


Test patch:
diff --git a/test/unittests/test_report_collect.py b/test/unittests/test_report_collect.py
index 8d789be2..3575f820 100644
--- a/test/unittests/test_report_collect.py
+++ b/test/unittests/test_report_collect.py
@@ -511,24 +511,36 @@ id            0x19041a12
         collect.collect_corosync_blackbox(mock_ctx_inst)
         mock_debug.assert_called_once_with(f"Dump corosync blackbox info into {constants.COROSYNC_RECORDER_F}")
 
+    @mock.patch('crmsh.corosync.query_quorum_status')
+    @mock.patch('crmsh.report.collect.ServiceManager')
+    def test_collect_qdevice_info_return(self, mock_service, mock_quorum):
+        mock_ctx_inst = mock.Mock(work_dir="/opt/workdir")
+        mock_service_inst = mock.Mock()
+        mock_service.return_value = mock_service_inst
+        mock_service_inst.service_is_active.return_value = False
+        collect.collect_qdevice_info(mock_ctx_inst)
+        mock_quorum.assert_not_called()
+
     @mock.patch('logging.Logger.debug')
     @mock.patch('crmsh.report.utils.real_path')
     @mock.patch('crmsh.utils.str2file')
+    @mock.patch('os.path.join')
     @mock.patch('crmsh.corosync.query_qnetd_status')
     @mock.patch('crmsh.corosync.query_qdevice_status')
-    @mock.patch('crmsh.service_manager.ServiceManager')
     @mock.patch('crmsh.corosync.query_quorum_status')
-    def test_collect_qdevice_info(self, mock_quorum, mock_service, mock_qdevice, mock_qnetd, mock_str2file, mock_real_path, mock_debug):
+    @mock.patch('crmsh.report.collect.ServiceManager')
+    def test_collect_qdevice_info(self, mock_service, mock_quorum, mock_qdevice, mock_qnetd, mock_join, mock_str2file, mock_real_path, mock_debug):
         mock_ctx_inst = mock.Mock(work_dir="/opt/workdir")
         mock_service_inst = mock.Mock()
         mock_service.return_value = mock_service_inst
-        mock_service_inst.service_is_active.return_value = True
-        mock_quorum.return_value = "quorum_data"
+        mock_service_inst.service_is_active.side_effect = [True, True]
         mock_qdevice.return_value = "qdevice_data"
         mock_qnetd.return_value = "qnetd_data"
+        mock_quorum.return_value = "quorum_data"
+        mock_join.return_value = "/opt/workdir/qdevice.txt"
         mock_real_path.return_value = "/opt/workdir/qdevice.txt"
         collect.collect_qdevice_info(mock_ctx_inst)
-        mock_debug.assert_called_once_with(f"Dump quorum/qdevice/qnetd information into {mock_real_path.return_value}")
+        mock_debug.assert_called_once_with('Dump quorum/qdevice/qnetd information into /opt/workdir/qdevice.txt')
 
     @mock.patch("logging.Logger.error")
     @mock.patch('crmsh.utils.str2file')
diff --git a/test/unittests/test_report_utils.py b/test/unittests/test_report_utils.py
index bed3064e..f6a9fa19 100644
--- a/test/unittests/test_report_utils.py
+++ b/test/unittests/test_report_utils.py
@@ -791,12 +791,6 @@ pacemaker-schedulerd[5677]:  error: Resource"""
         middle_timestamp = utils.get_timestamp(data_list[1], pacemaker_file_path)
         last_timestamp = utils.get_timestamp(data_list[2], pacemaker_file_path)
         assert first_timestamp < middle_timestamp < last_timestamp
-        line_stamp = crmutils.parse_to_timestamp("Jan 03 11:03:41 2024")
-        result_line = utils.findln_by_timestamp(data, line_stamp, pacemaker_file_path)
-        assert result_line == 2
-        line_stamp = crmutils.parse_to_timestamp("Jan 03 12:03:41 2024")
-        result_line = utils.findln_by_timestamp(data, line_stamp, pacemaker_file_path)
-        assert result_line == 3
 
     def test_findln_by_timestamp_irregular(self):
         data = """line1
diff --git a/test/unittests/test_sbd.py b/test/unittests/test_sbd.py
index f73a8734..bf0a0a8c 100644
--- a/test/unittests/test_sbd.py
+++ b/test/unittests/test_sbd.py
@@ -725,18 +725,16 @@ class TestSBDManager(unittest.TestCase):
     @patch('crmsh.utils.delete_property')
     @patch('crmsh.utils.get_property')
     @patch('crmsh.sbd.sh.cluster_shell')
-    @patch('crmsh.sbd.SBDUtils.get_sbd_device_from_config')
     @patch('crmsh.sbd.xmlutil.CrmMonXmlParser')
     @patch('crmsh.utils.set_property')
     @patch('crmsh.sbd.ServiceManager')
-    def test_configure_sbd(self, mock_ServiceManager, mock_set_property, mock_CrmMonXmlParser, mock_get_sbd_device_from_config, mock_cluster_shell, mock_get_property, mock_delete_property):
+    def test_configure_sbd(self, mock_ServiceManager, mock_set_property, mock_CrmMonXmlParser, mock_cluster_shell, mock_get_property, mock_delete_property):
         mock_get_property.return_value = -1
         mock_CrmMonXmlParser.return_value.is_resource_configured.return_value = False
-        mock_get_sbd_device_from_config.return_value = ['/dev/sbd_device']
         mock_cluster_shell.return_value.get_stdout_or_raise_error.return_value = "data"
         sbdmanager_instance = SBDManager()
         sbdmanager_instance.configure_sbd()
-        mock_cluster_shell.return_value.get_stdout_or_raise_error.assert_called_once_with("crm configure primitive stonith-sbd stonith:fence_sbd params devices=\"/dev/sbd_device\"")
+        mock_cluster_shell.return_value.get_stdout_or_raise_error.assert_called_once_with("crm configure primitive stonith-sbd stonith:fence_sbd")
 
 
 class TestOutterFunctions(unittest.TestCase):