Repository: ClusterLabs/crmsh

Issue:
Error when peer nodes are unreachable
When peer nodes node2 and node3 are all unreachable:
```
# crm cluster stop node2 node3
WARNING: host "node2" is unreachable
ERROR: cluster.stop: Failed to run command systemctl is-active 'corosync.service' on root@node3: ssh: connect to host alp-3 port 22: No route to host Please configure passwordless authentication with "crm cluster init ssh" and "crm cluster join ssh"
```

Expected output:
```
# crm cluster stop node2 node3
WARNING: host "node2" is unreachable
WARNING: host "node3" is unreachable
```

Discussion:

Solution patch:
diff --git a/contrib/bash_completion.sh b/contrib/bash_completion.sh
index bfdc1b02..fe050c72 100644
--- a/contrib/bash_completion.sh
+++ b/contrib/bash_completion.sh
@@ -253,4 +253,4 @@ _crm() {
 
 	__crm_compgen
 } &&
-complete -o bashdefault -o default -o nospace -F _crm crm || complete -o default -o nospace -F _crm crm
+complete -o bashdefault -o default -o nospace -o nosort -F _crm crm || complete -o default -o nospace -o nosort -F _crm crm
diff --git a/crmsh/command.py b/crmsh/command.py
index ee257f92..4ae46510 100644
--- a/crmsh/command.py
+++ b/crmsh/command.py
@@ -540,6 +540,7 @@ class ChildInfo(object):
                 ret = self.completer([self.name] + args, context)
             else:
                 ret = self.completer([self.name] + args)
+            ret = ret if already_used_custom_sort_order() else sorted(ret)
         return ret
 
     def __repr__(self):
@@ -567,8 +568,12 @@ def enable_custom_sort_order():
         sort_completion_inst.value = 0
 
 
+def already_used_custom_sort_order():
+    return sort_completion_inst is not None and sort_completion_inst.value == 0
+
+
 def disable_custom_sort_order():
-    if sort_completion_inst is not None and sort_completion_inst.value == 0:
+    if already_used_custom_sort_order():
         # Restore the original value of rl_sort_completion_matches
         # before calling the completer again
         sort_completion_inst.value = orig_sort_completion_value
diff --git a/crmsh/report/core.py b/crmsh/report/core.py
index 5a947017..c88c56d9 100644
--- a/crmsh/report/core.py
+++ b/crmsh/report/core.py
@@ -352,14 +352,7 @@ def process_node_list(context: Context) -> None:
         if not context.node_list:
             raise utils.ReportGenericError("Could not figure out a list of nodes; is this a cluster node?")
 
-    for node in context.node_list[:]:
-        if node == context.me:
-            continue
-        try:
-            crmutils.node_reachable_check(node)
-        except Exception as err:
-            logger.error(str(err))
-            context.node_list.remove(node)
+    context.node_list = crmutils.get_reachable_node_list(context.node_list)
 
 
 def process_arguments(context: Context) -> None:
diff --git a/crmsh/ui_node.py b/crmsh/ui_node.py
index d05be96d..62380122 100644
--- a/crmsh/ui_node.py
+++ b/crmsh/ui_node.py
@@ -254,13 +254,7 @@ keep the node in standby after reboot. The life time defaults to
             context.fatal_error(f"Node '{node}' is not a member of the cluster")
 
     node_list = member_list if options.all else args
-    for node in node_list:
-        try:
-            utils.node_reachable_check(node)
-        except ValueError as err:
-            logger.warning(str(err))
-            node_list.remove(node)
-    return node_list
+    return utils.get_reachable_node_list(node_list)
 
 
 class NodeMgmt(command.UI):
diff --git a/crmsh/ui_sbd.py b/crmsh/ui_sbd.py
index 69aeb147..dab3ac6e 100644
--- a/crmsh/ui_sbd.py
+++ b/crmsh/ui_sbd.py
@@ -142,12 +142,7 @@ class SBD(command.UI):
         self.service_manager = ServiceManager()
         self.cluster_shell = sh.cluster_shell()
         self.cluster_nodes = utils.list_cluster_nodes() or [utils.this_node()]
