From 66b5e393aebd84b08047f33d09bc4cbce730e205 Mon Sep 17 00:00:00 2001

From: Ondrej Mular <omular@redhat.com>

Date: Tue, 23 Aug 2016 11:19:20 +0200

Subject: [PATCH] sbd: fix check if ATB is required when enabling sbd

---

 pcs/common/report_codes.py |   1 +

 pcs/lib/commands/sbd.py    |   3 +-

 pcs/lib/reports.py         |  12 +++

 pcs/lib/sbd.py             |  39 ++++++++-

 pcs/test/test_lib_sbd.py   | 193 +++++++--------------------------------------

 5 files changed, 80 insertions(+), 168 deletions(-)

diff --git a/pcs/common/report_codes.py b/pcs/common/report_codes.py

index 5e46a1f..e6a86ec 100644

--- a/pcs/common/report_codes.py

+++ b/pcs/common/report_codes.py

@@ -155,6 +155,7 @@ SBD_DISABLING_STARTED = "SBD_DISABLING_STARTED"

 SBD_ENABLING_STARTED = "SBD_ENABLING_STARTED"

 SBD_NOT_INSTALLED = "SBD_NOT_INSTALLED"

 SBD_NOT_ENABLED = "SBD_NOT_ENABLED"

+SBD_REQUIRES_ATB = "SBD_REQUIRES_ATB"

 SERVICE_DISABLE_ERROR = "SERVICE_DISABLE_ERROR"

 SERVICE_DISABLE_STARTED = "SERVICE_DISABLE_STARTED"

 SERVICE_DISABLE_SUCCESS = "SERVICE_DISABLE_SUCCESS"

diff --git a/pcs/lib/commands/sbd.py b/pcs/lib/commands/sbd.py

index 265ebb5..2acb104 100644

--- a/pcs/lib/commands/sbd.py

+++ b/pcs/lib/commands/sbd.py

@@ -159,7 +159,8 @@ def enable_sbd(

     # enable ATB if needed

     corosync_conf = lib_env.get_corosync_conf()

-    if sbd.atb_has_to_be_enabled(lib_env.cmd_runner(), corosync_conf):

+    if sbd.atb_has_to_be_enabled_pre_enable_check(corosync_conf):

+        lib_env.report_processor.process(reports.sbd_requires_atb())

         corosync_conf.set_quorum_options(

             lib_env.report_processor, {"auto_tie_breaker": "1"}

         )

diff --git a/pcs/lib/reports.py b/pcs/lib/reports.py

index 568bb7e..a701679 100644

--- a/pcs/lib/reports.py

+++ b/pcs/lib/reports.py

@@ -1928,3 +1928,15 @@ def quorum_cannot_disable_atb_due_to_sbd(

         "unable to disable auto_tie_breaker: SBD fencing will have no effect",

         forceable=forceable

     )

+

+

+def sbd_requires_atb():

+    """

+    Warning that ATB will be enabled in order to make SBD fencing effective.

+    """

+    return ReportItem.warning(

+        report_codes.SBD_REQUIRES_ATB,

+        "auto_tie_breaker quorum option will be enabled to make SBD fencing "

+        "effective. Cluster has to be offline to be able to make this change."

+    )

+

diff --git a/pcs/lib/sbd.py b/pcs/lib/sbd.py

index c9f013b..39de740 100644

--- a/pcs/lib/sbd.py

+++ b/pcs/lib/sbd.py

@@ -46,6 +46,25 @@ def _run_parallel_and_raise_lib_error_on_failure(func, param_list):

         raise LibraryError(*report_list)

+def _even_number_of_nodes_and_no_qdevice(

+    corosync_conf_facade, node_number_modifier=0

+):

+    """

+    Returns True whenever cluster has no quorum device configured and number of

+    nodes + node_number_modifier is even number, False otherwise.

+

+    corosync_conf_facade --

+    node_number_modifier -- this value will be added to current number of nodes.

+        This can be useful to test whenever is ATB needed when adding/removing

+        node.

+    """

+    return (

+        not corosync_conf_facade.has_quorum_device()

+        and

+        (len(corosync_conf_facade.get_nodes()) + node_number_modifier) % 2 == 0

+    )

+

+

 def is_auto_tie_breaker_needed(

     runner, corosync_conf_facade, node_number_modifier=0

 ):

@@ -60,15 +79,29 @@ def is_auto_tie_breaker_needed(

         node.

     """

     return (

-        not corosync_conf_facade.has_quorum_device()

-        and

-        (len(corosync_conf_facade.get_nodes()) + node_number_modifier) % 2 == 0

+        _even_number_of_nodes_and_no_qdevice(

+            corosync_conf_facade, node_number_modifier

+        )

         and

         is_sbd_installed(runner)

         and

         is_sbd_enabled(runner)

     )

+

+def atb_has_to_be_enabled_pre_enable_check(corosync_conf_facade):

+    """

+    Returns True whenever quorum option auto_tie_breaker is needed to be enabled

+    for proper working of SBD fencing. False if it is not needed. This function

+    doesn't check if sbd is installed nor enabled.

+     """

+    return (

+        not corosync_conf_facade.is_enabled_auto_tie_breaker()

+        and

+        _even_number_of_nodes_and_no_qdevice(corosync_conf_facade)

+    )

+

+

 def atb_has_to_be_enabled(runner, corosync_conf_facade, node_number_modifier=0):

     """

     Return True whenever quorum option auto tie breaker has to be enabled for

diff --git a/pcs/test/test_lib_sbd.py b/pcs/test/test_lib_sbd.py

index fd29484..516e0bd 100644

--- a/pcs/test/test_lib_sbd.py

+++ b/pcs/test/test_lib_sbd.py

@@ -86,195 +86,60 @@ class RunParallelAndRaiseLibErrorOnFailureTest(TestCase):

         )

-@mock.patch("pcs.lib.sbd.is_sbd_installed")

-@mock.patch("pcs.lib.sbd.is_sbd_enabled")

-class IsAutoTieBreakerNeededTest(TestCase):

+class EvenNumberOfNodesAndNoQdevice(TestCase):

     def setUp(self):

-        self.runner = "runner"

         self.mock_corosync_conf = mock.MagicMock(spec_set=CorosyncConfigFacade)

     def _set_ret_vals(self, nodes, qdevice):

         self.mock_corosync_conf.get_nodes.return_value = nodes

         self.mock_corosync_conf.has_quorum_device.return_value = qdevice

-    def test_sbd_enabled_even_nodes_has_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2], True)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

