Fix problems of softdog watchdog driver

crmsh/sbd.py

@@ -756,6 +756,7 @@ def check_and_fix(self) -> CheckResult:
                 else:
                     raise FixFailure(f"Failed to fix {name} issue")
 
+        watchdog.Watchdog.warn_if_using_softdog()
         SBDManager.warn_diskless_sbd()
 
         return SBDConfigChecker._return_helper(check_res_list)

crmsh/ui_sbd.py

@@ -672,14 +672,14 @@ def _print_sbd_status(self):
     def _print_watchdog_info(self):
         padding = 2
         max_node_len = max(len(node) for node in self.cluster_nodes) + padding
-        watchdog_sbd_re = r"\[[0-9]+\] (/dev/.*)\nIdentity: Busy: .*sbd.*\nDriver: (.*)"
         device_list, driver_list, kernel_timeout_list = [], [], []
 
         for node in self.cluster_nodes:
-            out = self.cluster_shell.get_stdout_or_raise_error("sbd query-watchdog", node)
-            res = re.search(watchdog_sbd_re, out)
-            if res:
-                device, driver = res.groups()
+            out = self.cluster_shell.get_stdout_or_raise_error(watchdog.Watchdog.QUERY_CMD, node)
+            watchdog_info = watchdog.Watchdog.get_watchdog_info(out, sbd_only=True)
+            if watchdog_info:
+                device = next(iter(watchdog_info))
+                driver = watchdog_info[device]
                 kernel_timeout = self.cluster_shell.get_stdout_or_raise_error("cat /proc/sys/kernel/watchdog_thresh", node)
             else:
                 device, driver, kernel_timeout = "N/A", "N/A", "N/A"

crmsh/utils.py

@@ -1662,7 +1662,7 @@ def list_cluster_nodes(no_reg=False) -> list[str]:
     return [x.uname for x in cibquery.get_cluster_nodes(cib)]
 
 
-def cluster_run_cmd(cmd, node_list=[]):
+def cluster_run_cmd(cmd, node_list=None):
     """
     Run cmd in cluster nodes
     """

crmsh/watchdog.py

@@ -1,17 +1,24 @@
+import logging
 import re
+
 from . import utils
 from .sh import ShellUtils
 from . import sh
 from . import sbd
 
 
+logger = logging.getLogger(__name__)
+
+
 class Watchdog(object):
     """
     Class to find valid watchdog device name
     """
     WATCHDOG_CFG = "/etc/modules-load.d/watchdog.conf"
     QUERY_CMD = "sudo sbd query-watchdog"
