Dev: sbd: Refactor to enable checking when cluster is down

Author: liangxin1300

crmsh/bootstrap.py

@@ -2068,14 +2068,9 @@ def adjust_priority_fencing_delay(is_2node_wo_qdevice):
     and the current cluster is 2 nodes without qdevice,
     set priority-fencing-delay=2*pcmk_delay_max
     """
-    out = sh.cluster_shell().get_stdout_or_raise_error("crm configure show related:stonith")
-    if not out:
-        return
-    pcmk_delay_max_v_list = re.findall(r"pcmk_delay_max=(\w+)", out)
-    if pcmk_delay_max_v_list:
-        max_value = max([int(utils.crm_msec(v)/1000) for v in pcmk_delay_max_v_list])
-    if pcmk_delay_max_v_list and is_2node_wo_qdevice:
-        utils.set_property("priority-fencing-delay", 2*max_value, conditional=True)
+    pcmk_delay_max_value = utils.get_pcmk_delay_max_configured_value()
+    if pcmk_delay_max_value > 0 and is_2node_wo_qdevice:
+        utils.set_property("priority-fencing-delay", 2*pcmk_delay_max_value, conditional=True)
     else:
         utils.set_property("priority-fencing-delay", 0)
 
@@ -2174,10 +2169,9 @@ def remove_node_from_cluster(node, dead_node=False):
         corosync.del_node(node_ip if node_ip is not None else node)
 
     corosync.configure_two_node(removing=True)
-    adjust_properties()
-
     logger.info("Propagating configuration changes across the remaining nodes")
     sync_path(corosync.conf())
+    adjust_properties()
 
     sh.cluster_shell().get_stdout_or_raise_error("corosync-cfgtool -R")
 
@@ -2761,19 +2755,19 @@ def adjust_pcmk_delay_max(is_2node_wo_qdevice):
             logger.info("Delete parameter 'pcmk_delay_max' for resource '{}'".format(res))
 
 
-def adjust_stonith_timeout(with_sbd: bool = False):
+def adjust_stonith_timeout():
     """
     Adjust stonith-timeout for sbd and other scenarios
     """
-    if ServiceManager().service_is_active(constants.SBD_SERVICE) or with_sbd:
-        sbd.SBDTimeoutChecker(quiet=True, fix=True, from_bootstrap=True).check_and_fix()
+    if ServiceManager().service_is_active(constants.SBD_SERVICE):
+        sbd.SBDTimeoutChecker(quiet=True, fix=True).check_and_fix()
     else:
         value = get_stonith_timeout_generally_expected()
         if value:
             utils.set_property("stonith-timeout", value, conditional=True)
 
 
-def adjust_properties(with_sbd: bool = False):
+def adjust_properties():
     """
     Adjust properties for the cluster:
     - pcmk_delay_max
@@ -2791,7 +2785,7 @@ def adjust_properties(with_sbd: bool = False):
         return
     is_2node_wo_qdevice = utils.is_2node_cluster_without_qdevice()
     adjust_pcmk_delay_max(is_2node_wo_qdevice)
-    adjust_stonith_timeout(with_sbd=with_sbd)
+    adjust_stonith_timeout()
     adjust_priority_in_rsc_defaults(is_2node_wo_qdevice)
     adjust_priority_fencing_delay(is_2node_wo_qdevice)
 

crmsh/sbd.py

@@ -11,6 +11,7 @@
 from . import corosync
 from . import xmlutil
 from . import watchdog
+from . import cibquery
 from .service_manager import ServiceManager
 from .sh import ShellUtils
 
@@ -176,6 +177,15 @@ def handle_input_sbd_devices(dev_list, dev_list_from_config=None):
 
         return overwrite_list, no_overwrite_list
 
