From b3b7bf416724f424a280c63f94121fb47e7397e6 Mon Sep 17 00:00:00 2001

From: Tomas Jelinek <tojeline@redhat.com>

Date: Mon, 11 Dec 2017 08:54:26 +0100

Subject: [PATCH] warn when a stonith device has method=cycle set

---

 pcs/status.py                    | 21 ++++++++++++++--

 pcs/test/test_status.py          | 47 ++++++++++++++++++++++++-----------

 pcsd/cluster_entity.rb           | 22 +++++++++++++++++

 pcsd/test/cib1.xml               |  2 ++

 pcsd/test/test_cluster_entity.rb | 53 ++++++++++++++++++++++++++++++++++++++++

 5 files changed, 129 insertions(+), 16 deletions(-)

diff --git a/pcs/status.py b/pcs/status.py

index ec10d61..8a517be 100644

--- a/pcs/status.py

+++ b/pcs/status.py

@@ -125,6 +125,7 @@ def status_stonith_check():

     stonith_enabled = True

     stonith_devices = []

     stonith_devices_id_action = []

+    stonith_devices_id_method_cycle = []

     sbd_running = False

     cib = utils.get_cib_dom()

@@ -155,6 +156,14 @@ def status_stonith_check():

                             stonith_devices_id_action.append(

                                 resource.getAttribute("id")

                             )

+                        if (

+                            nvpair.getAttribute("name") == "method"

+                            and

+                            nvpair.getAttribute("value") == "cycle"

+                        ):

+                            stonith_devices_id_method_cycle.append(

+                                resource.getAttribute("id")

+                            )

     if not utils.usefile:

         # check if SBD daemon is running