-    DEVICE_FIND_REGREX = "\\[[0-9]+\\] (/dev/.*)\n.*\nDriver: (.*)"
+    # output format might like:
+    #   [1] /dev/watchdog\nIdentity: Software Watchdog\nDriver: softdog\n
+    DEVICE_FIND_REGREX = r"[ \t]*\[[0-9]+\] (/dev/[^\n]+)\n[ \t]*Identity: ([^\n]+)\n[ \t]*Driver: ([^\n]+)"
 
     def __init__(self, _input=None, remote_user=None, peer_host=None):
         """
@@ -28,25 +35,23 @@ def watchdog_device_name(self):
         return self._watchdog_device_name
 
     @staticmethod
-    def verify_watchdog_device(dev, ignore_error=False):
+    def verify_watchdog_device(dev):
         """
         Use wdctl to verify watchdog device
         """
-        rc, _, err = ShellUtils().get_stdout_stderr("wdctl {}".format(dev))
+        rc, _, err = ShellUtils().get_stdout_stderr(f"wdctl {dev}")
         if rc != 0:
-            if ignore_error:
-                return False
-            else:
-                utils.fatal("Invalid watchdog device {}: {}".format(dev, err))
+            utils.fatal(f"Invalid watchdog device {dev}: {err}")
         return True
 
     @staticmethod
-    def _load_watchdog_driver(driver):
+    def _load_watchdog_driver(driver, join=False):
         """
         Load specific watchdog driver
         """
-        ShellUtils().get_stdout_stderr(f"echo {driver} > {Watchdog.WATCHDOG_CFG}")
-        ShellUtils().get_stdout_stderr("systemctl restart systemd-modules-load")
+        cmd = f"echo {driver} > {Watchdog.WATCHDOG_CFG} && systemctl restart systemd-modules-load"
+        node_list = utils.this_node() if join else None
+        utils.cluster_run_cmd(cmd, node_list)
 
     @staticmethod
     def get_watchdog_device_from_sbd_config():
@@ -64,16 +69,57 @@ def _driver_is_loaded(driver):
         _, out, _ = ShellUtils().get_stdout_stderr("lsmod")
         return re.search("\n{}\\s+".format(driver), out)
 
+    @staticmethod
+    def _get_configured_watchdog_driver():
+        """
+        Get watchdog driver name from modules-load config.
+        """
+        try:
+            with open(Watchdog.WATCHDOG_CFG) as f:
+                return f.readline().strip()
+        except OSError:
+            return None
+
+    @classmethod
+    def get_watchdog_info(cls, out, sbd_only=False):
+        """
+        Parse sbd query-watchdog output into {device_name: driver_name}.
+        """
+        if not out:
+            return {}
+
+        watchdog_info = {}
+        for device, identity, driver in re.findall(cls.DEVICE_FIND_REGREX, out):
+            if sbd_only and not re.search(r"Busy: .*sbd", identity):
+                continue
+            if driver == "<unknown>":
+                configured_driver = cls._get_configured_watchdog_driver()
+                if configured_driver and cls._driver_is_loaded(configured_driver):
+                    driver = configured_driver
+            watchdog_info[device] = driver
+        return watchdog_info
+
+    @staticmethod
+    def warn_if_using_softdog():
+        """
+        Warn if SBD is using softdog as watchdog driver.
+        """
+        rc, out, err = ShellUtils().get_stdout_stderr(Watchdog.QUERY_CMD)
+        if rc != 0 or not out:
+            logger.debug("Failed to run %s: %s", Watchdog.QUERY_CMD, err)
+            return
+
+        if "softdog" in Watchdog.get_watchdog_info(out, sbd_only=True).values():
+            logger.warning("It's not recommended to use softdog as watchdog driver in production environment")
+
     def _set_watchdog_info(self):
         """
         Set watchdog info through sbd query-watchdog command
         Content in self._watchdog_info_dict: {device_name: driver_name}
         """
         rc, out, err = ShellUtils().get_stdout_stderr(self.QUERY_CMD)
         if rc == 0 and out:
-            # output format might like:
-            #   [1] /dev/watchdog\nIdentity: Software Watchdog\nDriver: softdog\n
-            self._watchdog_info_dict = dict(re.findall(self.DEVICE_FIND_REGREX, out))
+            self._watchdog_info_dict = self.get_watchdog_info(out)
         else:
             utils.fatal("Failed to run {}: {}".format(self.QUERY_CMD, err))
 
@@ -92,39 +138,24 @@ def _get_driver_through_device_remotely(self, dev_name):
         """
         rc, out, err = sh.cluster_shell().get_rc_stdout_stderr_without_input(self._peer_host, self.QUERY_CMD)
         if rc == 0 and out:
-            # output format might like:
-            #   [1] /dev/watchdog\nIdentity: Software Watchdog\nDriver: softdog\n
-            device_driver_dict = dict(re.findall(self.DEVICE_FIND_REGREX, out))
-            if device_driver_dict and dev_name in device_driver_dict:
+            device_driver_dict = self.get_watchdog_info(out)
+            if dev_name in device_driver_dict:
                 return device_driver_dict[dev_name]
             else:
                 return None
         else:
             utils.fatal("Failed to run {} remotely: {}".format(self.QUERY_CMD, err))
 
-    def _get_first_unused_device(self):
-        """
-        Get first unused watchdog device name
-        """
-        for dev in self._watchdog_info_dict:
-            if self.verify_watchdog_device(dev, ignore_error=True):
-                return dev
-        return None
-
     def _set_input(self):
-        """
-        If self._input was not provided by option:
-          1. Try to get it from sbd config file
-          2. Try to get the first valid device from result of sbd query-watchdog
-          3. Set the self._input as softdog
-        """
-        if not self._input:
-            dev = self.get_watchdog_device_from_sbd_config()
-            if dev and self.verify_watchdog_device(dev, ignore_error=True):
+        if self._input:
+            return
+
+        for dev, driver in self._watchdog_info_dict.items():
+            if driver != "softdog":
                 self._input = dev
                 return