+    @staticmethod
+    def diskbased_sbd_configured() -> bool:
+        return bool(SBDUtils.get_sbd_device_from_config())
+
+    @staticmethod
+    def diskless_sbd_configured() -> bool:
+        value = utils.get_property("stonith-watchdog-timeout")
+        return value and utils.crm_msec(value) > 0
+
 
 class SBDTimeout(object):
     '''
@@ -448,11 +458,10 @@ class CheckResult(Enum):
 
 class SBDTimeoutChecker(SBDTimeout):
 
-    def __init__(self, quiet=False, fix=False, from_bootstrap=False):
+    def __init__(self, quiet=False, fix=False):
         super().__init__()
         self.quiet = quiet
         self.fix = fix
-        self.from_bootstrap = from_bootstrap
         self.peer_node_list = []
         self.service_disabled_node_list = []
 
@@ -465,6 +474,22 @@ def _return_helper(check_res_list: list[CheckResult]) -> CheckResult:
         else:
             return CheckResult.WARNING
 
+    @staticmethod
+    def log_and_return(check_res: CheckResult, fix_flag: bool = False) -> bool:
+        if check_res == CheckResult.SUCCESS:
+            logger.info('SBD: Check sbd timeout configuration: OK.')
+            return True
+        cmd = "crm cluster health sbd --fix"
+        issue_type = "error" if check_res == CheckResult.ERROR else "warning"
+        if not fix_flag:
+            logger.info(f'Please run "{cmd}" to fix the above {issue_type} on the running cluster')
+        if check_res == CheckResult.ERROR:
+            logger.error("SBD: Check sbd timeout configuration: FAIL.")
+            return False
+        elif check_res == CheckResult.WARNING:
+            logger.info('SBD: Check sbd timeout configuration: OK.')
+            return True
+
     def check_and_fix(self) -> CheckResult:
         checks_and_fixes = [
             # issue name, check method, fix method, SSH required, prerequisites checks
@@ -549,8 +574,11 @@ def check_and_fix(self) -> CheckResult:
             )
         ]
 
-        if not self.from_bootstrap and not ServiceManager().service_is_active(constants.SBD_SERVICE):
-            raise FixAborted("%s is not active, skip SBD timeout checks" % constants.SBD_SERVICE)
+        if not ServiceManager().service_is_active(constants.SBD_SERVICE):
+            if self.fix:
+                raise FixAborted("%s is not active, skip fixing SBD timeout issues" % constants.SBD_SERVICE)
+            elif not SBDUtils.diskbased_sbd_configured() and not SBDUtils.diskless_sbd_configured():
+                raise FixAborted("Neither disk-based nor disk-less SBD is configured, skip checking SBD timeout issues")
 
         all_nodes_reachable = True
         self.peer_node_list = utils.list_cluster_nodes_except_me()
@@ -592,9 +620,6 @@ def check_and_fix(self) -> CheckResult:
 
     def _check_config_consistency(self, error_msg: str = "") -> bool:
         consistent = True
-        # Don't check consistency during bootstrap process
-        if self.from_bootstrap:
-            return consistent
 
         if not self.peer_node_list:
             if error_msg:
@@ -636,7 +661,7 @@ def _check_sbd_disk_metadata(self) -> CheckResult:
         '''
         For disk-based SBD, check if the sbd msgwait and watchdog timeout are below expected values
         '''
-        if self.disk_based and not self.from_bootstrap:
+        if self.disk_based:
             self.sbd_watchdog_timeout_expected, self.sbd_msgwait_expected = SBDTimeout.get_sbd_metadata_expected()
             if self.sbd_watchdog_timeout < self.sbd_watchdog_timeout_expected:
                 if not self.quiet:
@@ -661,7 +686,7 @@ def _check_sbd_watchdog_timeout(self) -> CheckResult:
         '''
         For diskless SBD, check if SBD_WATCHDOG_TIMEOUT is below expected value
         '''
-        if not self.disk_based and not self.from_bootstrap:
+        if not self.disk_based:
             self.sbd_watchdog_timeout_expected = SBDTimeout.get_sbd_watchdog_timeout_expected(diskless=True)
             if self.sbd_watchdog_timeout < self.sbd_watchdog_timeout_expected:
                 if not self.quiet:
@@ -731,22 +756,28 @@ def _fix_sbd_systemd_start_timeout(self):
         utils.cluster_run_cmd("systemctl daemon-reload")
 
     def _check_stonith_watchdog_timeout(self) -> CheckResult:
-        value = utils.get_property("stonith-watchdog-timeout", get_default=False)
+        value = utils.get_property("stonith-watchdog-timeout")
+        value = int(utils.crm_msec(value)/1000)
         if self.disk_based:
-            if value:
+            if value > 0:
                 if not self.quiet:
                     logger.warning("It's recommended that stonith-watchdog-timeout is not set when using disk-based SBD")
                 return CheckResult.WARNING
         else:
-            if not value or int(value) < self.stonith_watchdog_timeout:
+            if value == 0:
+                if not self.quiet:
+                    logger.error("It's recommended that stonith-watchdog-timeout is set to %d, now is not set",
+                                self.stonith_watchdog_timeout)
+                return CheckResult.ERROR
+            if value < self.stonith_watchdog_timeout:
                 if not self.quiet:
-                    logger.error("It's recommended that stonith-watchdog-timeout is set to %d, now is %s",
-                                self.stonith_watchdog_timeout, value if value else "not set")
+                    logger.error("It's recommended that stonith-watchdog-timeout is set to %d, now is %d",
+                                 self.stonith_watchdog_timeout, value)
                 return CheckResult.ERROR
-            elif int(value) > self.stonith_watchdog_timeout:
+            elif value > self.stonith_watchdog_timeout:
                 if not self.quiet:
                     logger.warning("It's recommended that stonith-watchdog-timeout is set to %d, now is %d",
-                                   self.stonith_watchdog_timeout, int(value))
+                                   self.stonith_watchdog_timeout, value)
                 return CheckResult.WARNING
         return CheckResult.SUCCESS
 
@@ -760,16 +791,18 @@ def _fix_stonith_watchdog_timeout(self):
 
     def _check_stonith_timeout(self) -> CheckResult:
         expected_value = self.get_stonith_timeout_expected()
-        value = utils.get_property("stonith-timeout", get_default=False)
-        if not value or int(value) < expected_value:
+        value = utils.get_property("stonith-timeout")
+        # will get default value from pacemaker metadata if not set
+        value = int(utils.crm_msec(value)/1000)
+        if value < expected_value:
             if not self.quiet:
-                logger.error("It's recommended that stonith-timeout is set to %d, now is %s",
-                             expected_value, value if value else "not set")
+                logger.error("It's recommended that stonith-timeout is set to %d, now is %d",
+                             expected_value, value)
             return CheckResult.ERROR
-        elif int(value) > expected_value:
+        elif value > expected_value:
             if not self.quiet:
                 logger.warning("It's recommended that stonith-timeout is set to %d, now is %d",
-                               expected_value, int(value))
+                               expected_value, value)
             return CheckResult.WARNING
         return CheckResult.SUCCESS
 
@@ -822,14 +855,24 @@ def _fix_sbd_service_is_enabled(self):
             service_manager.enable_service(constants.SBD_SERVICE, node)
 
     def _check_fence_sbd(self) -> CheckResult:
-        if not self.disk_based or self.from_bootstrap:
+        if not self.disk_based:
             return CheckResult.SUCCESS
         xml_inst = xmlutil.CrmMonXmlParser()
+        if xml_inst.not_connected():
+            cib = xmlutil.text2elem(sh.cluster_shell().get_stdout_or_raise_error("crm configure show xml"))
+            ra = cibquery.ResourceAgent("stonith", "", "fence_sbd")
+            configured = cibquery.get_primitives_with_ra(cib, ra)
+            if configured:
+                return CheckResult.SUCCESS
+            else:
+                if not self.quiet:
+                    logger.error("Fence agent %s is not configured", SBDManager.SBD_RA)
+                return CheckResult.ERROR
         if not xml_inst.is_resource_configured(SBDManager.SBD_RA):
             if not self.quiet:
                 logger.error("Fence agent %s is not configured", SBDManager.SBD_RA)
             return CheckResult.ERROR
-        elif not xml_inst.is_resource_started(SBDManager.SBD_RA):
+        elif not xml_inst.is_resource_started(SBDManager.SBD_RA) and not utils.is_cluster_in_maintenance_mode():
             if not self.quiet:
                 logger.error("Fence agent %s is not started", SBDManager.SBD_RA)
             return CheckResult.ERROR
@@ -1130,12 +1173,12 @@ def init_and_deploy_sbd(self, restart_first=False):
                 # Only then should additional properties be configured,
                 # because the stonith-watchdog-timeout property requires sbd.service to be active.
                 restart_cluster_first = restart_first or \
-                        (self.diskless_sbd and not ServiceManager().service_is_active(constants.SBD_SERVICE))
+                        not self.diskless_sbd or \
+                        not ServiceManager().service_is_active(constants.SBD_SERVICE)
                 if restart_cluster_first:
                     bootstrap.restart_cluster()
 
-                self.configure_sbd()
-                bootstrap.adjust_properties(with_sbd=True)
+                bootstrap.adjust_properties()
 
                 # In other cases, it is better to restart the cluster
                 # after modifying SBD-related configurations.

crmsh/ui_cluster.py

@@ -839,6 +839,7 @@ def do_health(self, context, *args):
                         logger.error("hawk2: passwordless ssh authentication: FAIL.")
                         logger.warning('Please run "crm cluster health hawk2 --fix"')
                         return False
+
             case 'sbd':
                 fix = parsed_args.fix
                 try:
@@ -850,16 +851,8 @@ def do_health(self, context, *args):
                     logger.error('%s', e)
                     logger.error('SBD: Check sbd timeout configuration: FAIL.')
                     return False
-                if result == sbd.CheckResult.ERROR:
-                    if not fix:
-                        logger.info('Please run "crm cluster health sbd --fix" to fix the above error')
-                    logger.error('SBD: Check sbd timeout configuration: FAIL.')
-                    return False
-                else:
-                    if result == sbd.CheckResult.WARNING and not fix:
-                        logger.info('Please run "crm cluster health sbd --fix" to fix the above warning')
-                    logger.info('SBD: Check sbd timeout configuration: OK.')
-                    return True
+                return sbd.SBDTimeoutChecker.log_and_return(result, fix)
+
             case 'sles16':
                 try:
                     if parsed_args.fix:

crmsh/ui_sbd.py

@@ -252,6 +252,7 @@ def _configure_show(self, args) -> None:
             print()
             self._show_property()
 
+        print()
         return SBD.check_timeout_configurations()
 
     @staticmethod
@@ -262,11 +263,7 @@ def check_timeout_configurations() -> bool:
         except sbd.FixAborted as e:
             logger.error('%s', e)
             return False
-        if check_rc != sbd.CheckResult.SUCCESS:
-            issue_type = "error" if check_rc == sbd.CheckResult.ERROR else "warning"
-            logger.info('Please run "crm cluster health sbd --fix" to fix the above %s', issue_type)
-
-        return check_rc in (sbd.CheckResult.SUCCESS, sbd.CheckResult.WARNING)
+        return sbd.SBDTimeoutChecker.log_and_return(check_rc)
 
     def _parse_args(self, args: tuple[str, ...]) -> dict[str, int|str]:
         '''

