From b949d78b1af2333191674b1594264ad53ab2a2a1 Mon Sep 17 00:00:00 2001 From: nicholasyang Date: Mon, 6 Jul 2026 19:11:57 +0800 Subject: [PATCH 1/3] Dev: corosync: Add corosync health check Introduce the corosync health check under 'crm cluster health corosync'. - Add crmsh/corosync_healthcheck.py containing local configuration validity, cross-node consistency, quorum status, link status, and Node ID mapping validation. - Implement ui_cluster.py integration for the 'corosync' health component. - Write test/unittests/test_corosync_healthcheck.py and test_ui_cluster.py for comprehensive unit test coverage. - Update doc/crm.8.adoc with help information and documentation. - Add functional behavior tests in test/features/corosync_ui.feature. --- crmsh/corosync_healthcheck.py | 414 ++++++++++++++ crmsh/ui_cluster.py | 71 ++- data-manifest | 1 + doc/crm.8.adoc | 5 +- test/features/corosync_ui.feature | 18 + test/unittests/test_corosync_healthcheck.py | 600 ++++++++++++++++++++ test/unittests/test_ui_cluster.py | 136 +++++ 7 files changed, 1243 insertions(+), 2 deletions(-) create mode 100644 crmsh/corosync_healthcheck.py create mode 100644 test/unittests/test_corosync_healthcheck.py diff --git a/crmsh/corosync_healthcheck.py b/crmsh/corosync_healthcheck.py new file mode 100644 index 0000000000..fc41eb34e0 --- /dev/null +++ b/crmsh/corosync_healthcheck.py @@ -0,0 +1,414 @@ +import dataclasses +import functools +import logging +import re +import shlex +import subprocess +import typing + +from io import StringIO + +import lxml.etree + +from . import corosync +from . import cibquery +from . import constants +from .prun import prun +from . import sh + + +logger = logging.getLogger(__name__) + + +@dataclasses.dataclass +class CheckResult: + check_name: str + checked_nodes: list[str] + returncode: int + result_description: typing.Optional[str] + recommended_action: typing.Optional[str] + + +def validate_config_file_consistency(nodes: list[str]) -> CheckResult: + assert nodes + CHECK_NAME = 'Validate Corosync Configuration File Consistency' + config_file_path = corosync.conf() + result = prun.prun({node: f'sha256sum {shlex.quote(config_file_path)}' for node in nodes}) + ssh_errors = {k: v for k, v in result.items() if isinstance(v, prun.SSHError)} + if ssh_errors: + for node, error in ssh_errors.items(): + logger.error("%s", error) + return CheckResult( + CHECK_NAME, + list(ssh_errors.keys()), + 255, + "ssh error", + None, + ) + command_errors = {k: v for k, v in result.items() if v.returncode != 0} + if command_errors: + for node, process_result in command_errors.items(): + logger.error("%s: %s", node, process_result.stderr) + return CheckResult( + CHECK_NAME, + list(command_errors.keys()), + functools.reduce(lambda a, b: a | b, (v.returncode for v in command_errors.values())), + "sha256sum error", + None, + ) + prev = None + for x in result.values(): + if prev is None or x.stdout == prev: + prev = x.stdout + else: + result_description = StringIO() + result_description.write(f"Corosync configuration file {config_file_path} is inconsistent across nodes:\n") + for node_name, y in result.items(): + stdout_str = y.stdout.decode('utf-8', 'replace').strip() + parts = stdout_str.split(maxsplit=1) + if len(parts) == 2: + sha256_hash, _ = parts + result_description.write(f" {sha256_hash} {node_name}\n") + else: + result_description.write(f" {stdout_str} {node_name}\n") + result_description.flush() + return CheckResult( + CHECK_NAME, + nodes, + 1, + result_description.getvalue(), + 'Run "crm corosync diff" to inspect the differences, and then "crm corosync push" to synchronize the configuration across cluster nodes.', + ) + return CheckResult( + CHECK_NAME, + nodes, + 0, + None, + None, + ) + + +def validate_config_file(node: str) -> CheckResult: + result = sh.cluster_shell().subprocess_run_without_input( + node, 'root', + 'corosync -t', + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + ) + CHECK_NAME = 'Validate Corosync Configuration File' + match result.returncode: + case 0: + return CheckResult( + CHECK_NAME, + [node], + result.returncode, + None, + None, + ) + case _: + return CheckResult( + CHECK_NAME, + [node], + result.returncode, + result.stdout.decode('utf-8', 'replace').strip(), + None, + ) + + +def check_quorum_status(local_node: str) -> CheckResult: + command_args = ['corosync-quorumtool', '-s'] + result = subprocess.run( + command_args, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + ) + CHECK_NAME = "Check Quorum Status" + if result.returncode not in ( + 0, + 2, # not quorate + ): + return CheckResult( + CHECK_NAME, + [local_node], + result.returncode, + result.stdout.decode('utf-8', 'replace').strip(), + "Check if the corosync service is running.", + ) + try: + quorum_info = _parse_quorum_status(result.stdout.decode('utf-8', 'replace')) + except ValueError as e: + return CheckResult( + CHECK_NAME, + [local_node], + 1, + f"Failed to parse '{' '.join(command_args)}' output: {e}", + None, + ) + if not quorum_info.quorate: + return CheckResult( + CHECK_NAME, + [local_node], + 1, + f"The cluster is not quorate. Total votes: {quorum_info.total_votes}, Quorum: {quorum_info.quorum}", + "Ensure that enough nodes are online and connected to form a quorum.", + ) + if quorum_info.total_votes < quorum_info.expected_votes: + return CheckResult( + CHECK_NAME, + [local_node], + 1, + f"Total votes ({quorum_info.total_votes}) is less than expected votes ({quorum_info.expected_votes}).", + "Check that all expected cluster nodes are online and connected.", + ) + return CheckResult( + CHECK_NAME, + [local_node], + 0, + None, + None, + ) + + +@dataclasses.dataclass +class _QuorumInfo: + expected_votes: int + total_votes: int + quorum: int + quorate: bool + + +def _parse_quorum_status(output: str) -> _QuorumInfo: + """ + Parses the output of 'corosync-quorumtool -s' to extract + expected_votes, total_votes, quorum, and quorate status from Flags. + """ + expected_votes = None + total_votes = None + quorum = None + quorate = None + for line in output.splitlines(): + line = line.strip() + if ':' not in line: + continue + key, val = line.split(':', 1) + key = key.strip() + val = val.strip() + parts = val.split() + if not parts: + continue + if key == 'Expected votes': + expected_votes = int(parts[0]) + elif key == 'Total votes': + total_votes = int(parts[0]) + elif key == 'Quorum': + quorum = int(parts[0]) + elif key == 'Flags': + quorate = 'Quorate' in parts + if expected_votes is None: + raise ValueError("Missing 'Expected votes' in quorum status output") + if total_votes is None: + raise ValueError("Missing 'Total votes' in quorum status output") + if quorum is None: + raise ValueError("Missing 'Quorum' in quorum status output") + if quorate is None: + raise ValueError("Missing 'Flags' in quorum status output") + return _QuorumInfo( + expected_votes=expected_votes, + total_votes=total_votes, + quorum=quorum, + quorate=quorate, + ) + + +def check_nodeid_to_nodename_mapping(local_node: str) -> CheckResult: + """ + Check against the mismatch of nodeid to nodename mapping between corosync.conf and CIB. + """ + CHECK_NAME = 'Check Node ID to Node Name Mapping' + rc, out, err = sh.ShellUtils().get_stdout_stderr(constants.CIB_QUERY) + if rc != 0: + return CheckResult( + CHECK_NAME, + [local_node], + 255, + f"Failed to load CIB: {err}", + None, + ) + try: + cib = lxml.etree.fromstring(out) + except lxml.etree.ParseError as e: + return CheckResult( + CHECK_NAME, + [local_node], + 255, + f"Failed to parse CIB: {e}", + None, + ) + try: + cib_nodes = cibquery.get_cluster_nodes(cib) + except AssertionError as e: + return CheckResult( + CHECK_NAME, + [local_node], + 255, + f"Failed to extract cluster nodes from CIB: {e}", + None, + ) + + try: + lm = corosync.LinkManager.load_config_file() + try: + config_nodelist = lm._config['nodelist']['node'] + except KeyError: + config_nodelist = [] + if not isinstance(config_nodelist, list): + config_nodelist = [config_nodelist] + except ValueError as e: + return CheckResult( + CHECK_NAME, + [local_node], + 255, + f"Failed to load or parse corosync.conf: {e}", + None, + ) + corosync_id_to_name = {} + for node in config_nodelist: + nodeid_str = node.get('nodeid') + name = node.get('name') + if nodeid_str is not None: + try: + nodeid = int(nodeid_str) + except ValueError: + continue + if name is not None: + corosync_id_to_name[nodeid] = name + + cib_id_to_name = {node.node_id: node.uname for node in cib_nodes} + mismatches = [] + if corosync_id_to_name == cib_id_to_name: + pass + for nodeid, nodename in corosync_id_to_name.items(): + cib_name = cib_id_to_name.get(nodeid, None) + if cib_name is None: + mismatches.append( + f"Node ID {nodeid} with name '{nodename}' in corosync.conf is not found in CIB." + ) + elif cib_name != nodename: + mismatches.append( + f"Node ID {nodeid} is associated with name '{nodename}' in corosync.conf but '{cib_name}' in CIB." + ) + for nodeid, nodename in cib_id_to_name.items(): + corosync_name = corosync_id_to_name.get(nodeid, None) + if corosync_name is None: + mismatches.append( + f"Node ID {nodeid} with name '{nodename}' in CIB is not found in corosync.conf." + ) + if mismatches: + result_description = StringIO() + result_description.write("Mismatches between corosync.conf and CIB found.") + for x in mismatches: + result_description.write('\n * ') + result_description.write(x) + return CheckResult( + CHECK_NAME, + [local_node], + 1, + result_description.getvalue(), + "Ensure nodeid and name mapping are consistent between corosync.conf and CIB.", + ) + else: + return CheckResult( + CHECK_NAME, + [local_node], + 0, + None, + None, + ) + + +def check_links_status(local_node: str) -> CheckResult: + """ + Check if corosync links are operational. + """ + command_args = ['corosync-cfgtool', '-s'] + result = subprocess.run( + command_args, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + ) + CHECK_NAME = "Check Corosync Links Status" + if result.returncode != 0: + return CheckResult( + CHECK_NAME, + [local_node], + result.returncode, + result.stdout.decode('utf-8', 'replace').strip(), + "Check if the corosync service is running.", + ) + try: + links_info = _parse_links_status(result.stdout.decode('utf-8', 'replace')) + except ValueError as e: + return CheckResult( + CHECK_NAME, + [local_node], + 1, + f"Failed to parse '{' '.join(command_args)}' output: {e}", + None, + ) + disconnected_links = [] + for link_id, nodes in links_info.items(): + for node_id, status in nodes.items(): + if status not in ("localhost", "connected"): + disconnected_links.append((link_id, node_id, status)) + if disconnected_links: + disconnected_links.sort() + msg_parts = [] + for link_id, node_id, status in disconnected_links: + msg_parts.append(f"Link {link_id} to node {node_id} has status '{status}'.") + result_description = "Corosync link(s) are not operational:\n" + "\n".join(msg_parts) + return CheckResult( + CHECK_NAME, + [local_node], + 1, + result_description, + "Check network connectivity and corosync configuration/service on the affected nodes.", + ) + return CheckResult( + CHECK_NAME, + [local_node], + 0, + None, + None, + ) + + +def _parse_links_status(output: str) -> dict[int, dict[int, str]]: + """ + Parses the output of 'corosync-cfgtool -s' to extract + link statuses per node. + """ + links = {} + current_link = None + for line in output.splitlines(): + line = line.strip() + if not line: + continue + link_match = re.match(r'^LINK ID\s+(\d+)', line) + if link_match: + current_link = int(link_match.group(1)) + links[current_link] = {} + continue + node_match = re.match(r'^nodeid:\s*(\d+):\s*(.*)', line) + if node_match: + if current_link is None: + raise ValueError("Found node ID status before any LINK ID section") + node_id = int(node_match.group(1)) + status = node_match.group(2).strip() + links[current_link][node_id] = status + if not links: + raise ValueError("No corosync links found in output") + return links + + diff --git a/crmsh/ui_cluster.py b/crmsh/ui_cluster.py index 34530e01b2..bfe8a3643c 100644 --- a/crmsh/ui_cluster.py +++ b/crmsh/ui_cluster.py @@ -12,6 +12,9 @@ import subprocess import glob import time +import json +import dataclasses +import functools from argparse import ArgumentParser, RawDescriptionHelpFormatter import crmsh.parallax @@ -22,6 +25,8 @@ from . import completers as compl from . import bootstrap from . import corosync +from . import corosync_healthcheck +from . import term from . import qdevice from . import xmlutil from . import cibconfig @@ -141,6 +146,18 @@ class ArgparseUserAtHostAppendAction( pass +def _print_corosync_check_result(r): + if r.returncode == 0: + print(term.render(f"${{GREEN}}[PASS]${{NORMAL}} {r.check_name}")) + else: + print(term.render(f"${{RED}}[FAIL]${{NORMAL}} {r.check_name}")) + if r.result_description: + for line in r.result_description.splitlines(): + print(f" {line}") + if r.recommended_action: + for line in r.recommended_action.splitlines(): + print(f" Recommended Action: {line}") + class Cluster(command.UI): ''' @@ -805,7 +822,7 @@ def do_geo_init_arbitrator(self, context, *args): bootstrap.bootstrap_arbitrator(geo_context) return True - HEALTH_COMPONENTS = ['hawk2', 'sles16', 'sbd'] + HEALTH_COMPONENTS = ['hawk2', 'sles16', 'sbd', 'corosync'] @command.completers(compl.choice(HEALTH_COMPONENTS)) def do_health(self, context, *args): ''' @@ -872,6 +889,58 @@ def do_health(self, context, *args): except migration.MigrationFailure as e: logger.error('%s', e) return False + + case 'corosync': + corosync_parser = argparse.ArgumentParser('corosync') + corosync_parser.add_argument('--local', action='store_true') + corosync_parser.add_argument('--json', action='store_true') + try: + corosync_args = corosync_parser.parse_args(remaining_args) + except SystemExit: + return False + + local_node = utils.this_node() + results = [] + + if corosync_args.json: + print_cb = lambda r: None + else: + print_cb = _print_corosync_check_result + + res_local = corosync_healthcheck.validate_config_file(local_node) + results.append(res_local) + print_cb(res_local) + + if res_local.returncode == 0: + if not corosync_args.local: + nodes = utils.list_cluster_nodes() or [local_node] + res_consistency = corosync_healthcheck.validate_config_file_consistency(nodes) + results.append(res_consistency) + print_cb(res_consistency) + + res_quorum = corosync_healthcheck.check_quorum_status(local_node) + results.append(res_quorum) + print_cb(res_quorum) + + res_links = corosync_healthcheck.check_links_status(local_node) + results.append(res_links) + print_cb(res_links) + + res_mapping = corosync_healthcheck.check_nodeid_to_nodename_mapping(local_node) + results.append(res_mapping) + print_cb(res_mapping) + + returncode = functools.reduce(lambda a, b: a | b, (r.returncode for r in results)) + if corosync_args.json: + json_data = { + "returncode": returncode, + "results": [dataclasses.asdict(r) for r in results] + } + json.dump(json_data, sys.stdout, ensure_ascii=False) + sys.stdout.write('\n') + + return 0 == returncode + case _: logger.error('Unknown component: %s', parsed_args.component) return False diff --git a/data-manifest b/data-manifest index 6ecc13c733..9e78f33a89 100644 --- a/data-manifest +++ b/data-manifest @@ -193,6 +193,7 @@ test/unittests/test_cliformat.py test/unittests/test_cluster_fs.py test/unittests/test.conf test/unittests/test_corosync_config_format.py +test/unittests/test_corosync_healthcheck.py test/unittests/test_corosync.py test/unittests/test_crashtest_main.py test/unittests/test_crashtest_task.py diff --git a/doc/crm.8.adoc b/doc/crm.8.adoc index 6b0592be94..34ed0eac27 100644 --- a/doc/crm.8.adoc +++ b/doc/crm.8.adoc @@ -1093,7 +1093,7 @@ Usage 2: Topic-Specified Health Check Verifies the health of a specified topic. ............... -health hawk2|sbd|sles16 [--local] [--fix] +health hawk2|sbd|sles16|corosync [--local] [--fix] [--json] ............... * `hawk2`: check or fix key-based ssh authentication for user hacluster, which @@ -1105,6 +1105,9 @@ health hawk2|sbd|sles16 [--local] [--fix] * `sles16`: check whether the cluster is good to migrate to SLES 16. ** `--local`: run checks in local mode ** `--fix`: attempts to automatically resolve any detected issues. +* `corosync`: check corosync-related configurations and status. + ** `--local`: run checks in local mode. + ** `--json`: output the check results in JSON format. [[cmdhelp.cluster.init,Initializes a new HA cluster,From Code]] ==== `init` diff --git a/test/features/corosync_ui.feature b/test/features/corosync_ui.feature index 744c16041b..aa832d4991 100644 --- a/test/features/corosync_ui.feature +++ b/test/features/corosync_ui.feature @@ -17,6 +17,8 @@ Feature: crm corosync ui test cases Then Except "No such file or directory: '/etc/corosync/corosync.conf'" in stderr When Try "crm corosync link remove 0" on "hanode1" Then Except "No such file or directory: '/etc/corosync/corosync.conf'" in stderr + When Try "crm cluster health corosync" on "hanode1" + Then Expected "[FAIL] Validate Corosync Configuration File" in stdout Scenario: link show/add/update/remove # background @@ -64,3 +66,19 @@ Feature: crm corosync ui test cases Then Expected "2" in stdout When Run "crm corosync set totem.token 6000" on "hanode1" Then Expected "Use "crm corosync push" to sync" in stdout + + Scenario: corosync health check + Given Nodes ["hanode1", "hanode2"] are cleaned up + And Cluster service is "stopped" on "hanode1" + And Cluster service is "stopped" on "hanode2" + When Run "crm cluster init -y" on "hanode1" + Then Cluster service is "started" on "hanode1" + When Run "crm cluster join -c hanode1 -y" on "hanode2" + Then Cluster service is "started" on "hanode2" + And Online nodes are "hanode1 hanode2" + When Run "crm cluster health corosync" on "hanode1" + Then Expected "[PASS] Check Node ID to Node Name Mapping" in stdout + When Run "crm cluster health corosync --local" on "hanode1" + Then Expected "Validate Corosync Configuration File Consistency" not in stdout + When Run "crm cluster health corosync --json" on "hanode1" + Then Expected "returncode" in stdout diff --git a/test/unittests/test_corosync_healthcheck.py b/test/unittests/test_corosync_healthcheck.py new file mode 100644 index 0000000000..6ed4552f1d --- /dev/null +++ b/test/unittests/test_corosync_healthcheck.py @@ -0,0 +1,600 @@ +# Copyright (C) 2026 Nicholas +# See COPYING for license information. +# +# unit tests for corosync_healthcheck.py + +import subprocess +import unittest +from unittest import mock +from crmsh import corosync_healthcheck + + +class TestParseQuorumStatus(unittest.TestCase): + def setUp(self): + self.valid_output = """Quorum information +------------------ +Date: Fri Jun 26 13:44:28 2026 +Quorum provider: corosync_votequorum +Nodes: 2 +Node ID: 1 +Ring ID: 1.e +Quorate: Yes + +Votequorum information +---------------------- +Expected votes: 3 +Highest expected: 3 +Total votes: 3 +Quorum: 2 +Flags: Quorate Qdevice + +Membership information +---------------------- + Nodeid Votes Qdevice Name + 1 1 A,V,NMW ha-3-1 (local) + 2 1 A,V,NMW ha-3-2 + 0 1 Qdevice""" + + self.not_quorate_output = """Quorum information +------------------ +Date: Fri Jun 26 13:44:28 2026 +Quorum provider: corosync_votequorum +Nodes: 2 +Node ID: 1 +Ring ID: 1.e +Quorate: No + +Votequorum information +---------------------- +Expected votes: 3 +Highest expected: 3 +Total votes: 1 +Quorum: 2 +Flags: Qdevice + +Membership information +---------------------- + Nodeid Votes Qdevice Name + 1 1 A,V,NMW ha-3-1 (local) + 0 0 Qdevice""" + + self.blocked_output = """Quorum information +------------------ +Date: Fri Jun 26 13:44:28 2026 +Quorum provider: corosync_votequorum +Nodes: 2 +Node ID: 1 +Ring ID: 1.e +Quorate: No + +Votequorum information +---------------------- +Expected votes: 3 +Highest expected: 3 +Total votes: 2 +Quorum: 2 Activity blocked +Flags: Qdevice + +Membership information +---------------------- + Nodeid Votes Qdevice Name + 1 1 A,V,NMW ha-3-1 (local) + 0 1 Qdevice""" + + def test_parse_quorum_status_success(self): + info = corosync_healthcheck._parse_quorum_status(self.valid_output) + self.assertEqual(info.expected_votes, 3) + self.assertEqual(info.total_votes, 3) + self.assertEqual(info.quorum, 2) + self.assertTrue(info.quorate) + + def test_parse_quorum_status_not_quorate(self): + info = corosync_healthcheck._parse_quorum_status(self.not_quorate_output) + self.assertEqual(info.expected_votes, 3) + self.assertEqual(info.total_votes, 1) + self.assertEqual(info.quorum, 2) + self.assertFalse(info.quorate) + + def test_parse_quorum_status_activity_blocked(self): + info = corosync_healthcheck._parse_quorum_status(self.blocked_output) + self.assertEqual(info.expected_votes, 3) + self.assertEqual(info.total_votes, 2) + self.assertEqual(info.quorum, 2) + self.assertFalse(info.quorate) + + def test_parse_quorum_status_missing_expected_votes(self): + output = """Votequorum information +---------------------- +Highest expected: 3 +Total votes: 3 +Quorum: 2 +Flags: Quorate""" + with self.assertRaises(ValueError) as excinfo: + corosync_healthcheck._parse_quorum_status(output) + self.assertIn("Missing 'Expected votes'", str(excinfo.exception)) + + def test_parse_quorum_status_missing_total_votes(self): + output = """Votequorum information +---------------------- +Expected votes: 3 +Highest expected: 3 +Quorum: 2 +Flags: Quorate""" + with self.assertRaises(ValueError) as excinfo: + corosync_healthcheck._parse_quorum_status(output) + self.assertIn("Missing 'Total votes'", str(excinfo.exception)) + + def test_parse_quorum_status_missing_quorum(self): + output = """Votequorum information +---------------------- +Expected votes: 3 +Highest expected: 3 +Total votes: 3 +Flags: Quorate""" + with self.assertRaises(ValueError) as excinfo: + corosync_healthcheck._parse_quorum_status(output) + self.assertIn("Missing 'Quorum'", str(excinfo.exception)) + + def test_parse_quorum_status_missing_flags(self): + output = """Votequorum information +---------------------- +Expected votes: 3 +Highest expected: 3 +Total votes: 3 +Quorum: 2""" + with self.assertRaises(ValueError) as excinfo: + corosync_healthcheck._parse_quorum_status(output) + self.assertIn("Missing 'Flags'", str(excinfo.exception)) + + def test_parse_quorum_status_invalid_int(self): + output = """Votequorum information +---------------------- +Expected votes: three +Highest expected: 3 +Total votes: 3 +Quorum: 2 +Flags: Quorate""" + with self.assertRaises(ValueError): + corosync_healthcheck._parse_quorum_status(output) + + +class TestCheckQuorumStatus(unittest.TestCase): + @mock.patch("subprocess.run") + @mock.patch("crmsh.corosync_healthcheck._parse_quorum_status") + def test_check_quorum_status_success(self, mock_parse, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 0 + mock_proc.stdout = b"some raw stdout" + mock_run.return_value = mock_proc + + mock_info = mock.MagicMock() + mock_info.expected_votes = 3 + mock_info.total_votes = 3 + mock_info.quorum = 2 + mock_info.quorate = True + mock_parse.return_value = mock_info + + result = corosync_healthcheck.check_quorum_status("ha-3-1") + self.assertEqual(result.check_name, "Check Quorum Status") + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + self.assertIsNone(result.recommended_action) + mock_parse.assert_called_once_with("some raw stdout") + + @mock.patch("subprocess.run") + @mock.patch("crmsh.corosync_healthcheck._parse_quorum_status") + def test_check_quorum_status_not_quorate(self, mock_parse, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 0 + mock_proc.stdout = b"some raw stdout" + mock_run.return_value = mock_proc + + mock_info = mock.MagicMock() + mock_info.expected_votes = 3 + mock_info.total_votes = 1 + mock_info.quorum = 2 + mock_info.quorate = False + mock_parse.return_value = mock_info + + result = corosync_healthcheck.check_quorum_status("ha-3-1") + self.assertEqual(result.check_name, "Check Quorum Status") + self.assertEqual(result.returncode, 1) + self.assertIn("The cluster is not quorate", result.result_description) + self.assertEqual(result.recommended_action, "Ensure that enough nodes are online and connected to form a quorum.") + mock_parse.assert_called_once_with("some raw stdout") + + @mock.patch("subprocess.run") + @mock.patch("crmsh.corosync_healthcheck._parse_quorum_status") + def test_check_quorum_status_votes_less_than_expected(self, mock_parse, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 0 + mock_proc.stdout = b"some raw stdout" + mock_run.return_value = mock_proc + + mock_info = mock.MagicMock() + mock_info.expected_votes = 3 + mock_info.total_votes = 2 + mock_info.quorum = 2 + mock_info.quorate = True + mock_parse.return_value = mock_info + + result = corosync_healthcheck.check_quorum_status("ha-3-1") + self.assertEqual(result.check_name, "Check Quorum Status") + self.assertEqual(result.returncode, 1) + self.assertEqual(result.result_description, "Total votes (2) is less than expected votes (3).") + self.assertEqual(result.recommended_action, "Check that all expected cluster nodes are online and connected.") + mock_parse.assert_called_once_with("some raw stdout") + + @mock.patch("subprocess.run") + def test_check_quorum_status_command_failure(self, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 1 + mock_proc.stdout = b"corosync-quorumtool: cannot connect to corosync" + mock_run.return_value = mock_proc + + result = corosync_healthcheck.check_quorum_status("ha-3-1") + self.assertEqual(result.check_name, "Check Quorum Status") + self.assertEqual(result.returncode, 1) + self.assertEqual(result.result_description, "corosync-quorumtool: cannot connect to corosync") + self.assertEqual(result.recommended_action, "Check if the corosync service is running.") + + @mock.patch("subprocess.run") + @mock.patch("crmsh.corosync_healthcheck._parse_quorum_status") + def test_check_quorum_status_parse_failure(self, mock_parse, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 0 + mock_proc.stdout = b"some raw stdout" + mock_run.return_value = mock_proc + + mock_parse.side_effect = ValueError("Missing 'Expected votes' in quorum status output") + + result = corosync_healthcheck.check_quorum_status("ha-3-1") + self.assertEqual(result.check_name, "Check Quorum Status") + self.assertEqual(result.returncode, 1) + self.assertIn("Failed to parse 'corosync-quorumtool -s' output", result.result_description) + mock_parse.assert_called_once_with("some raw stdout") + + +class TestCheckNodeIDToNodeNameMapping(unittest.TestCase): + def setUp(self): + self.local_node = "node1" + self.valid_cib_xml = """ + + + + + + +""" + + @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") + @mock.patch("crmsh.corosync.LinkManager.load_config_file") + def test_success(self, mock_load, mock_get): + mock_get.return_value = (0, self.valid_cib_xml, "") + mock_lm = mock.MagicMock() + mock_lm._config = { + "nodelist": { + "node": [ + {"nodeid": "1", "name": "node1"}, + {"nodeid": "2", "name": "node2"} + ] + } + } + mock_load.return_value = mock_lm + + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + + @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") + def test_cib_load_failure(self, mock_get): + mock_get.return_value = (1, "", "cibadmin not found") + + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") + self.assertEqual(result.returncode, 255) + self.assertIn("Failed to load CIB: cibadmin not found", result.result_description) + + @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") + def test_cib_parse_failure(self, mock_get): + mock_get.return_value = (0, "invalid xml", "") + + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") + self.assertEqual(result.returncode, 255) + self.assertIn("Failed to parse CIB", result.result_description) + + @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") + @mock.patch("crmsh.corosync.LinkManager.load_config_file") + def test_corosync_load_failure(self, mock_load, mock_get): + mock_get.return_value = (0, self.valid_cib_xml, "") + mock_load.side_effect = ValueError("corosync.conf not readable") + + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") + self.assertEqual(result.returncode, 255) + self.assertIn("Failed to load or parse corosync.conf: corosync.conf not readable", result.result_description) + + @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") + @mock.patch("crmsh.corosync.LinkManager.load_config_file") + def test_mismatch_missing_in_cib(self, mock_load, mock_get): + mock_get.return_value = (0, self.valid_cib_xml, "") + mock_lm = mock.MagicMock() + mock_lm._config = { + "nodelist": { + "node": [ + {"nodeid": "1", "name": "node1"}, + {"nodeid": "2", "name": "node2"}, + {"nodeid": "3", "name": "node3"} + ] + } + } + mock_load.return_value = mock_lm + + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") + self.assertEqual(result.returncode, 1) + self.assertIn("Node ID 3 with name 'node3' in corosync.conf is not found in CIB.", result.result_description) + + @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") + @mock.patch("crmsh.corosync.LinkManager.load_config_file") + def test_mismatch_missing_in_corosync(self, mock_load, mock_get): + mock_get.return_value = (0, self.valid_cib_xml, "") + mock_lm = mock.MagicMock() + mock_lm._config = { + "nodelist": { + "node": [ + {"nodeid": "1", "name": "node1"} + ] + } + } + mock_load.return_value = mock_lm + + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") + self.assertEqual(result.returncode, 1) + self.assertIn("Node ID 2 with name 'node2' in CIB is not found in corosync.conf.", result.result_description) + + @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") + @mock.patch("crmsh.corosync.LinkManager.load_config_file") + def test_mismatch_different_name(self, mock_load, mock_get): + mock_get.return_value = (0, self.valid_cib_xml, "") + mock_lm = mock.MagicMock() + mock_lm._config = { + "nodelist": { + "node": [ + {"nodeid": "1", "name": "node1"}, + {"nodeid": "2", "name": "node2-alt"} + ] + } + } + mock_load.return_value = mock_lm + + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") + self.assertEqual(result.returncode, 1) + self.assertIn("Node ID 2 is associated with name 'node2-alt' in corosync.conf but 'node2' in CIB.", result.result_description) + + +class TestParseLinksStatus(unittest.TestCase): + def test_parse_links_status_success_all_connected(self): + output = """Local node ID 1, transport knet +LINK ID 0 udp + addr = 192.168.122.73 + status: + nodeid: 1: localhost + nodeid: 2: connected +LINK ID 1 udp + addr = 192.168.123.73 + status: + nodeid: 1: localhost + nodeid: 2: connected""" + links = corosync_healthcheck._parse_links_status(output) + self.assertEqual(links, { + 0: {1: "localhost", 2: "connected"}, + 1: {1: "localhost", 2: "connected"} + }) + + def test_parse_links_status_success_some_disconnected(self): + output = """Local node ID 1, transport knet +LINK ID 0 udp + addr = 192.168.122.73 + status: + nodeid: 1: localhost + nodeid: 2: disconnected +LINK ID 1 udp + addr = 192.168.123.73 + status: + nodeid: 1: localhost + nodeid: 2: disconnected""" + links = corosync_healthcheck._parse_links_status(output) + self.assertEqual(links, { + 0: {1: "localhost", 2: "disconnected"}, + 1: {1: "localhost", 2: "disconnected"} + }) + + def test_parse_links_status_missing_link_id_before_nodeid(self): + output = """ nodeid: 1: localhost""" + with self.assertRaises(ValueError) as excinfo: + corosync_healthcheck._parse_links_status(output) + self.assertIn("Found node ID status before any LINK ID section", str(excinfo.exception)) + + def test_parse_links_status_no_links(self): + output = """Local node ID 1, transport knet""" + with self.assertRaises(ValueError) as excinfo: + corosync_healthcheck._parse_links_status(output) + self.assertIn("No corosync links found in output", str(excinfo.exception)) + + +class TestCheckLinksStatus(unittest.TestCase): + @mock.patch("subprocess.run") + @mock.patch("crmsh.corosync_healthcheck._parse_links_status") + def test_check_links_status_success(self, mock_parse, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 0 + mock_proc.stdout = b"some raw stdout" + mock_run.return_value = mock_proc + + mock_parse.return_value = { + 0: {1: "localhost", 2: "connected"}, + 1: {1: "localhost", 2: "connected"} + } + + result = corosync_healthcheck.check_links_status("ha-3-1") + self.assertEqual(result.check_name, "Check Corosync Links Status") + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + self.assertIsNone(result.recommended_action) + mock_parse.assert_called_once_with("some raw stdout") + + @mock.patch("subprocess.run") + @mock.patch("crmsh.corosync_healthcheck._parse_links_status") + def test_check_links_status_some_disconnected(self, mock_parse, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 0 + mock_proc.stdout = b"some raw stdout" + mock_run.return_value = mock_proc + + mock_parse.return_value = { + 0: {1: "localhost", 2: "disconnected"}, + 1: {1: "localhost", 2: "connected"} + } + + result = corosync_healthcheck.check_links_status("ha-3-1") + self.assertEqual(result.check_name, "Check Corosync Links Status") + self.assertEqual(result.returncode, 1) + self.assertIn("Corosync link(s) are not operational", result.result_description) + self.assertIn("Link 0 to node 2 has status 'disconnected'", result.result_description) + self.assertEqual(result.recommended_action, "Check network connectivity and corosync configuration/service on the affected nodes.") + mock_parse.assert_called_once_with("some raw stdout") + + @mock.patch("subprocess.run") + def test_check_links_status_command_failure(self, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 1 + mock_proc.stdout = b"corosync-cfgtool: cannot connect to corosync" + mock_run.return_value = mock_proc + + result = corosync_healthcheck.check_links_status("ha-3-1") + self.assertEqual(result.check_name, "Check Corosync Links Status") + self.assertEqual(result.returncode, 1) + self.assertEqual(result.result_description, "corosync-cfgtool: cannot connect to corosync") + self.assertEqual(result.recommended_action, "Check if the corosync service is running.") + + @mock.patch("subprocess.run") + @mock.patch("crmsh.corosync_healthcheck._parse_links_status") + def test_check_links_status_parse_failure(self, mock_parse, mock_run): + mock_proc = mock.MagicMock() + mock_proc.returncode = 0 + mock_proc.stdout = b"some raw stdout" + mock_run.return_value = mock_proc + + mock_parse.side_effect = ValueError("No corosync links found in output") + + result = corosync_healthcheck.check_links_status("ha-3-1") + self.assertEqual(result.check_name, "Check Corosync Links Status") + self.assertEqual(result.returncode, 1) + self.assertIn("Failed to parse 'corosync-cfgtool -s' output", result.result_description) + mock_parse.assert_called_once_with("some raw stdout") + + +class TestValidateConfigFileConsistency(unittest.TestCase): + @mock.patch("crmsh.corosync.conf") + @mock.patch("crmsh.prun.prun.prun") + def test_validate_config_file_consistency_success(self, mock_prun, mock_conf): + mock_conf.return_value = "/etc/corosync/corosync.conf" + from crmsh.prun.prun import ProcessResult + mock_prun.return_value = { + "node1": ProcessResult(0, b"hash1 /etc/corosync/corosync.conf\n", b""), + "node2": ProcessResult(0, b"hash1 /etc/corosync/corosync.conf\n", b"") + } + res = corosync_healthcheck.validate_config_file_consistency(["node1", "node2"]) + self.assertEqual(res.returncode, 0) + self.assertIsNone(res.result_description) + + @mock.patch("crmsh.corosync.conf") + @mock.patch("crmsh.prun.prun.prun") + def test_validate_config_file_consistency_inconsistent(self, mock_prun, mock_conf): + mock_conf.return_value = "/etc/corosync/corosync.conf" + from crmsh.prun.prun import ProcessResult + mock_prun.return_value = { + "node1": ProcessResult(0, b"hash1 /etc/corosync/corosync.conf\n", b""), + "node2": ProcessResult(0, b"hash2 /etc/corosync/corosync.conf\n", b"") + } + res = corosync_healthcheck.validate_config_file_consistency(["node1", "node2"]) + self.assertEqual(res.returncode, 1) + self.assertIn("Corosync configuration file /etc/corosync/corosync.conf is inconsistent across nodes:", res.result_description) + self.assertIn(" hash1 node1\n", res.result_description) + self.assertIn(" hash2 node2\n", res.result_description) + self.assertEqual(res.recommended_action, 'Run "crm corosync diff" to inspect the differences, and then "crm corosync push" to synchronize the configuration across cluster nodes.') + + @mock.patch("crmsh.corosync.conf") + @mock.patch("crmsh.prun.prun.prun") + def test_validate_config_file_consistency_ssh_error(self, mock_prun, mock_conf): + mock_conf.return_value = "/etc/corosync/corosync.conf" + from crmsh.prun.prun import SSHError + mock_prun.return_value = { + "node1": SSHError("root", "node1", "host unreachable") + } + res = corosync_healthcheck.validate_config_file_consistency(["node1"]) + self.assertEqual(res.returncode, 255) + self.assertEqual(res.result_description, "ssh error") + + @mock.patch("crmsh.corosync.conf") + @mock.patch("crmsh.prun.prun.prun") + def test_validate_config_file_consistency_command_error(self, mock_prun, mock_conf): + mock_conf.return_value = "/etc/corosync/corosync.conf" + from crmsh.prun.prun import ProcessResult + mock_prun.return_value = { + "node1": ProcessResult(1, b"", b"sha256sum: error") + } + res = corosync_healthcheck.validate_config_file_consistency(["node1"]) + self.assertEqual(res.returncode, 1) + self.assertEqual(res.result_description, "sha256sum error") + + +class TestValidateConfigFile(unittest.TestCase): + @mock.patch("crmsh.sh.cluster_shell") + def test_validate_config_file_success(self, mock_cluster_shell): + mock_shell_inst = mock.Mock() + mock_cluster_shell.return_value = mock_shell_inst + mock_result = mock.Mock() + mock_result.returncode = 0 + mock_result.stdout = b"success" + mock_shell_inst.subprocess_run_without_input.return_value = mock_result + + result = corosync_healthcheck.validate_config_file("node1") + self.assertEqual(result.check_name, "Validate Corosync Configuration File") + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + self.assertIsNone(result.recommended_action) + self.assertEqual(result.checked_nodes, ["node1"]) + mock_shell_inst.subprocess_run_without_input.assert_called_once_with( + "node1", "root", "corosync -t", + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + + @mock.patch("crmsh.sh.cluster_shell") + def test_validate_config_file_failure(self, mock_cluster_shell): + mock_shell_inst = mock.Mock() + mock_cluster_shell.return_value = mock_shell_inst + mock_result = mock.Mock() + mock_result.returncode = 1 + # Include an invalid UTF-8 byte to test 'replace' decoder option + mock_result.stdout = b"error in config: \xff\n" + mock_shell_inst.subprocess_run_without_input.return_value = mock_result + + result = corosync_healthcheck.validate_config_file("node1") + self.assertEqual(result.check_name, "Validate Corosync Configuration File") + self.assertEqual(result.returncode, 1) + self.assertEqual(result.result_description, "error in config: \ufffd") + self.assertIsNone(result.recommended_action) + self.assertEqual(result.checked_nodes, ["node1"]) + mock_shell_inst.subprocess_run_without_input.assert_called_once_with( + "node1", "root", "corosync -t", + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + diff --git a/test/unittests/test_ui_cluster.py b/test/unittests/test_ui_cluster.py index c2653f12a4..e00eaa4a68 100644 --- a/test/unittests/test_ui_cluster.py +++ b/test/unittests/test_ui_cluster.py @@ -1,11 +1,13 @@ import logging import unittest +from io import StringIO try: from unittest import mock except ImportError: import mock from crmsh import ui_cluster +from crmsh import corosync_healthcheck class TestCluster(unittest.TestCase): @@ -179,3 +181,137 @@ def test_node_ready_to_stop_cluster_service(self, mock_service_manager, mock_inf mock.call("pacemaker.service", remote_addr="node1"), ]) mock_info.assert_not_called() + + @mock.patch('crmsh.utils.this_node') + @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') + @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') + @mock.patch('crmsh.corosync_healthcheck.check_links_status') + @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') + def test_do_health_corosync_success(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node): + mock_this_node.return_value = "node1" + mock_nodes.return_value = ["node1", "node2"] + mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) + mock_consistency.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File Consistency", ["node1", "node2"], 0, None, None) + mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) + mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) + mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + + res = self.ui_cluster_inst.do_health(None, "corosync") + self.assertTrue(res) + + mock_this_node.assert_called_once() + mock_validate.assert_called_once_with("node1") + mock_consistency.assert_called_once_with(["node1", "node2"]) + mock_quorum.assert_called_once_with("node1") + mock_links.assert_called_once_with("node1") + mock_mapping.assert_called_once_with("node1") + + @mock.patch('crmsh.utils.this_node') + @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') + @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') + @mock.patch('crmsh.corosync_healthcheck.check_links_status') + @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') + def test_do_health_corosync_local_fail(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node): + mock_this_node.return_value = "node1" + mock_nodes.return_value = ["node1", "node2"] + mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 1, "config file is invalid", "Fix it") + + res = self.ui_cluster_inst.do_health(None, "corosync") + self.assertFalse(res) + + mock_this_node.assert_called_once() + mock_validate.assert_called_once_with("node1") + mock_consistency.assert_not_called() + mock_quorum.assert_not_called() + mock_links.assert_not_called() + mock_mapping.assert_not_called() + + @mock.patch('sys.stdout', new_callable=StringIO) + @mock.patch('crmsh.utils.this_node') + @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') + @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') + @mock.patch('crmsh.corosync_healthcheck.check_links_status') + @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') + def test_do_health_corosync_json(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node, mock_stdout): + mock_this_node.return_value = "node1" + mock_nodes.return_value = ["node1", "node2"] + mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) + mock_consistency.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File Consistency", ["node1", "node2"], 1, "checksum error", None) + mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) + mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) + mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + + res = self.ui_cluster_inst.do_health(None, "corosync", "--json") + self.assertFalse(res) + + output = mock_stdout.getvalue() + import json + parsed = json.loads(output) + self.assertEqual(parsed["returncode"], 1) + self.assertEqual(len(parsed["results"]), 5) + self.assertEqual(parsed["results"][0]["check_name"], "Validate Corosync Configuration File") + self.assertEqual(parsed["results"][1]["returncode"], 1) + + @mock.patch('crmsh.utils.this_node') + @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') + @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') + @mock.patch('crmsh.corosync_healthcheck.check_links_status') + @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') + def test_do_health_corosync_local(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node): + mock_this_node.return_value = "node1" + mock_nodes.return_value = ["node1", "node2"] + mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) + mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) + mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) + mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + + res = self.ui_cluster_inst.do_health(None, "corosync", "--local") + self.assertTrue(res) + + mock_this_node.assert_called_once() + mock_validate.assert_called_once_with("node1") + mock_consistency.assert_not_called() + mock_quorum.assert_called_once_with("node1") + mock_links.assert_called_once_with("node1") + mock_mapping.assert_called_once_with("node1") + + @mock.patch('sys.stdout', new_callable=StringIO) + @mock.patch('crmsh.utils.this_node') + @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file') + @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') + @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') + @mock.patch('crmsh.corosync_healthcheck.check_links_status') + @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') + def test_do_health_corosync_local_json(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node, mock_stdout): + mock_this_node.return_value = "node1" + mock_nodes.return_value = ["node1", "node2"] + mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) + mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) + mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) + mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + + res = self.ui_cluster_inst.do_health(None, "corosync", "--local", "--json") + self.assertTrue(res) + + mock_this_node.assert_called_once() + mock_validate.assert_called_once_with("node1") + mock_consistency.assert_not_called() + mock_quorum.assert_called_once_with("node1") + mock_links.assert_called_once_with("node1") + mock_mapping.assert_called_once_with("node1") + + output = mock_stdout.getvalue() + import json + parsed = json.loads(output) + self.assertEqual(parsed["returncode"], 0) + self.assertEqual(len(parsed["results"]), 4) + self.assertEqual(parsed["results"][0]["check_name"], "Validate Corosync Configuration File") From a2ec178c3132613371b7e4cf03eeedc72dd9055a Mon Sep 17 00:00:00 2001 From: nicholasyang Date: Tue, 11 Aug 2026 21:50:57 +0800 Subject: [PATCH 2/3] Dev: corosync: Warn on non-knet transport in health check Add check_deprecated_transport in health check to give a warning when corosync uses a deprecated non-knet transport (such as udp or udpu). Load LinkManager once in ui_cluster.py for corosync health checks and pass it to check_nodeid_to_nodename_mapping and check_deprecated_transport. --- crmsh/corosync_healthcheck.py | 48 ++++++++++------ crmsh/ui_cluster.py | 13 ++++- test/unittests/test_corosync_healthcheck.py | 63 +++++++++++---------- test/unittests/test_ui_cluster.py | 46 +++++++++++---- 4 files changed, 114 insertions(+), 56 deletions(-) diff --git a/crmsh/corosync_healthcheck.py b/crmsh/corosync_healthcheck.py index fc41eb34e0..e49ba93883 100644 --- a/crmsh/corosync_healthcheck.py +++ b/crmsh/corosync_healthcheck.py @@ -221,7 +221,7 @@ def _parse_quorum_status(output: str) -> _QuorumInfo: ) -def check_nodeid_to_nodename_mapping(local_node: str) -> CheckResult: +def check_nodeid_to_nodename_mapping(local_node: str, lm: corosync.LinkManager) -> CheckResult: """ Check against the mismatch of nodeid to nodename mapping between corosync.conf and CIB. """ @@ -257,21 +257,11 @@ def check_nodeid_to_nodename_mapping(local_node: str) -> CheckResult: ) try: - lm = corosync.LinkManager.load_config_file() - try: - config_nodelist = lm._config['nodelist']['node'] - except KeyError: - config_nodelist = [] - if not isinstance(config_nodelist, list): - config_nodelist = [config_nodelist] - except ValueError as e: - return CheckResult( - CHECK_NAME, - [local_node], - 255, - f"Failed to load or parse corosync.conf: {e}", - None, - ) + config_nodelist = lm._config['nodelist']['node'] + except KeyError: + config_nodelist = [] + if not isinstance(config_nodelist, list): + config_nodelist = [config_nodelist] corosync_id_to_name = {} for node in config_nodelist: nodeid_str = node.get('nodeid') @@ -412,3 +402,29 @@ def _parse_links_status(output: str) -> dict[int, dict[int, str]]: return links +def check_deprecated_transport(local_node: str, lm: corosync.LinkManager) -> CheckResult: + """ + Check if corosync transport is knet. Give a warning if non-knet transport is used. + """ + CHECK_NAME = "Check Deprecated Corosync Transport" + transport = lm.totem_transport() + + if transport != 'knet': + return CheckResult( + CHECK_NAME, + [local_node], + 2, + f'Corosync transport "{transport}" is deprecated. Please use knet.', + "Upgrade corosync transport to knet.", + ) + + return CheckResult( + CHECK_NAME, + [local_node], + 0, + None, + None, + ) + + + diff --git a/crmsh/ui_cluster.py b/crmsh/ui_cluster.py index bfe8a3643c..0364519262 100644 --- a/crmsh/ui_cluster.py +++ b/crmsh/ui_cluster.py @@ -912,12 +912,23 @@ def do_health(self, context, *args): print_cb(res_local) if res_local.returncode == 0: + try: + lm = corosync.LinkManager.load_config_file() + except ValueError as e: + # unlikely, as we just validated the config file in the previous check + logger.error("Failed to load or parse corosync.conf: %s", e) + return False + if not corosync_args.local: nodes = utils.list_cluster_nodes() or [local_node] res_consistency = corosync_healthcheck.validate_config_file_consistency(nodes) results.append(res_consistency) print_cb(res_consistency) + res_transport = corosync_healthcheck.check_deprecated_transport(local_node, lm) + results.append(res_transport) + print_cb(res_transport) + res_quorum = corosync_healthcheck.check_quorum_status(local_node) results.append(res_quorum) print_cb(res_quorum) @@ -926,7 +937,7 @@ def do_health(self, context, *args): results.append(res_links) print_cb(res_links) - res_mapping = corosync_healthcheck.check_nodeid_to_nodename_mapping(local_node) + res_mapping = corosync_healthcheck.check_nodeid_to_nodename_mapping(local_node, lm) results.append(res_mapping) print_cb(res_mapping) diff --git a/test/unittests/test_corosync_healthcheck.py b/test/unittests/test_corosync_healthcheck.py index 6ed4552f1d..07fe2cbbbd 100644 --- a/test/unittests/test_corosync_healthcheck.py +++ b/test/unittests/test_corosync_healthcheck.py @@ -268,8 +268,7 @@ def setUp(self): """ @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") - @mock.patch("crmsh.corosync.LinkManager.load_config_file") - def test_success(self, mock_load, mock_get): + def test_success(self, mock_get): mock_get.return_value = (0, self.valid_cib_xml, "") mock_lm = mock.MagicMock() mock_lm._config = { @@ -280,9 +279,8 @@ def test_success(self, mock_load, mock_get): ] } } - mock_load.return_value = mock_lm - result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node, mock_lm) self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") self.assertEqual(result.returncode, 0) self.assertIsNone(result.result_description) @@ -290,8 +288,9 @@ def test_success(self, mock_load, mock_get): @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") def test_cib_load_failure(self, mock_get): mock_get.return_value = (1, "", "cibadmin not found") + mock_lm = mock.MagicMock() - result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node, mock_lm) self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") self.assertEqual(result.returncode, 255) self.assertIn("Failed to load CIB: cibadmin not found", result.result_description) @@ -299,26 +298,15 @@ def test_cib_load_failure(self, mock_get): @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") def test_cib_parse_failure(self, mock_get): mock_get.return_value = (0, "invalid xml", "") + mock_lm = mock.MagicMock() - result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node, mock_lm) self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") self.assertEqual(result.returncode, 255) self.assertIn("Failed to parse CIB", result.result_description) @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") - @mock.patch("crmsh.corosync.LinkManager.load_config_file") - def test_corosync_load_failure(self, mock_load, mock_get): - mock_get.return_value = (0, self.valid_cib_xml, "") - mock_load.side_effect = ValueError("corosync.conf not readable") - - result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) - self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") - self.assertEqual(result.returncode, 255) - self.assertIn("Failed to load or parse corosync.conf: corosync.conf not readable", result.result_description) - - @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") - @mock.patch("crmsh.corosync.LinkManager.load_config_file") - def test_mismatch_missing_in_cib(self, mock_load, mock_get): + def test_mismatch_missing_in_cib(self, mock_get): mock_get.return_value = (0, self.valid_cib_xml, "") mock_lm = mock.MagicMock() mock_lm._config = { @@ -330,16 +318,14 @@ def test_mismatch_missing_in_cib(self, mock_load, mock_get): ] } } - mock_load.return_value = mock_lm - result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node, mock_lm) self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") self.assertEqual(result.returncode, 1) self.assertIn("Node ID 3 with name 'node3' in corosync.conf is not found in CIB.", result.result_description) @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") - @mock.patch("crmsh.corosync.LinkManager.load_config_file") - def test_mismatch_missing_in_corosync(self, mock_load, mock_get): + def test_mismatch_missing_in_corosync(self, mock_get): mock_get.return_value = (0, self.valid_cib_xml, "") mock_lm = mock.MagicMock() mock_lm._config = { @@ -349,16 +335,14 @@ def test_mismatch_missing_in_corosync(self, mock_load, mock_get): ] } } - mock_load.return_value = mock_lm - result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node, mock_lm) self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") self.assertEqual(result.returncode, 1) self.assertIn("Node ID 2 with name 'node2' in CIB is not found in corosync.conf.", result.result_description) @mock.patch("crmsh.sh.ShellUtils.get_stdout_stderr") - @mock.patch("crmsh.corosync.LinkManager.load_config_file") - def test_mismatch_different_name(self, mock_load, mock_get): + def test_mismatch_different_name(self, mock_get): mock_get.return_value = (0, self.valid_cib_xml, "") mock_lm = mock.MagicMock() mock_lm._config = { @@ -369,9 +353,8 @@ def test_mismatch_different_name(self, mock_load, mock_get): ] } } - mock_load.return_value = mock_lm - result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node) + result = corosync_healthcheck.check_nodeid_to_nodename_mapping(self.local_node, mock_lm) self.assertEqual(result.check_name, "Check Node ID to Node Name Mapping") self.assertEqual(result.returncode, 1) self.assertIn("Node ID 2 is associated with name 'node2-alt' in corosync.conf but 'node2' in CIB.", result.result_description) @@ -598,3 +581,25 @@ def test_validate_config_file_failure(self, mock_cluster_shell): stderr=subprocess.STDOUT ) + +class TestCheckDeprecatedTransport(unittest.TestCase): + def test_check_deprecated_transport_knet(self): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + result = corosync_healthcheck.check_deprecated_transport("node1", mock_lm) + self.assertEqual(result.check_name, "Check Deprecated Corosync Transport") + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + self.assertIsNone(result.recommended_action) + + def test_check_deprecated_transport_non_knet(self): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "udp" + + result = corosync_healthcheck.check_deprecated_transport("node1", mock_lm) + self.assertEqual(result.check_name, "Check Deprecated Corosync Transport") + self.assertEqual(result.returncode, 2) + self.assertIn('Corosync transport "udp" is deprecated. Please use knet.', result.result_description) + self.assertEqual(result.recommended_action, "Upgrade corosync transport to knet.") + diff --git a/test/unittests/test_ui_cluster.py b/test/unittests/test_ui_cluster.py index e00eaa4a68..bb58236608 100644 --- a/test/unittests/test_ui_cluster.py +++ b/test/unittests/test_ui_cluster.py @@ -184,19 +184,24 @@ def test_node_ready_to_stop_cluster_service(self, mock_service_manager, mock_inf @mock.patch('crmsh.utils.this_node') @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync.LinkManager.load_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') - def test_do_health_corosync_success(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node): + @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') + def test_do_health_corosync_success(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] + mock_lm = mock.MagicMock() + mock_lm_load.return_value = mock_lm mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) mock_consistency.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File Consistency", ["node1", "node2"], 0, None, None) mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync") self.assertTrue(res) @@ -206,7 +211,8 @@ def test_do_health_corosync_success(self, mock_mapping, mock_links, mock_quorum, mock_consistency.assert_called_once_with(["node1", "node2"]) mock_quorum.assert_called_once_with("node1") mock_links.assert_called_once_with("node1") - mock_mapping.assert_called_once_with("node1") + mock_mapping.assert_called_once_with("node1", mock_lm) + mock_transport.assert_called_once_with("node1", mock_lm) @mock.patch('crmsh.utils.this_node') @mock.patch('crmsh.utils.list_cluster_nodes') @@ -215,7 +221,8 @@ def test_do_health_corosync_success(self, mock_mapping, mock_links, mock_quorum, @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') - def test_do_health_corosync_local_fail(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node): + @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') + def test_do_health_corosync_local_fail(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 1, "config file is invalid", "Fix it") @@ -229,23 +236,29 @@ def test_do_health_corosync_local_fail(self, mock_mapping, mock_links, mock_quor mock_quorum.assert_not_called() mock_links.assert_not_called() mock_mapping.assert_not_called() + mock_transport.assert_not_called() @mock.patch('sys.stdout', new_callable=StringIO) @mock.patch('crmsh.utils.this_node') @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync.LinkManager.load_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') - def test_do_health_corosync_json(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node, mock_stdout): + @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') + def test_do_health_corosync_json(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node, mock_stdout): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] + mock_lm = mock.MagicMock() + mock_lm_load.return_value = mock_lm mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) mock_consistency.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File Consistency", ["node1", "node2"], 1, "checksum error", None) mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync", "--json") self.assertFalse(res) @@ -254,24 +267,30 @@ def test_do_health_corosync_json(self, mock_mapping, mock_links, mock_quorum, mo import json parsed = json.loads(output) self.assertEqual(parsed["returncode"], 1) - self.assertEqual(len(parsed["results"]), 5) + self.assertEqual(len(parsed["results"]), 6) self.assertEqual(parsed["results"][0]["check_name"], "Validate Corosync Configuration File") + self.assertEqual(parsed["results"][1]["check_name"], "Validate Corosync Configuration File Consistency") self.assertEqual(parsed["results"][1]["returncode"], 1) @mock.patch('crmsh.utils.this_node') @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync.LinkManager.load_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') - def test_do_health_corosync_local(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node): + @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') + def test_do_health_corosync_local(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] + mock_lm = mock.MagicMock() + mock_lm_load.return_value = mock_lm mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync", "--local") self.assertTrue(res) @@ -281,23 +300,29 @@ def test_do_health_corosync_local(self, mock_mapping, mock_links, mock_quorum, m mock_consistency.assert_not_called() mock_quorum.assert_called_once_with("node1") mock_links.assert_called_once_with("node1") - mock_mapping.assert_called_once_with("node1") + mock_mapping.assert_called_once_with("node1", mock_lm) + mock_transport.assert_called_once_with("node1", mock_lm) @mock.patch('sys.stdout', new_callable=StringIO) @mock.patch('crmsh.utils.this_node') @mock.patch('crmsh.utils.list_cluster_nodes') + @mock.patch('crmsh.corosync.LinkManager.load_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file') @mock.patch('crmsh.corosync_healthcheck.validate_config_file_consistency') @mock.patch('crmsh.corosync_healthcheck.check_quorum_status') @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') - def test_do_health_corosync_local_json(self, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_nodes, mock_this_node, mock_stdout): + @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') + def test_do_health_corosync_local_json(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node, mock_stdout): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] + mock_lm = mock.MagicMock() + mock_lm_load.return_value = mock_lm mock_validate.return_value = corosync_healthcheck.CheckResult("Validate Corosync Configuration File", ["node1"], 0, None, None) mock_quorum.return_value = corosync_healthcheck.CheckResult("Check Quorum Status", ["node1"], 0, None, None) mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) + mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync", "--local", "--json") self.assertTrue(res) @@ -307,11 +332,12 @@ def test_do_health_corosync_local_json(self, mock_mapping, mock_links, mock_quor mock_consistency.assert_not_called() mock_quorum.assert_called_once_with("node1") mock_links.assert_called_once_with("node1") - mock_mapping.assert_called_once_with("node1") + mock_mapping.assert_called_once_with("node1", mock_lm) + mock_transport.assert_called_once_with("node1", mock_lm) output = mock_stdout.getvalue() import json parsed = json.loads(output) self.assertEqual(parsed["returncode"], 0) - self.assertEqual(len(parsed["results"]), 4) + self.assertEqual(len(parsed["results"]), 5) self.assertEqual(parsed["results"][0]["check_name"], "Validate Corosync Configuration File") From f9d5fb2aa01fc7d8f8da114aa23008e4e7104fe0 Mon Sep 17 00:00:00 2001 From: nicholasyang Date: Fri, 28 Aug 2026 11:21:43 +0800 Subject: [PATCH 3/3] feat(corosync_health): check that multiple knet links are on different network interfaces Add check_knet_link_network_interface in health checks to verify that when multiple knet links are configured for redundancy, they reside on different network interfaces. - Check knet link address from corosync.conf against network interface listed by `ip -j addr show`. - Warn if multiple links share the same local network interface. - Update tests and mocks in test_corosync_healthcheck.py and test_ui_cluster.py. --- crmsh/corosync_healthcheck.py | 92 ++++++ crmsh/ui_cluster.py | 4 + test/unittests/test_corosync_healthcheck.py | 303 ++++++++++++++++++++ test/unittests/test_ui_cluster.py | 23 +- 4 files changed, 416 insertions(+), 6 deletions(-) diff --git a/crmsh/corosync_healthcheck.py b/crmsh/corosync_healthcheck.py index e49ba93883..4c08741cbb 100644 --- a/crmsh/corosync_healthcheck.py +++ b/crmsh/corosync_healthcheck.py @@ -1,5 +1,7 @@ import dataclasses import functools +import ipaddress +import json import logging import re import shlex @@ -13,6 +15,7 @@ from . import corosync from . import cibquery from . import constants +from . import iproute2 from .prun import prun from . import sh @@ -427,4 +430,93 @@ def check_deprecated_transport(local_node: str, lm: corosync.LinkManager) -> Che ) +def check_knet_link_network_interface(local_node: str, lm: corosync.LinkManager) -> CheckResult: + """ + Check if multiple knet links are configured on different network interfaces. + """ + CHECK_NAME = "Check Knet Link Network Interface" + if lm.totem_transport() != 'knet': + return CheckResult( + CHECK_NAME, + [local_node], + 0, + None, + None, + ) + + link_addrs: dict[int, str] = {} # Maps link number to any configured node address on that link + for link in lm.links(): + if link is None: + continue + for node in link.nodes: + if node.addr: + link_addrs[link.linknumber] = node.addr + break + + if len(link_addrs) <= 1: + return CheckResult( + CHECK_NAME, + [local_node], + 0, + None, + None, + ) + + try: + ip_j_addr = sh.LocalShell().get_stdout_or_raise_error(None, 'ip -j addr') + local_interfaces = iproute2.IPAddr(json.loads(ip_j_addr)).interfaces() + except Exception as e: + return CheckResult( + CHECK_NAME, + [local_node], + 1, + f"Failed to load local network interfaces: {e}", + None, + ) + + interface_to_links: dict[str, list[int]] = {} # Maps local network interface name to configured link numbers + for linknumber, addr_str in link_addrs.items(): + try: + link_ip = ipaddress.ip_address(addr_str) + except ValueError: + continue + + found_iface = None + for iface in local_interfaces: + for addr_info in iface.addr_info: + if link_ip in addr_info.network: + found_iface = iface.ifname + break + if found_iface is not None: + break + + if found_iface is not None: + interface_to_links.setdefault(found_iface, []).append(linknumber) + + conflicts: dict[str, list[int]] = { + iface: links for iface, links in interface_to_links.items() if len(links) > 1 + } # Identifies interfaces hosting multiple knet links (non-redundant) + + if conflicts: + result_description = StringIO() + result_description.write("Multiple knet links are configured on the same network interface:\n") + for iface, links in conflicts.items(): + result_description.write(f" Interface '{iface}' hosts knet links: {', '.join(map(str, links))}\n") + return CheckResult( + CHECK_NAME, + [local_node], + 1, + result_description.getvalue().strip(), + "To ensure network redundancy, configure multiple knet links on different network interfaces.", + ) + + return CheckResult( + CHECK_NAME, + [local_node], + 0, + None, + None, + ) + + diff --git a/crmsh/ui_cluster.py b/crmsh/ui_cluster.py index 0364519262..0c9b31ffed 100644 --- a/crmsh/ui_cluster.py +++ b/crmsh/ui_cluster.py @@ -929,6 +929,10 @@ def do_health(self, context, *args): results.append(res_transport) print_cb(res_transport) + res_knet_interface = corosync_healthcheck.check_knet_link_network_interface(local_node, lm) + results.append(res_knet_interface) + print_cb(res_knet_interface) + res_quorum = corosync_healthcheck.check_quorum_status(local_node) results.append(res_quorum) print_cb(res_quorum) diff --git a/test/unittests/test_corosync_healthcheck.py b/test/unittests/test_corosync_healthcheck.py index 07fe2cbbbd..6c7496e689 100644 --- a/test/unittests/test_corosync_healthcheck.py +++ b/test/unittests/test_corosync_healthcheck.py @@ -603,3 +603,306 @@ def test_check_deprecated_transport_non_knet(self): self.assertIn('Corosync transport "udp" is deprecated. Please use knet.', result.result_description) self.assertEqual(result.recommended_action, "Upgrade corosync transport to knet.") + +class TestCheckKnetLinkNetworkInterface(unittest.TestCase): + def test_check_knet_link_network_interface_non_knet(self): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "udpu" + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.check_name, "Check Knet Link Network Interface") + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + + def test_check_knet_link_network_interface_one_link(self): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + mock_link = mock.MagicMock() + mock_link.linknumber = 0 + mock_node = mock.MagicMock() + mock_node.name = "node1" + mock_node.addr = "192.168.0.3" + mock_link.nodes = [mock_node] + + mock_lm.links.return_value = [mock_link] + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + + @mock.patch("crmsh.sh.LocalShell.get_stdout_or_raise_error") + def test_check_knet_link_network_interface_success(self, mock_get_stdout): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + mock_link1 = mock.MagicMock() + mock_link1.linknumber = 0 + mock_node1 = mock.MagicMock() + mock_node1.name = "node1" + mock_node1.addr = "192.168.0.3" + mock_link1.nodes = [mock_node1] + + mock_link2 = mock.MagicMock() + mock_link2.linknumber = 1 + mock_node2 = mock.MagicMock() + mock_node2.name = "node1" + mock_node2.addr = "192.168.1.3" + mock_link2.nodes = [mock_node2] + + mock_lm.links.return_value = [mock_link1, mock_link2] + + mock_get_stdout.return_value = """[ + { + "ifname": "eth0", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.0.3", + "prefixlen": 24 + } + ] + }, + { + "ifname": "eth1", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.1.3", + "prefixlen": 24 + } + ] + } + ]""" + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + + @mock.patch("crmsh.sh.LocalShell.get_stdout_or_raise_error") + def test_check_knet_link_network_interface_conflict(self, mock_get_stdout): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + mock_link1 = mock.MagicMock() + mock_link1.linknumber = 0 + mock_node1 = mock.MagicMock() + mock_node1.name = "node1" + mock_node1.addr = "192.168.0.3" + mock_link1.nodes = [mock_node1] + + mock_link2 = mock.MagicMock() + mock_link2.linknumber = 1 + mock_node2 = mock.MagicMock() + mock_node2.name = "node1" + mock_node2.addr = "192.168.0.4" + mock_link2.nodes = [mock_node2] + + mock_lm.links.return_value = [mock_link1, mock_link2] + + mock_get_stdout.return_value = """[ + { + "ifname": "eth0", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.0.3", + "prefixlen": 24 + }, + { + "local": "192.168.0.4", + "prefixlen": 24 + } + ] + } + ]""" + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.returncode, 1) + self.assertIn("Multiple knet links are configured on the same network interface:", result.result_description) + self.assertIn("eth0", result.result_description) + self.assertIn("0, 1", result.result_description) + + @mock.patch("crmsh.sh.LocalShell.get_stdout_or_raise_error") + def test_check_knet_link_network_interface_conflict_different_subnets(self, mock_get_stdout): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + mock_link1 = mock.MagicMock() + mock_link1.linknumber = 0 + mock_node1 = mock.MagicMock() + mock_node1.name = "node1" + mock_node1.addr = "192.168.0.3" + mock_link1.nodes = [mock_node1] + + mock_link2 = mock.MagicMock() + mock_link2.linknumber = 1 + mock_node2 = mock.MagicMock() + mock_node2.name = "node1" + mock_node2.addr = "192.168.10.3" + mock_link2.nodes = [mock_node2] + + mock_lm.links.return_value = [mock_link1, mock_link2] + + mock_get_stdout.return_value = """[ + { + "ifname": "eth0", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.0.3", + "prefixlen": 24 + }, + { + "local": "192.168.10.3", + "prefixlen": 24 + } + ] + } + ]""" + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.returncode, 1) + self.assertIn("Multiple knet links are configured on the same network interface:", result.result_description) + self.assertIn("eth0", result.result_description) + self.assertIn("0, 1", result.result_description) + + @mock.patch("crmsh.sh.LocalShell.get_stdout_or_raise_error") + def test_check_knet_link_network_interface_conflict_mixed_interfaces(self, mock_get_stdout): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + mock_link1 = mock.MagicMock() + mock_link1.linknumber = 0 + mock_node1 = mock.MagicMock() + mock_node1.name = "node1" + mock_node1.addr = "192.168.0.3" + mock_link1.nodes = [mock_node1] + + mock_link2 = mock.MagicMock() + mock_link2.linknumber = 1 + mock_node2 = mock.MagicMock() + mock_node2.name = "node1" + mock_node2.addr = "192.168.10.3" + mock_link2.nodes = [mock_node2] + + mock_link3 = mock.MagicMock() + mock_link3.linknumber = 2 + mock_node3 = mock.MagicMock() + mock_node3.name = "node1" + mock_node3.addr = "192.168.20.3" + mock_link3.nodes = [mock_node3] + + mock_lm.links.return_value = [mock_link1, mock_link2, mock_link3] + + mock_get_stdout.return_value = """[ + { + "ifname": "eth0", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.0.3", + "prefixlen": 24 + }, + { + "local": "192.168.10.3", + "prefixlen": 24 + } + ] + }, + { + "ifname": "eth1", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.20.3", + "prefixlen": 24 + } + ] + } + ]""" + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.returncode, 1) + self.assertIn("Multiple knet links are configured on the same network interface:", result.result_description) + self.assertIn("eth0", result.result_description) + self.assertIn("0, 1", result.result_description) + self.assertNotIn("eth1", result.result_description) + + @mock.patch("crmsh.sh.LocalShell.get_stdout_or_raise_error") + def test_check_knet_link_network_interface_shell_error(self, mock_get_stdout): + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + mock_link1 = mock.MagicMock() + mock_link1.linknumber = 0 + mock_node1 = mock.MagicMock() + mock_node1.name = "node1" + mock_node1.addr = "192.168.0.3" + mock_link1.nodes = [mock_node1] + + mock_link2 = mock.MagicMock() + mock_link2.linknumber = 1 + mock_node2 = mock.MagicMock() + mock_node2.name = "node1" + mock_node2.addr = "192.168.1.3" + mock_link2.nodes = [mock_node2] + + mock_lm.links.return_value = [mock_link1, mock_link2] + + mock_get_stdout.side_effect = ValueError("ip command not found") + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.returncode, 1) + self.assertIn("Failed to load local network interfaces", result.result_description) + + @mock.patch("crmsh.sh.LocalShell.get_stdout_or_raise_error") + def test_check_knet_link_network_interface_other_node_ip(self, mock_get_stdout): + # Even if the link configuration doesn't specify node1's IP or node1 is missing from node.addr, + # we check the subnet using any node's IP configured on the link. + mock_lm = mock.MagicMock() + mock_lm.totem_transport.return_value = "knet" + + mock_link1 = mock.MagicMock() + mock_link1.linknumber = 0 + mock_node_other1 = mock.MagicMock() + mock_node_other1.name = "node2" + mock_node_other1.addr = "192.168.0.4" + mock_link1.nodes = [mock_node_other1] + + mock_link2 = mock.MagicMock() + mock_link2.linknumber = 1 + mock_node_other2 = mock.MagicMock() + mock_node_other2.name = "node2" + mock_node_other2.addr = "192.168.1.4" + mock_link2.nodes = [mock_node_other2] + + mock_lm.links.return_value = [mock_link1, mock_link2] + + mock_get_stdout.return_value = """[ + { + "ifname": "eth0", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.0.3", + "prefixlen": 24 + } + ] + }, + { + "ifname": "eth1", + "flags": ["UP"], + "addr_info": [ + { + "local": "192.168.1.3", + "prefixlen": 24 + } + ] + } + ]""" + + result = corosync_healthcheck.check_knet_link_network_interface("node1", mock_lm) + self.assertEqual(result.returncode, 0) + self.assertIsNone(result.result_description) + diff --git a/test/unittests/test_ui_cluster.py b/test/unittests/test_ui_cluster.py index bb58236608..c268e7f2e0 100644 --- a/test/unittests/test_ui_cluster.py +++ b/test/unittests/test_ui_cluster.py @@ -191,7 +191,8 @@ def test_node_ready_to_stop_cluster_service(self, mock_service_manager, mock_inf @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') - def test_do_health_corosync_success(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node): + @mock.patch('crmsh.corosync_healthcheck.check_knet_link_network_interface') + def test_do_health_corosync_success(self, mock_knet_interface, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] mock_lm = mock.MagicMock() @@ -202,6 +203,7 @@ def test_do_health_corosync_success(self, mock_transport, mock_mapping, mock_lin mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) + mock_knet_interface.return_value = corosync_healthcheck.CheckResult("Check Knet Link Network Interface", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync") self.assertTrue(res) @@ -213,6 +215,7 @@ def test_do_health_corosync_success(self, mock_transport, mock_mapping, mock_lin mock_links.assert_called_once_with("node1") mock_mapping.assert_called_once_with("node1", mock_lm) mock_transport.assert_called_once_with("node1", mock_lm) + mock_knet_interface.assert_called_once_with("node1", mock_lm) @mock.patch('crmsh.utils.this_node') @mock.patch('crmsh.utils.list_cluster_nodes') @@ -248,7 +251,8 @@ def test_do_health_corosync_local_fail(self, mock_transport, mock_mapping, mock_ @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') - def test_do_health_corosync_json(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node, mock_stdout): + @mock.patch('crmsh.corosync_healthcheck.check_knet_link_network_interface') + def test_do_health_corosync_json(self, mock_knet_interface, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node, mock_stdout): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] mock_lm = mock.MagicMock() @@ -259,6 +263,7 @@ def test_do_health_corosync_json(self, mock_transport, mock_mapping, mock_links, mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) + mock_knet_interface.return_value = corosync_healthcheck.CheckResult("Check Knet Link Network Interface", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync", "--json") self.assertFalse(res) @@ -267,7 +272,7 @@ def test_do_health_corosync_json(self, mock_transport, mock_mapping, mock_links, import json parsed = json.loads(output) self.assertEqual(parsed["returncode"], 1) - self.assertEqual(len(parsed["results"]), 6) + self.assertEqual(len(parsed["results"]), 7) self.assertEqual(parsed["results"][0]["check_name"], "Validate Corosync Configuration File") self.assertEqual(parsed["results"][1]["check_name"], "Validate Corosync Configuration File Consistency") self.assertEqual(parsed["results"][1]["returncode"], 1) @@ -281,7 +286,8 @@ def test_do_health_corosync_json(self, mock_transport, mock_mapping, mock_links, @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') - def test_do_health_corosync_local(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node): + @mock.patch('crmsh.corosync_healthcheck.check_knet_link_network_interface') + def test_do_health_corosync_local(self, mock_knet_interface, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] mock_lm = mock.MagicMock() @@ -291,6 +297,7 @@ def test_do_health_corosync_local(self, mock_transport, mock_mapping, mock_links mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) + mock_knet_interface.return_value = corosync_healthcheck.CheckResult("Check Knet Link Network Interface", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync", "--local") self.assertTrue(res) @@ -302,6 +309,7 @@ def test_do_health_corosync_local(self, mock_transport, mock_mapping, mock_links mock_links.assert_called_once_with("node1") mock_mapping.assert_called_once_with("node1", mock_lm) mock_transport.assert_called_once_with("node1", mock_lm) + mock_knet_interface.assert_called_once_with("node1", mock_lm) @mock.patch('sys.stdout', new_callable=StringIO) @mock.patch('crmsh.utils.this_node') @@ -313,7 +321,8 @@ def test_do_health_corosync_local(self, mock_transport, mock_mapping, mock_links @mock.patch('crmsh.corosync_healthcheck.check_links_status') @mock.patch('crmsh.corosync_healthcheck.check_nodeid_to_nodename_mapping') @mock.patch('crmsh.corosync_healthcheck.check_deprecated_transport') - def test_do_health_corosync_local_json(self, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node, mock_stdout): + @mock.patch('crmsh.corosync_healthcheck.check_knet_link_network_interface') + def test_do_health_corosync_local_json(self, mock_knet_interface, mock_transport, mock_mapping, mock_links, mock_quorum, mock_consistency, mock_validate, mock_lm_load, mock_nodes, mock_this_node, mock_stdout): mock_this_node.return_value = "node1" mock_nodes.return_value = ["node1", "node2"] mock_lm = mock.MagicMock() @@ -323,6 +332,7 @@ def test_do_health_corosync_local_json(self, mock_transport, mock_mapping, mock_ mock_links.return_value = corosync_healthcheck.CheckResult("Check Corosync Links Status", ["node1"], 0, None, None) mock_mapping.return_value = corosync_healthcheck.CheckResult("Check Node ID to Node Name Mapping", ["node1"], 0, None, None) mock_transport.return_value = corosync_healthcheck.CheckResult("Check Deprecated Corosync Transport", ["node1"], 0, None, None) + mock_knet_interface.return_value = corosync_healthcheck.CheckResult("Check Knet Link Network Interface", ["node1"], 0, None, None) res = self.ui_cluster_inst.do_health(None, "corosync", "--local", "--json") self.assertTrue(res) @@ -334,10 +344,11 @@ def test_do_health_corosync_local_json(self, mock_transport, mock_mapping, mock_ mock_links.assert_called_once_with("node1") mock_mapping.assert_called_once_with("node1", mock_lm) mock_transport.assert_called_once_with("node1", mock_lm) + mock_knet_interface.assert_called_once_with("node1", mock_lm) output = mock_stdout.getvalue() import json parsed = json.loads(output) self.assertEqual(parsed["returncode"], 0) - self.assertEqual(len(parsed["results"]), 5) + self.assertEqual(len(parsed["results"]), 6) self.assertEqual(parsed["results"][0]["check_name"], "Validate Corosync Configuration File")