-        ))

-

-    def test_sbd_enabled_even_nodes_no_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2], False)

-        self.assertTrue(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

-        ))

-

-    def test_sbd_not_installed_even_nodes_no_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = False

-        mock_installed.return_value = False

+    def test_even_num_no_qdevice(self):

         self._set_ret_vals([1, 2], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

-        ))

-

-    def test_sbd_enabled_odd_nodes_has_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3], True)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

-        ))

-

-    def test_sbd_enabled_odd_nodes_no_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

+        self.assertTrue(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf

         ))

-    def test_sbd_disabled_even_nodes_has_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = False

-        mock_installed.return_value = True

+    def test_even_num_qdevice(self):

         self._set_ret_vals([1, 2], True)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

-        ))

-

-    def test_sbd_disabled_even_nodes_no_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = False

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

+        self.assertFalse(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf

         ))

-    def test_sbd_disabled_odd_nodes_has_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = False

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3], True)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

-        ))

-

-    def test_sbd_disabled_odd_nodes_no_qdevice(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = False

-        mock_installed.return_value = True

+    def test_odd_num_no_qdevice(self):

         self._set_ret_vals([1, 2, 3], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf

+        self.assertFalse(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf

         ))

-    def test_sbd_enabled_odd_nodes_no_qdevice_plus_node(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3], False)

-        self.assertTrue(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, 1

-        ))

-

-    def test_sbd_not_installed_odd_nodes_no_qdevice_plus_node(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = False

-        mock_installed.return_value = False

-        self._set_ret_vals([1, 2, 3], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, 1

-        ))

-

-    def test_sbd_enabled_odd_nodes_no_qdevice_minus_node(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3], False)

-        self.assertTrue(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, -1

-        ))

-

-    def test_sbd_enabled_odd_nodes_no_qdevice_plus_2_nodes(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, 2

-        ))

-

-    def test_sbd_enabled_odd_nodes_no_qdevice_minus_2_nodes(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, -2

+    def test_odd_num_qdevice(self):

+        self._set_ret_vals([1, 2, 3], True)

+        self.assertFalse(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf

         ))

-    def test_sbd_enabled_even_nodes_no_qdevice_plus_node(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

+    def test_even_num_no_qdevice_plus_one(self):

         self._set_ret_vals([1, 2], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, 1

+        self.assertFalse(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf, 1

         ))

-    def test_sbd_enabled_even_nodes_no_qdevice_minus_node(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2], False)

-        self.assertFalse(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, -1

+    def test_even_num_qdevice_plus_one(self):

+        self._set_ret_vals([1, 2], True)

+        self.assertFalse(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf, 1

         ))

-    def test_sbd_enabled_even_nodes_no_qdevice_plus_2_nodes(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2], False)

-        self.assertTrue(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, 2

+    def test_odd_num_no_qdevice_plus_one(self):

+        self._set_ret_vals([1, 2, 3], False)

+        self.assertTrue(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf, 1

         ))

-    def test_sbd_enabled_even_nodes_no_qdevice_minus_2_nodes(

-        self, mock_enabled, mock_installed

-    ):

-        mock_enabled.return_value = True

-        mock_installed.return_value = True

-        self._set_ret_vals([1, 2, 3, 4], False)

-        self.assertTrue(lib_sbd.is_auto_tie_breaker_needed(

-            self.runner, self.mock_corosync_conf, -2

+    def test_odd_num_qdevice_plus_one(self):

+        self._set_ret_vals([1, 2, 3], True)

+        self.assertFalse(lib_sbd._even_number_of_nodes_and_no_qdevice(

+            self.mock_corosync_conf, 1

         ))

-- 

1.8.3.1