@@ -171,14 +180,22 @@ def status_stonith_check():

     if stonith_devices_id_action:

         print(

-            "WARNING: following stonith devices have the 'action' attribute"

-            " set, it is recommended to set {0} instead: {1}".format(

+            "WARNING: following stonith devices have the 'action' option set, "

+            "it is recommended to set {0} instead: {1}".format(

                 ", ".join(

                     ["'{0}'".format(x) for x in _STONITH_ACTION_REPLACED_BY]

                 ),

                 ", ".join(sorted(stonith_devices_id_action))

             )

         )

+    if stonith_devices_id_method_cycle:

+        print(

+            "WARNING: following stonith devices have the 'method' option set "

+            "to 'cycle' which is potentially dangerous, please consider using "

+            "'onoff': {0}".format(

+                ", ".join(sorted(stonith_devices_id_method_cycle))

+            )

+        )

 # Parse crm_mon for status

 def nodes_status(argv):

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

index b412b91..1a4fb70 100644

--- a/pcs/test/test_status.py

+++ b/pcs/test/test_status.py

@@ -21,45 +21,64 @@ class StonithWarningTest(TestCase, AssertPcsMixin):

         shutil.copy(self.empty_cib, self.temp_cib)

         self.pcs_runner = PcsRunner(self.temp_cib)

-    def fixture_stonith(self, action=False):

+    def fixture_stonith_action(self):

         self.assert_pcs_success(

-            "stonith create S fence_apc ipaddr=i login=l {0} --force".format(

-                "action=reboot" if action else ""

-            ),

+            "stonith create Sa fence_apc ipaddr=i login=l action=reboot --force",

             "Warning: stonith option 'action' is deprecated and should not be"

                 " used, use pcmk_off_action, pcmk_reboot_action instead\n"

-            if action

-            else ""

+        )

+

+    def fixture_stonith_cycle(self):

+        self.assert_pcs_success(

+            "stonith create Sc fence_ipmilan method=cycle"

         )

     def fixture_resource(self):

         self.assert_pcs_success(

-            "resource create dummy ocf:pacemaker:Dummy action=reboot --force",

-            "Warning: invalid resource option 'action', allowed options are: "

-                "envfile, fail_start_on, fake, op_sleep, passwd, state,"

-                " trace_file, trace_ra\n"

+            "resource create dummy ocf:pacemaker:Dummy action=reboot "

+                "method=cycle --force"

+            ,

+            "Warning: invalid resource options: 'action', 'method', allowed "

+                "options are: envfile, fail_start_on, fake, op_sleep, passwd, "

+                "state, trace_file, trace_ra\n"

         )

     def test_warning_stonith_action(self):

-        self.fixture_stonith(action=True)

+        self.fixture_stonith_action()

+        self.fixture_resource()

         self.assert_pcs_success(

             "status",

             stdout_start=dedent("""\

                 Cluster name: test99

-                WARNING: following stonith devices have the 'action' attribute set, it is recommended to set 'pcmk_off_action', 'pcmk_reboot_action' instead: S

+                WARNING: following stonith devices have the 'action' option set, it is recommended to set 'pcmk_off_action', 'pcmk_reboot_action' instead: Sa

                 Stack: unknown

                 Current DC: NONE

             """)

         )

-    def test_action_ignored_for_non_stonith_resources(self):

-        self.fixture_stonith(action=False)

+    def test_warning_stonith_method_cycle(self):

+        self.fixture_stonith_cycle()

         self.fixture_resource()

+        self.assert_pcs_success(

+            "status",

+            stdout_start=dedent("""\

+                Cluster name: test99

+                WARNING: following stonith devices have the 'method' option set to 'cycle' which is potentially dangerous, please consider using 'onoff': Sc

+                Stack: unknown

+                Current DC: NONE

+            """)

+        )

+    def test_stonith_warnings(self):

+        self.fixture_stonith_action()

+        self.fixture_stonith_cycle()

+        self.fixture_resource()

         self.assert_pcs_success(

             "status",

             stdout_start=dedent("""\

                 Cluster name: test99

+                WARNING: following stonith devices have the 'action' option set, it is recommended to set 'pcmk_off_action', 'pcmk_reboot_action' instead: Sa

+                WARNING: following stonith devices have the 'method' option set to 'cycle' which is potentially dangerous, please consider using 'onoff': Sc

                 Stack: unknown

                 Current DC: NONE

             """)

diff --git a/pcsd/cluster_entity.rb b/pcsd/cluster_entity.rb

index 21092c5..3675719 100644

--- a/pcsd/cluster_entity.rb

+++ b/pcsd/cluster_entity.rb

@@ -516,6 +516,28 @@ module ClusterEntity

           @utilization << ClusterEntity::NvPair.from_dom(e)

         }

         @stonith = @_class == 'stonith'

+        if @stonith

+          @instance_attr.each{ |attr|

+            if attr.name == 'action'

+              @warning_list << {

+                :message => (

+                  'This fence-device has the "action" option set, it is ' +

+                  'recommended to set "pcmk_off_action", "pcmk_reboot_action" ' +

+                  'instead'

+                )

+              }

+            end

+            if attr.name == 'method' and attr.value == 'cycle'

+              @warning_list << {

+                :message => (

+                  'This fence-device has the "method" option set to "cycle" ' +

+                  'which is potentially dangerous, please consider using ' +

+                  '"onoff"'

+                )

+              }

+            end

+          }

+        end

         if @id and rsc_status

           @crm_status = rsc_status[@id] || []

         end

diff --git a/pcsd/test/cib1.xml b/pcsd/test/cib1.xml

index f603f24..03749ab 100644

--- a/pcsd/test/cib1.xml

+++ b/pcsd/test/cib1.xml

@@ -28,6 +28,8 @@

       <primitive class="stonith" id="node2-stonith" type="fence_xvm">

         <instance_attributes id="node2-stonith-instance_attributes">

           <nvpair id="node2-stonith-instance_attributes-domain" name="domain" value="node2"/>

+          <nvpair id="node2-stonith-instance_attributes-action" name="action" value="monitor"/>

+          <nvpair id="node2-stonith-instance_attributes-method" name="method" value="cycle"/>

         </instance_attributes>

         <operations>

           <op id="node2-stonith-monitor-interval-60s" interval="60s" name="monitor"/>

diff --git a/pcsd/test/test_cluster_entity.rb b/pcsd/test/test_cluster_entity.rb

index 2b67e19..60719ef 100644

--- a/pcsd/test/test_cluster_entity.rb

+++ b/pcsd/test/test_cluster_entity.rb

@@ -719,6 +719,59 @@ class TestPrimitive < Test::Unit::TestCase

     assert(obj.operations.empty?)

   end

+  def test_init_stonith_with_warnings

+    obj = ClusterEntity::Primitive.new(

+      @cib.elements["//primitive[@id='node2-stonith']"]

+    )

+    assert_nil(obj.parent)

+    assert_nil(obj.get_master)

+    assert_nil(obj.get_clone)

+    assert_nil(obj.get_group)

+    assert(obj.meta_attr.empty?)

+    assert_equal('node2-stonith', obj.id)

+    assert(obj.error_list.empty?)

+    assert_equal(

+      obj.warning_list,

+      [

+        {

+          :message => (

+            'This fence-device has the "action" option set, it is ' +

+            'recommended to set "pcmk_off_action", "pcmk_reboot_action" instead'

+          )

+        },

+        {

+          :message => (

+            'This fence-device has the "method" option set to "cycle" which ' +

+            'is potentially dangerous, please consider using "onoff"'

+          )

+        }

+      ]

+    )

+    assert_equal('stonith:fence_xvm', obj.agentname)

+    assert_equal('stonith', obj._class)

+    assert_nil(obj.provider)

+    assert_equal('fence_xvm', obj.type)

+    assert(obj.stonith)

+    instance_attr = ClusterEntity::NvSet.new << ClusterEntity::NvPair.new(

+      'node2-stonith-instance_attributes-domain',

+      'domain',

+      'node2'

+    )

+    instance_attr << ClusterEntity::NvPair.new(

+      'node2-stonith-instance_attributes-action',

+      'action',

+      'monitor'

+    )

+    instance_attr << ClusterEntity::NvPair.new(

+      'node2-stonith-instance_attributes-method',

+      'method',

+      'cycle'

+    )

+    assert_equal_NvSet(instance_attr, obj.instance_attr)

+    assert(obj.crm_status.empty?)

+    assert(obj.operations.empty?)

+  end

+

   def test_init_stonith_with_crm

     obj = ClusterEntity::Primitive.new(

       @cib.elements["//primitive[@id='node1-stonith']"],

-- 

1.8.3.1