-        for node in self.cluster_nodes[:]:
-            try:
-                utils.node_reachable_check(node)
-            except Exception as e:
-                logger.error(e)
-                self.cluster_nodes.remove(node)
+        self.cluster_nodes = utils.get_reachable_node_list(self.cluster_nodes)
 
     def requires(self) -> bool:
         '''
diff --git a/crmsh/utils.py b/crmsh/utils.py
index a267e0e4..1a772f99 100644
--- a/crmsh/utils.py
+++ b/crmsh/utils.py
@@ -2452,6 +2452,17 @@ def node_reachable_check(node, ping_count=1, port=22, timeout=3):
     raise ValueError(f"host \"{node}\" is unreachable")
 
 
+def get_reachable_node_list(node_list:list[str]) -> list[str]:
+    reachable_node_list = []
+    for node in node_list:
+        try:
+            if node_reachable_check(node):
+                reachable_node_list.append(node)
+        except ValueError as e:
+            logger.warning(str(e))
+    return reachable_node_list
+
+
 def calculate_quorate_status(expected_votes, actual_votes):
     """
     Given expected votes and actual votes, calculate if is quorated


Test patch:
diff --git a/test/unittests/test_report_core.py b/test/unittests/test_report_core.py
index ecfc9b51..55fa5a4b 100644
--- a/test/unittests/test_report_core.py
+++ b/test/unittests/test_report_core.py
@@ -466,21 +466,22 @@ class TestRun(unittest.TestCase):
             core.process_node_list(mock_ctx_inst)
             self.assertEqual("Could not figure out a list of nodes; is this a cluster node?", str(err.exception))
 
+    @mock.patch('crmsh.utils.get_reachable_node_list')
     @mock.patch('crmsh.report.core.local_mode')
     @mock.patch('crmsh.utils.list_cluster_nodes')
-    def test_process_node_list_single(self, mock_list_nodes, mock_local_mode):
+    def test_process_node_list_single(self, mock_list_nodes, mock_local_mode, mock_reachable):
         mock_local_mode.return_value = True
         mock_ctx_inst = mock.Mock(node_list=["node1", "node2"], single=True, me="node1")
         core.process_node_list(mock_ctx_inst)
 
+    @mock.patch('crmsh.utils.get_reachable_node_list')
     @mock.patch('crmsh.report.core.local_mode')
     @mock.patch('logging.Logger.error')
-    @mock.patch('crmsh.utils.node_reachable_check')
     @mock.patch('crmsh.utils.list_cluster_nodes')
-    def test_process_node_list(self, mock_list_nodes, mock_reachable, mock_error, mock_local_mode):
+    def test_process_node_list(self, mock_list_nodes, mock_error, mock_local_mode, mock_reachable):
         mock_local_mode.return_value = False
         mock_ctx_inst = mock.Mock(node_list=["node1", "node2"], single=False, me="node1")
-        mock_reachable.side_effect = ValueError("error")
+        mock_reachable.return_value = ["node1"]
         core.process_node_list(mock_ctx_inst)
         self.assertEqual(mock_ctx_inst.node_list, ["node1"])
 
diff --git a/test/unittests/test_utils.py b/test/unittests/test_utils.py
index 60952f51..eff51ac1 100644
--- a/test/unittests/test_utils.py
+++ b/test/unittests/test_utils.py
@@ -1391,3 +1391,16 @@ def test_is_dc_idle(mock_dc, mock_shell):
     mock_shell.return_value = mock_shell_inst
     mock_shell_inst.get_stdout_stderr.return_value = (0, "in S_IDLE: ok", None)
     assert utils.is_dc_idle() is True
+
+
+@mock.patch('logging.Logger.warning')
+@mock.patch('crmsh.utils.node_reachable_check')
+def test_get_reachable_node_list(mock_reachable, mock_warn):
+    mock_reachable.side_effect = [False, True, ValueError("error for node3")]
+    assert utils.get_reachable_node_list(["node1", "node2", "node3"]) == ["node2"]
+    mock_warn.assert_called_once_with("error for node3")
+    mock_reachable.assert_has_calls([
+        mock.call("node1"),
+        mock.call("node2"),
+        mock.call("node3")
+    ])