Fix: Adjust the process of configuring qdevice (bsc#1261884)

crmsh/bootstrap.py

@@ -1542,9 +1542,11 @@ def configure_qdevice_interactive():
     """
     if _context.yes_to_all:
         return
+
     logger.info("Configure Qdevice/Qnetd:\n" + QDEVICE_HELP_INFO + "\n")
     if not confirm("Do you want to configure QDevice?"):
         return
+
     while True:
         if utils.package_is_installed("corosync-qdevice"):
             break
@@ -1631,16 +1633,15 @@ def init_qdevice():
     """
     Setup qdevice and qnetd service
     """
-    if not _context.qdevice_inst:
-        configure_qdevice_interactive()
-    if not _context.qdevice_inst:
-        ServiceManager().disable_service("corosync-qdevice.service")
-        return
-
-    logger.info("""Configure Qdevice/Qnetd:""")
-
     is_qdevice_stage = _context.stage == "qdevice"
     if is_qdevice_stage:
+        if qdevice.is_qdevice_running_on_cluster():
+            logger.info("Qdevice is already running on this cluster - will not reconfigure")
+            return
+        elif corosync.is_qdevice_configured():
+            qdevice.QDevice.start_qdevice_service()
+            return
+
         qdevice_reload_policy = qdevice.evaluate_qdevice_quorum_effect(qdevice.QDEVICE_ADD)
         if qdevice_reload_policy == qdevice.QdevicePolicy.QDEVICE_RESTART_LATER:
             with utils.leverage_maintenance_mode() as enabled:
@@ -1653,21 +1654,25 @@ def init_qdevice():
 
 
 def do_init_qdevice(in_stage: bool = False):
+    if not _context.qdevice_inst:
+        configure_qdevice_interactive()
+    if not _context.qdevice_inst:
+        if not in_stage:
+            ServiceManager().disable_service("corosync-qdevice.service")
+        return
+
+    logger.info("""Configure Qdevice/Qnetd:""")
+
     cluster_node_list = qdevice.get_node_list(in_stage)
     _setup_passwordless_ssh_for_qnetd(cluster_node_list)
 
     qdevice_inst = _context.qdevice_inst
-    if corosync.is_qdevice_configured() and not confirm("Qdevice is already configured - overwrite?"):
-        if in_stage:
-            qdevice_inst.start_qdevice_service()
-        return
-
     qdevice_inst.set_cluster_name()
     qdevice_inst.validate_and_start_qnetd()
     qdevice_inst.certificate_and_config_qdevice()
 
     if in_stage:
-        qdevice_inst.start_qdevice_service()
+        qdevice.QDevice.start_qdevice_service()
 
     adjust_properties()
 

crmsh/constants.py

@@ -379,7 +379,7 @@
   a third-party arbitrator Qnetd, it provides votes so that a cluster 
   is able to sustain more node failures than standard quorum rules 
   allow. It is recommended for clusters with an even number of nodes 
-  and highly recommended for 2 node clusters."""
+  and highly recommended for 2-node clusters."""
 
 
 SSH_OPTION_ARGS = ["-o", "StrictHostKeyChecking=no"]

crmsh/qdevice.py

@@ -17,6 +17,7 @@
 from . import lock
 from . import log
 from . import sbd
+from . import constants
 from .service_manager import ServiceManager
 
 
@@ -105,6 +106,12 @@ def get_node_list(is_stage: bool) -> list[str]:
         return [me]
 
 
+def is_qdevice_running_on_cluster() -> bool:
+    service_manager = ServiceManager()
+    node_list = utils.list_cluster_nodes()
+    return all(service_manager.service_is_active(constants.COROSYNC_QDEVICE_SERVICE, node) for node in node_list)
+
+
 class QDevice(object):
     """Class to manage qdevice configuration and services
 
@@ -133,7 +140,6 @@ def __init__(self, qnetd_addr, port=5403, algo="ffsplit", tie_breaker="lowest",
         self.cmds = cmds
         self.mode = mode
         self.cluster_name = cluster_name
-        self.qdevice_reload_policy = QdevicePolicy.QDEVICE_RESTART
         self.is_stage = is_stage
 
     @property
@@ -528,18 +534,19 @@ def config_qnetd_port(self):
         logger.info("Add port {} to firewalld on {}".format(self.port, self.qnetd_addr))
         shell.get_stdout_or_raise_error("firewall-cmd --reload", self.qnetd_addr)
 
-    def start_qdevice_service(self):
+    @staticmethod
+    def start_qdevice_service():
         logger.info("Enable corosync-qdevice.service in cluster")
         utils.cluster_run_cmd("systemctl enable corosync-qdevice")
 
-        self.qdevice_reload_policy = evaluate_qdevice_quorum_effect(QDEVICE_ADD)
+        qdevice_reload_policy = evaluate_qdevice_quorum_effect(QDEVICE_ADD)
 
-        if self.qdevice_reload_policy == QdevicePolicy.QDEVICE_RELOAD:
+        if qdevice_reload_policy == QdevicePolicy.QDEVICE_RELOAD:
             logger.info("Reloading cluster configuration before starting corosync-qdevice.service")
             sh.cluster_shell().get_stdout_or_raise_error("corosync-cfgtool -R")
             logger.info("Starting corosync-qdevice.service in cluster")
             utils.cluster_run_cmd("systemctl restart corosync-qdevice")
-        elif self.qdevice_reload_policy == QdevicePolicy.QDEVICE_RESTART:
+        elif qdevice_reload_policy == QdevicePolicy.QDEVICE_RESTART:
             bootstrap.restart_cluster()
 
     def adjust_sbd_watchdog_timeout_with_qdevice(self):

test/features/steps/const.py

@@ -109,7 +109,7 @@
   a third-party arbitrator Qnetd, it provides votes so that a cluster 
   is able to sustain more node failures than standard quorum rules 
   allow. It is recommended for clusters with an even number of nodes 
-  and highly recommended for 2 node clusters.
+  and highly recommended for 2-node clusters.
 
   Options for configuring QDevice and QNetd.
 

test/unittests/test_bootstrap.py

@@ -1225,13 +1225,13 @@ def test_init_qdevice_no_config(self, mock_configure, mock_disable):
         mock_configure.assert_called_once_with()
         mock_disable.assert_called_once_with("corosync-qdevice.service")
 
+    @mock.patch('crmsh.corosync.is_qdevice_configured')
+    @mock.patch('crmsh.qdevice.is_qdevice_running_on_cluster')
     @mock.patch('crmsh.bootstrap.do_init_qdevice')
     @mock.patch('crmsh.utils.able_to_restart_cluster')
     @mock.patch('crmsh.utils.leverage_maintenance_mode')
     @mock.patch('crmsh.qdevice.evaluate_qdevice_quorum_effect')
-    @mock.patch('logging.Logger.info')
-    def test_init_qdevice_unable_to_restart_cluster(self, mock_info, mock_evaluate_qdevice_quorum_effect, mock_leverage_maintenance_mode,
-            mock_able_to_restart_cluster, mock_do_init_qdevice):
+    def test_init_qdevice_unable_to_restart_cluster(self, mock_evaluate_qdevice_quorum_effect, mock_leverage_maintenance_mode, mock_able_to_restart_cluster, mock_do_init_qdevice, mock_is_qdevice_running_on_cluster, mock_is_qdevice_configured):
         bootstrap._context = mock.Mock(qdevice_inst=self.qdevice_with_ip, stage="qdevice")
         mock_evaluate_qdevice_quorum_effect.return_value = qdevice.QdevicePolicy.QDEVICE_RESTART_LATER
         enable_value = True
@@ -1240,20 +1240,21 @@ def test_init_qdevice_unable_to_restart_cluster(self, mock_info, mock_evaluate_q
         cm.__exit__ = mock.Mock(return_value=False)
         mock_leverage_maintenance_mode.return_value = cm
         mock_able_to_restart_cluster.return_value = False
+        mock_is_qdevice_configured.return_value = False
+        mock_is_qdevice_running_on_cluster.return_value = False
 
         bootstrap.init_qdevice()
 
-        mock_info.assert_called_once_with("Configure Qdevice/Qnetd:")
         mock_able_to_restart_cluster.assert_called_once_with(True)
         mock_do_init_qdevice.assert_not_called()
 
+    @mock.patch('crmsh.corosync.is_qdevice_configured')
+    @mock.patch('crmsh.qdevice.is_qdevice_running_on_cluster')
     @mock.patch('crmsh.bootstrap.do_init_qdevice')
     @mock.patch('crmsh.utils.able_to_restart_cluster')
     @mock.patch('crmsh.utils.leverage_maintenance_mode')
     @mock.patch('crmsh.qdevice.evaluate_qdevice_quorum_effect')
-    @mock.patch('logging.Logger.info')
-    def test_init_qdevice_able_to_restart_cluster(self, mock_info, mock_evaluate_qdevice_quorum_effect, mock_leverage_maintenance_mode,
-            mock_able_to_restart_cluster, mock_do_init_qdevice):
+    def test_init_qdevice_able_to_restart_cluster(self, mock_evaluate_qdevice_quorum_effect, mock_leverage_maintenance_mode, mock_able_to_restart_cluster, mock_do_init_qdevice, mock_is_qdevice_running_on_cluster, mock_is_qdevice_configured):
         bootstrap._context = mock.Mock(qdevice_inst=self.qdevice_with_ip, stage="qdevice")
         mock_evaluate_qdevice_quorum_effect.return_value = qdevice.QdevicePolicy.QDEVICE_RESTART_LATER
         enable_value = True
@@ -1262,55 +1263,33 @@ def test_init_qdevice_able_to_restart_cluster(self, mock_info, mock_evaluate_qde
         cm.__exit__ = mock.Mock(return_value=False)
         mock_leverage_maintenance_mode.return_value = cm
         mock_able_to_restart_cluster.return_value = True
+        mock_is_qdevice_configured.return_value = False
+        mock_is_qdevice_running_on_cluster.return_value = False
 
         bootstrap.init_qdevice()
 
-        mock_info.assert_called_once_with("Configure Qdevice/Qnetd:")
         mock_able_to_restart_cluster.assert_called_once_with(True)
         mock_do_init_qdevice.assert_called_once_with(True)
 
-    @mock.patch('crmsh.bootstrap.confirm')
-    @mock.patch('crmsh.corosync.is_qdevice_configured')
-    @mock.patch('crmsh.bootstrap._setup_passwordless_ssh_for_qnetd')
-    @mock.patch('crmsh.qdevice.get_node_list')
-    def test_do_init_qdevice_already_configured(self, mock_list_nodes, mock_setup_passwordless_ssh_for_qnetd, mock_is_qdevice_configured, mock_confirm):
-        bootstrap._context = mock.Mock(qdevice_inst=self.qdevice_with_ip, stage="qdevice")
-        mock_list_nodes.return_value = ["node1"]
-        mock_is_qdevice_configured.return_value = True
-        mock_confirm.return_value = False
-        bootstrap._context.qdevice_inst.start_qdevice_service = mock.Mock()
-
-        bootstrap.do_init_qdevice(True)
-
-        mock_list_nodes.assert_called_once_with(True)
-        mock_is_qdevice_configured.assert_called_once_with()
-        mock_confirm.assert_called_once_with("Qdevice is already configured - overwrite?")
-        mock_setup_passwordless_ssh_for_qnetd.assert_called_once_with(["node1"])
-        bootstrap._context.qdevice_inst.start_qdevice_service.assert_called_once_with()
-
+    @mock.patch('crmsh.qdevice.QDevice.start_qdevice_service')
     @mock.patch('crmsh.bootstrap.adjust_properties')
-    @mock.patch('crmsh.corosync.is_qdevice_configured')
     @mock.patch('crmsh.bootstrap._setup_passwordless_ssh_for_qnetd')
     @mock.patch('crmsh.qdevice.get_node_list')
     def test_do_init_qdevice(self, mock_list_nodes, mock_setup_passwordless_ssh_for_qnetd,
-            mock_is_qdevice_configured, mock_adjust_properties):
+            mock_adjust_properties, mock_start_qdevice_service):
         bootstrap._context = mock.Mock(qdevice_inst=self.qdevice_with_ip, stage="qdevice")
         mock_list_nodes.return_value = ["node1"]
-        mock_is_qdevice_configured.return_value = False
         self.qdevice_with_ip.set_cluster_name = mock.Mock()
         self.qdevice_with_ip.validate_and_start_qnetd = mock.Mock()
         self.qdevice_with_ip.certificate_and_config_qdevice = mock.Mock()
-        bootstrap._context.qdevice_inst.start_qdevice_service = mock.Mock()
 
         bootstrap.do_init_qdevice(True)
 
         mock_list_nodes.assert_called_once_with(True)
-        mock_is_qdevice_configured.assert_called_once_with()
         mock_setup_passwordless_ssh_for_qnetd.assert_called_once_with(["node1"])
         self.qdevice_with_ip.set_cluster_name.assert_called_once_with()
         self.qdevice_with_ip.validate_and_start_qnetd.assert_called_once_with()
         self.qdevice_with_ip.certificate_and_config_qdevice.assert_called_once_with()
-        bootstrap._context.qdevice_inst.start_qdevice_service.assert_called_once_with()
 
     @mock.patch('crmsh.bootstrap.prompt_for_string')
     def test_configure_qdevice_interactive_return(self, mock_prompt):