From d39f6e50cf4f7c19edc9b9af990889e4f903a524 Mon Sep 17 00:00:00 2001 From: xin liang Date: Tue, 2 Apr 2024 22:12:52 +0800 Subject: [PATCH 1/3] Fix: ui_node: When `utils.list_cluster_nodes` return None, try to get ip list from corosync.conf PR#1312 try to fix the issue that `crm cluster start --all` failed when there is no CIB, but it also break what `list_cluster_nodes` returns, which is widely used in crmsh and will cause many other unexpected issues. This patch fix that regression by calling `utils.get_address_list_from_corosync_conf` after `utils.list_cluster_nodes` return None when parsing node args. --- crmsh/ui_node.py | 5 ++++- crmsh/utils.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crmsh/ui_node.py b/crmsh/ui_node.py index 1d015b00e0..658d86561d 100644 --- a/crmsh/ui_node.py +++ b/crmsh/ui_node.py @@ -246,9 +246,12 @@ def parse_option_for_nodes(context, *args): # return local node if (not options.all and not args) or (len(args) == 1 and args[0] == utils.this_node()): return [utils.this_node()] - member_list = utils.list_cluster_nodes() + member_list = utils.list_cluster_nodes() or utils.get_address_list_from_corosync_conf() if not member_list: context.fatal_error("Cannot get the node list from cluster") + for node in args: + if node not in member_list: + 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: diff --git a/crmsh/utils.py b/crmsh/utils.py index a114924efc..04f7de5ff9 100644 --- a/crmsh/utils.py +++ b/crmsh/utils.py @@ -1812,10 +1812,10 @@ def list_cluster_nodes(no_reg=False): else: cib_path = os.getenv('CIB_file', constants.CIB_RAW_FILE) if not os.path.isfile(cib_path): - return get_address_list_from_corosync_conf() + return None cib = xmlutil.file2cib_elem(cib_path) if cib is None: - return get_address_list_from_corosync_conf() + return None node_list = [] for node in cib.xpath(constants.XML_NODE_PATH): From 2f09a2dc52ff90668e934c3103deb2078ca89658 Mon Sep 17 00:00:00 2001 From: xin liang Date: Wed, 3 Apr 2024 08:57:38 +0800 Subject: [PATCH 2/3] Dev: behave: Adjust functional test for previous commit --- test/features/bootstrap_bugs.feature | 3 +++ test/features/crm_report_normal.feature | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/features/bootstrap_bugs.feature b/test/features/bootstrap_bugs.feature index 2da95928d9..0d55e6e89a 100644 --- a/test/features/bootstrap_bugs.feature +++ b/test/features/bootstrap_bugs.feature @@ -151,6 +151,9 @@ Feature: Regression test for bootstrap bugs Then Cluster service is "started" on "hanode1" Then Cluster service is "started" on "hanode2" + When Try "crm cluster start xxx" + Then Except "ERROR: cluster.start: Node 'xxx' is not a member of the cluster" + @clean Scenario: Can't stop all nodes' cluster service when local node's service is down(bsc#1213889) Given Cluster service is "stopped" on "hanode1" diff --git a/test/features/crm_report_normal.feature b/test/features/crm_report_normal.feature index 7b038ca66b..00a1f2b41c 100644 --- a/test/features/crm_report_normal.feature +++ b/test/features/crm_report_normal.feature @@ -105,4 +105,5 @@ Feature: crm report functional test for common cases When Run "crm cluster stop --all" on "hanode1" When Run "rm -f /var/lib/pacemaker/cib/cib*" on "hanode1" When Run "rm -f /var/lib/pacemaker/cib/cib*" on "hanode2" - When Run "crm report" OK + When Try "crm report" on "hanode1" + Then Expected "Could not figure out a list of nodes; is this a cluster node" in stderr From df8a7f1c1e6365ec13cd5ec1e7d6bbc7a1bd503d Mon Sep 17 00:00:00 2001 From: xin liang Date: Wed, 3 Apr 2024 09:29:10 +0800 Subject: [PATCH 3/3] Dev: unittest: Adjust unit test for previous commit --- test/unittests/test_utils.py | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/test/unittests/test_utils.py b/test/unittests/test_utils.py index 31e25f4d16..99d00a7172 100644 --- a/test/unittests/test_utils.py +++ b/test/unittests/test_utils.py @@ -1202,43 +1202,26 @@ def test_is_quorate(mock_run): mock_run_inst.get_stdout_or_raise_error.assert_called_once_with("corosync-quorumtool -s", None, success_exit_status={0, 2}) -@mock.patch('crmsh.utils.get_address_list_from_corosync_conf') @mock.patch('crmsh.utils.etree.fromstring') @mock.patch('crmsh.sh.ShellUtils.get_stdout_stderr') -def test_list_cluster_nodes_none(mock_run, mock_etree, mock_corosync): +def test_list_cluster_nodes_none(mock_run, mock_etree): mock_run.return_value = (0, "data", None) mock_etree.return_value = None - mock_corosync.return_value = ["node1", "node2"] res = utils.list_cluster_nodes() - assert res == ["node1", "node2"] + assert res is None mock_run.assert_called_once_with(constants.CIB_QUERY, no_reg=False) mock_etree.assert_called_once_with("data") -@mock.patch('crmsh.utils.get_address_list_from_corosync_conf') -@mock.patch('crmsh.utils.etree.fromstring') -@mock.patch('crmsh.sh.ShellUtils.get_stdout_stderr') -def test_list_cluster_nodes_none_no_reg(mock_run, mock_etree, mock_corosync): - mock_run.return_value = (0, "data", None) - mock_etree.return_value = None - mock_corosync.return_value = ["node1", "node2"] - res = utils.list_cluster_nodes(no_reg=True) - assert res == ["node1", "node2"] - mock_run.assert_called_once_with(constants.CIB_QUERY, no_reg=True) - mock_etree.assert_called_once_with("data") - - -@mock.patch('crmsh.utils.get_address_list_from_corosync_conf') @mock.patch('os.path.isfile') @mock.patch('os.getenv') @mock.patch('crmsh.sh.ShellUtils.get_stdout_stderr') -def test_list_cluster_nodes_cib_not_exist(mock_run, mock_env, mock_isfile, mock_corosync): +def test_list_cluster_nodes_cib_not_exist(mock_run, mock_env, mock_isfile): mock_run.return_value = (1, None, None) mock_env.return_value = constants.CIB_RAW_FILE mock_isfile.return_value = False - mock_corosync.return_value = ["node1", "node2"] res = utils.list_cluster_nodes() - assert res == ["node1", "node2"] + assert res is None mock_run.assert_called_once_with(constants.CIB_QUERY, no_reg=False) mock_env.assert_called_once_with("CIB_file", constants.CIB_RAW_FILE) mock_isfile.assert_called_once_with(constants.CIB_RAW_FILE)