-            first_unused = self._get_first_unused_device()
-            self._input = first_unused if first_unused else "softdog"
+
+        self._input = "softdog"
 
     def _valid_device(self, dev):
         """
@@ -136,7 +167,7 @@ def _valid_device(self, dev):
 
     def join_watchdog(self):
         """
-        In join proces, get watchdog device from config
+        In join process, get watchdog device from config
         If that device not exist, get driver name from init node, and load that driver
         """
         self._set_watchdog_info()
@@ -148,7 +179,7 @@ def join_watchdog(self):
 
         if not self._valid_device(self._input):
             driver = self._get_driver_through_device_remotely(self._input)
-            self._load_watchdog_driver(driver)
+            self._load_watchdog_driver(driver, join=True)
 
     def init_watchdog(self):
         """

test/unittests/test_sbd.py

@@ -392,11 +392,12 @@ def test_check_and_fix_sbd_inconsistent(self, mock_service_manager, mock_check_a
         self.instance_check._check_config_consistency.assert_called_once()
 
     @patch('crmsh.sbd.SBDConfigChecker._check_deprecated_property')
+    @patch("crmsh.watchdog.Watchdog.warn_if_using_softdog")
     @patch('crmsh.sbd.SBDManager.warn_diskless_sbd')
     @patch('crmsh.utils.list_cluster_nodes_except_me')
     @patch('crmsh.utils.check_all_nodes_reachable')
     @patch('crmsh.sbd.ServiceManager')
-    def test_check_and_fix_not_fix(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me, mock_warn_diskless_sbd, mock_check_deprecated_property):
+    def test_check_and_fix_not_fix(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me, mock_warn_diskless_sbd, mock_warn_if_using_softdog, mock_check_deprecated_property):
         mock_service_manager_inst = Mock()
         mock_service_manager.return_value = mock_service_manager_inst
         mock_service_manager_inst.service_is_active = Mock(return_value=True)
@@ -424,6 +425,7 @@ def test_check_and_fix_not_fix(self, mock_service_manager, mock_check_all_nodes_
         self.instance_check._check_config_consistency.assert_called_once()
         self.instance_check._load_configurations_from_runtime.assert_called_once()
         self.instance_check._check_sbd_disk_metadata.assert_called_once()
+        mock_warn_if_using_softdog.assert_called_once_with()
 
     @patch('crmsh.utils.list_cluster_nodes_except_me')
     @patch('crmsh.utils.check_all_nodes_reachable')
@@ -444,11 +446,12 @@ def test_check_and_fix_fix_failure(self, mock_service_manager, mock_check_all_no
         self.assertTrue("Failed to fix SBD disk metadata" in str(context.exception))
 
     @patch('crmsh.sbd.SBDConfigChecker._check_deprecated_property')
+    @patch("crmsh.watchdog.Watchdog.warn_if_using_softdog")
     @patch('crmsh.sbd.SBDManager.warn_diskless_sbd')
     @patch('crmsh.utils.list_cluster_nodes_except_me')
     @patch('crmsh.utils.check_all_nodes_reachable')
     @patch('crmsh.sbd.ServiceManager')
-    def test_check_and_fix_fix_success(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me, mock_warn_diskless_sbd, mock_check_deprecated_property):
+    def test_check_and_fix_fix_success(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me, mock_warn_diskless_sbd, mock_warn_if_using_softdog, mock_check_deprecated_property):
         mock_service_manager_inst = Mock()
         mock_service_manager.return_value = mock_service_manager_inst
         mock_service_manager_inst.service_is_active = Mock(return_value=True)
@@ -484,6 +487,7 @@ def test_check_and_fix_fix_success(self, mock_service_manager, mock_check_all_no
         self.instance_fix._check_fencing_timeout.assert_called_once()
         self.instance_fix._check_sbd_delay_start_unset_dropin.assert_called_once()
 
+        mock_warn_if_using_softdog.assert_called_once_with()
     @patch('logging.Logger.error')
     @patch('logging.Logger.warning')
     @patch('crmsh.utils.remote_diff_this')