Dev: unittests: Adjust unit test for previous commits

Author: liangxin1300

test/unittests/test_bootstrap.py

@@ -520,16 +520,10 @@ def tearDownClass(cls):
         Global tearDown.
         """
 
-    @mock.patch('crmsh.parallax.parallax_call')
     @mock.patch('crmsh.service_manager.ServiceManager.start_service')
-    @mock.patch('crmsh.sbd.SBDTimeout.is_sbd_delay_start')
-    @mock.patch('crmsh.service_manager.ServiceManager.service_is_enabled')
-    @mock.patch('crmsh.utils.package_is_installed')
-    def test_start_pacemaker(self, mock_installed, mock_enabled, mock_delay_start, mock_start, mock_parallax_call):
+    @mock.patch('crmsh.sbd.SBDManager.unset_sbd_delay_start')
+    def test_start_pacemaker(self, mock_unset_delay_start,  mock_start):
         bootstrap._context = None
-        mock_installed.return_value = True
-        mock_enabled.return_value = True
-        mock_delay_start.return_value = True
         node_list = ["node1", "node2", "node3", "node4", "node5", "node6"]
         bootstrap.start_pacemaker(node_list)
         mock_start.assert_has_calls([
@@ -541,11 +535,6 @@ def test_start_pacemaker(self, mock_installed, mock_enabled, mock_delay_start, m
             mock.call("corosync.service", remote_addr="node6"),
             mock.call("pacemaker.service", enable=False, node_list=node_list)
             ])
-        mock_parallax_call.assert_has_calls([
-            mock.call(node_list, f'mkdir -p {sbd.SBDManager.SBD_SYSTEMD_DELAY_START_DISABLE_DIR}'),
-            mock.call(node_list, f"echo -e '[Service]\nUnsetEnvironment=SBD_DELAY_START' > {sbd.SBDManager.SBD_SYSTEMD_DELAY_START_DISABLE_FILE}"),
-            mock.call(node_list, "systemctl daemon-reload"),
-            ])
 
     @mock.patch('crmsh.bootstrap.change_user_shell')
     @mock.patch('crmsh.bootstrap.configure_ssh_key')

test/unittests/test_sbd.py

@@ -147,13 +147,13 @@ def test_check_devices_metadata_consistent_multiple_devices_consistent(self, moc
         self.assertTrue(result)
 
     @patch('crmsh.sbd.SBDUtils.get_sbd_device_metadata')
-    @patch('logging.Logger.warning')
-    def test_check_devices_metadata_consistent_multiple_devices_inconsistent(self, mock_logger_warning, mock_get_sbd_device_metadata):
+    @patch('logging.Logger.error')
+    def test_check_devices_metadata_consistent_multiple_devices_inconsistent(self, mock_logger_error, mock_get_sbd_device_metadata):
         dev_list = ['/dev/sbd_device1', '/dev/sbd_device2']
         mock_get_sbd_device_metadata.side_effect = ['metadata1', 'metadata2']
         result = SBDUtils.check_devices_metadata_consistent(dev_list)
         self.assertFalse(result)
-        mock_logger_warning.assert_called()
+        mock_logger_error.assert_called_once_with("Device %s doesn't have the same metadata as %s", '/dev/sbd_device2', '/dev/sbd_device1')
 
     @patch('crmsh.sbd.SBDUtils.check_devices_metadata_consistent')
     @patch('crmsh.sbd.SBDUtils.no_overwrite_device_check')
@@ -206,7 +206,7 @@ def test_get_sbd_systemd_start_timeout(self, mock_cluster_shell, mock_get_system
         mock_get_systemd_timeout_start_in_sec.return_value = 90
         result = sbd.SBDTimeout.get_sbd_systemd_start_timeout()
         self.assertEqual(result, 90)
-        mock_cluster_shell.return_value.get_stdout_or_raise_error.assert_called_once_with(sbd.SBDTimeout.SHOW_SBD_START_TIMEOUT_CMD)
+        mock_cluster_shell.return_value.get_stdout_or_raise_error.assert_called_once_with(sbd.SBDTimeout.SHOW_SBD_START_TIMEOUT_CMD, None)
         mock_get_systemd_timeout_start_in_sec.assert_called_once_with("1min 30s")
 
     @patch('crmsh.corosync.token_and_consensus_timeout')
@@ -263,73 +263,89 @@ def setUp(self):
         self.instance_check = sbd.SBDTimeoutChecker(fix=False)
         self.instance_fix = sbd.SBDTimeoutChecker(fix=True)
 
-    @patch('logging.Logger.warning')
     @patch('crmsh.sbd.ServiceManager')
-    def test_check_and_fix_sbd_not_active(self, mock_service_manager, mock_logger_warning):
+    def test_check_and_fix_sbd_not_active(self, mock_service_manager):
         mock_service_manager_inst = Mock()
         mock_service_manager.return_value = mock_service_manager_inst
         mock_service_manager_inst.service_is_active = Mock(return_value=False)
-        with self.assertRaises(utils.TerminateSubCommand):
+        with self.assertRaises(sbd.FixAborted) as context: 
             self.instance_check.check_and_fix()
+        self.assertTrue("sbd.service is not active" in str(context.exception))
         mock_service_manager_inst.service_is_active.assert_called_once_with(constants.SBD_SERVICE)
-        mock_logger_warning.assert_called_once_with('%s is not active, skip SBD timeout checks', constants.SBD_SERVICE)
 
+    @patch('crmsh.utils.list_cluster_nodes_except_me')
+    @patch('crmsh.utils.check_all_nodes_reachable')
     @patch('crmsh.sbd.ServiceManager')
-    def test_check_and_fix_sbd_inconsistent(self, mock_service_manager):
+    def test_check_and_fix_sbd_inconsistent(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me):
         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)
         self.instance_check._check_config_consistency = Mock(return_value=False)
-        with self.assertRaises(utils.TerminateSubCommand):
+        with self.assertRaises(sbd.FixAborted):
             self.instance_check.check_and_fix()
         mock_service_manager_inst.service_is_active.assert_called_once_with(constants.SBD_SERVICE)
         self.instance_check._check_config_consistency.assert_called_once()
 
+    @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):
+    def test_check_and_fix_not_fix(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me):
         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)
         self.instance_check._check_config_consistency = Mock(return_value=True)
         self.instance_check._load_configurations_from_runtime = Mock()
-        self.instance_check._check_sbd_disk_metadata = Mock(return_value=False)
-        self.instance_check._check_sbd_watchdog_timeout = Mock(return_value=True)
-
-        self.assertFalse(self.instance_check.check_and_fix())
+        self.instance_check._check_sbd_disk_metadata = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_check._check_sbd_device_metadata_consistency = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_check._check_sbd_watchdog_timeout = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_check._check_sbd_delay_start = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_check._check_sbd_systemd_start_timeout = Mock(return_value=sbd.CheckResult.ERROR)
+        self.instance_check._check_stonith_watchdog_timeout = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_check._check_stonith_timeout = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_check._check_sbd_delay_start_unset_dropin = Mock(return_value=sbd.CheckResult.SUCCESS)
+
+        res = self.instance_check.check_and_fix()
+        self.assertEqual(res, sbd.CheckResult.ERROR)
 
         mock_service_manager_inst.service_is_active.assert_called_once_with(constants.SBD_SERVICE)
         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()
-        self.instance_check._check_sbd_watchdog_timeout.assert_not_called()
 
+    @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_failure(self, mock_service_manager):
+    def test_check_and_fix_fix_failure(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me):
         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)
         self.instance_fix._check_config_consistency = Mock(return_value=True)
         self.instance_fix._load_configurations_from_runtime = Mock()
-        self.instance_fix._check_sbd_disk_metadata = Mock(side_effect=[False, False])
+        self.instance_fix._check_sbd_device_metadata_consistency = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_fix._check_sbd_disk_metadata = Mock(side_effect=[sbd.CheckResult.ERROR, sbd.CheckResult.ERROR])
         self.instance_fix._fix_sbd_disk_metadata = Mock()
 
         with self.assertRaises(sbd.FixFailure) as context:
             self.instance_fix.check_and_fix()
         self.assertTrue("Failed to fix SBD disk metadata" in str(context.exception))
 
+    @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):
+    def test_check_and_fix_fix_success(self, mock_service_manager, mock_check_all_nodes_reachable, mock_list_cluster_nodes_except_me):
         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)
         self.instance_fix._check_config_consistency = Mock(return_value=True)
         self.instance_fix._load_configurations_from_runtime = Mock()
+        self.instance_fix._check_sbd_device_metadata_consistency = Mock(return_value=sbd.CheckResult.SUCCESS)
         self.instance_fix._check_sbd_disk_metadata = Mock(return_value=sbd.CheckResult.SUCCESS)
         self.instance_fix._check_sbd_watchdog_timeout = Mock(return_value=sbd.CheckResult.SUCCESS)
         self.instance_fix._check_sbd_delay_start = Mock(return_value=sbd.CheckResult.SUCCESS)
         self.instance_fix._check_sbd_systemd_start_timeout = Mock(return_value=sbd.CheckResult.SUCCESS)
         self.instance_fix._check_stonith_watchdog_timeout = Mock(return_value=sbd.CheckResult.SUCCESS)
         self.instance_fix._check_stonith_timeout = Mock(return_value=sbd.CheckResult.SUCCESS)
+        self.instance_fix._check_sbd_delay_start_unset_dropin = Mock(return_value=sbd.CheckResult.SUCCESS)
 
         res = self.instance_fix.check_and_fix()
         self.assertEqual(res, sbd.CheckResult.SUCCESS)
@@ -343,29 +359,24 @@ def test_check_and_fix_fix_success(self, mock_service_manager):
         self.instance_fix._check_sbd_systemd_start_timeout.assert_called_once()
         self.instance_fix._check_stonith_watchdog_timeout.assert_called_once()
         self.instance_fix._check_stonith_timeout.assert_called_once()
+        self.instance_fix._check_sbd_delay_start_unset_dropin.assert_called_once()
 
-    @patch('crmsh.sbd.SBDUtils.check_devices_metadata_consistent')
-    @patch('crmsh.sbd.SBDUtils.get_sbd_device_from_config')
     @patch('logging.Logger.error')
     @patch('logging.Logger.warning')
     @patch('crmsh.utils.remote_diff_this')
     @patch('crmsh.corosync.conf')
-    @patch('crmsh.utils.list_cluster_nodes_except_me')
     @patch('crmsh.utils.this_node')
-    def test_check_config_consistency(self, mock_this_node, mock_list_cluster_nodes_except_me, mock_corosync_conf, mock_remote_diff_this, mock_logger_warning, mock_logger_error, mock_get_sbd_device_from_config, mock_check_devices_metadata_consistent):
+    def test_check_config_consistency(self, mock_this_node, mock_corosync_conf, mock_remote_diff_this, mock_logger_warning, mock_logger_error):
         mock_this_node.return_value = 'node1'
-        mock_list_cluster_nodes_except_me.return_value = ['node2', 'node3']
         mock_corosync_conf.return_value = "corosync.conf"
         mock_remote_diff_this.side_effect = ["diff data1", "diff data2"]
-        mock_get_sbd_device_from_config.return_value = ['/dev/sbd_device']
-        mock_check_devices_metadata_consistent.return_value = False
+        self.instance_check.peer_node_list = ["node2", "node3"]
 
         self.assertFalse(self.instance_check._check_config_consistency())
 
         mock_logger_error.assert_has_calls([
-            call('corosync.conf is not consistent across cluster nodes'),
+            call('%s is not consistent across cluster nodes', "corosync.conf"),
             call('%s is not consistent across cluster nodes', sbd.SBDManager.SYSCONFIG_SBD),
-            call('SBD device metadata is not consistent across cluster nodes')
         ])
 
     @patch('logging.Logger.error')
@@ -442,7 +453,7 @@ def test_check_sbd_systemd_start_timeout_failure(self, mock_get_sbd_systemd_star
         mock_get_sbd_systemd_start_timeout.return_value = 30
         self.instance_check.sbd_systemd_start_timeout_expected = 60
         self.assertEqual(self.instance_check._check_sbd_systemd_start_timeout(), sbd.CheckResult.ERROR)
-        mock_logger_error.assert_called_once_with("It's recommended that systemd start timeout for sbd.service is set to %ds, now is %ds", 60, 30)
+        mock_logger_error.assert_called_once_with("It's recommended that systemd start timeout for sbd.service is set to %ds, now is %ds on node %s", 60, 30, utils.this_node())
 
     @patch('crmsh.sbd.SBDTimeout.get_sbd_systemd_start_timeout')
     def test_check_sbd_systemd_start_timeout_success(self, mock_get_sbd_systemd_start_timeout):

test/unittests/test_ui_sbd.py

@@ -153,7 +153,7 @@ def test_configure_usage_none(self, mock_is_using_disk_based_sbd, mock_is_using_
     @mock.patch('crmsh.sbd.SBDUtils.is_using_disk_based_sbd')
     def test_configure_usage_disk_diskbased(self, mock_is_using_disk_based_sbd, mock_is_using_diskless_sbd):
         mock_is_using_disk_based_sbd.return_value = True
-        timeout_usage_str = " ".join([f"[{t}-timeout=<integer>]" for t in ui_sbd.SBD.TIMEOUT_TYPE_MINIMUMS])
+        timeout_usage_str = " ".join([f"[{t}-timeout=<integer>]" for t in ui_sbd.SBD.DISKBASED_TIMEOUT_TYPES])
         show_usage = f"crm sbd configure show [{'|'.join(ui_sbd.SBD.SHOW_TYPES)}]"
         expected = f"Usage:\n{show_usage}\ncrm sbd configure {timeout_usage_str} [watchdog-device=<device>]\n"
         self.assertEqual(self.sbd_instance_diskbased.configure_usage, expected)
@@ -165,7 +165,7 @@ def test_configure_usage_disk_diskbased(self, mock_is_using_disk_based_sbd, mock
     def test_configure_usage_disk_diskless(self, mock_is_using_disk_based_sbd, mock_is_using_diskless_sbd):
         mock_is_using_disk_based_sbd.return_value = False
         mock_is_using_diskless_sbd.return_value = True
-        timeout_usage_str = " ".join([f"[{t}-timeout=<integer>]" for t in ui_sbd.SBD.DISKLESS_TIMEOUT_TYPE_MINIMUMS])
+        timeout_usage_str = " ".join([f"[{t}-timeout=<integer>]" for t in ui_sbd.SBD.DISKLESS_TIMEOUT_TYPES])
         show_usage = f"crm sbd configure show [{'|'.join(ui_sbd.SBD.DISKLESS_SHOW_TYPES)}]"
         expected = f"Usage:\n{show_usage}\ncrm sbd configure {timeout_usage_str} [watchdog-device=<device>]\n"
         self.assertEqual(self.sbd_instance_diskless.configure_usage, expected)
@@ -308,9 +308,11 @@ def test_parse_args_invalid_timeout_value(self):
             self.sbd_instance_diskbased._parse_args(["watchdog-timeout=xxx"])
         self.assertEqual(str(e.exception), "Invalid timeout value: xxx")
 
+    @mock.patch('crmsh.sbd.SBDTimeout.get_timeout_minimum_value')
     @mock.patch('logging.Logger.debug')
     @mock.patch('crmsh.watchdog.Watchdog.get_watchdog_device')
-    def test_parse_args(self, mock_get_watchdog_device, mock_logger_debug):
+    def test_parse_args(self, mock_get_watchdog_device, mock_logger_debug, mock_get_timeout_minimum_value):
+        mock_get_timeout_minimum_value.return_value = 5
         mock_get_watchdog_device.return_value = "/dev/watchdog0"
         args = self.sbd_instance_diskbased._parse_args(["watchdog-timeout=10", "watchdog-device=/dev/watchdog0"])
         self.assertEqual(args, {"watchdog": 10, "watchdog-device": "/dev/watchdog0"})

test/unittests/test_utils.py

@@ -992,7 +992,7 @@ def test_check_all_nodes_reachable_dead_nodes(mock_xml, mock_reachable):
 
     with pytest.raises(utils.DeadNodeError) as err:
         utils.check_all_nodes_reachable("testing")
-    assert err.value.dead_nodes == ["node2"]
+    assert err.value.summary.dead_nodes == ["node2"]
 
 
 @mock.patch('crmsh.utils.check_ssh_passwd_need')