crmsh/utils.py

@@ -2822,13 +2822,28 @@ def is_2node_cluster_without_qdevice():
     return (current_num + qdevice_num) == 2
 
 
+def get_pcmk_delay_max_configured_value() -> int:
+    out = sh.cluster_shell().get_stdout_or_raise_error("crm configure show related:stonith")
+    if not out:
+        return 0
+    pcmk_delay_max_v_list = re.findall(r"pcmk_delay_max=(\w+)", out)
+    if pcmk_delay_max_v_list:
+        return max([int(crm_msec(v)/1000) for v in pcmk_delay_max_v_list])
+    else:
+        return 0
+
+
 def get_pcmk_delay_max(two_node_without_qdevice=False):
     """
     Get value of pcmk_delay_max
     """
+    configured_value = get_pcmk_delay_max_configured_value()
+    if configured_value > 0:
+        return configured_value
     if ServiceManager().service_is_active("pacemaker.service") and two_node_without_qdevice:
         return constants.PCMK_DELAY_MAX
-    return 0
+    else:
+        return 0
 
 
 def get_property(name, property_type="crm_config", peer=None, get_default=True):

crmsh/xmlutil.py

@@ -1516,6 +1516,8 @@ class CrmMonXmlParser(object):
     """
     Class to parse xml output of crm_mon
     """
+    NOT_CONNECTED_CODE = 102
+
     def __init__(self, peer=None):
         """
         Init function
@@ -1531,6 +1533,10 @@ def _load(self):
         _, output, _ = sh.cluster_shell().get_rc_stdout_stderr_without_input(self.peer, constants.CRM_MON_XML_OUTPUT)
         return text2elem(output) if output else None
 
+    def not_connected(self):
+        xpath = f"//status[@code='{self.NOT_CONNECTED_CODE}']"
+        return bool(self.xml_elem.xpath(xpath))
+
     def with_quorum(self):
         """
         Check if cluster is with quorum