From e8d95b4ba03e62658ece669d6389b71b8553df1f Mon Sep 17 00:00:00 2001

From: Ivan Devat <idevat@redhat.com>

Date: Wed, 8 Aug 2018 13:48:24 +0200

Subject: [PATCH] fix pcs cluster cib-push for old feature set

---

 pcs/cluster.py                 | 56 +++++++++++++++++++++++++++++++++++++-----

 pcs/lib/cib/test/test_tools.py | 29 ++++++++++++++++++++++

 pcs/lib/cib/tools.py           |  6 ++---

 pcs/lib/env.py                 | 10 ++++++--

 4 files changed, 90 insertions(+), 11 deletions(-)

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

index b4d49d27..e8c94ab8 100644

--- a/pcs/cluster.py

+++ b/pcs/cluster.py

@@ -35,6 +35,7 @@ from pcs import (

 )

 from pcs.utils import parallel_for_nodes

 from pcs.common import report_codes

+from pcs.common.tools import Version

 from pcs.cli.common.errors import (

     CmdLineInputError,

     ERR_NODE_LIST_AND_ALL_MUTUALLY_EXCLUSIVE,

@@ -46,6 +47,7 @@ from pcs.lib import (

     reports as lib_reports,

 )

 from pcs.lib.booth import sync as booth_sync

+from pcs.lib.cib.tools import VERSION_FORMAT

 from pcs.lib.commands.remote_node import _share_authkey, _destroy_pcmk_remote_env

 from pcs.lib.commands.quorum import _add_device_model_net

 from pcs.lib.communication.corosync import CheckCorosyncOffline

@@ -74,6 +76,7 @@ from pcs.lib.external import (

     NodeCommunicationException,

     node_communicator_exception_to_report_item,

 )

+from pcs.lib.env import  MIN_FEATURE_SET_VERSION_FOR_DIFF

 from pcs.lib.env_tools import get_nodes

 from pcs.lib.node import NodeAddresses

 from pcs.lib import node_communication_format

@@ -1566,21 +1569,62 @@ def cluster_push(argv):

     if diff_against:

         try:

-            xml.dom.minidom.parse(diff_against)

+            original_cib = xml.dom.minidom.parse(diff_against)

         except (EnvironmentError, xml.parsers.expat.ExpatError) as e:

             utils.err("unable to parse original cib: %s" % e)

+

+        def unable_to_diff(reason):

+            return error(

+                "unable to diff against original cib '{0}': {1}"

+                .format(diff_against, reason)

+            )

+

+        cib_element_list = original_cib.getElementsByTagName("cib")

+

+        if len(cib_element_list) != 1:

+            raise unable_to_diff("there is not exactly one 'cib' element")

+

+        crm_feature_set = cib_element_list[0].getAttribute("crm_feature_set")

+        if not crm_feature_set:

+            raise unable_to_diff(

+                "the 'cib' element is missing 'crm_feature_set' value"

+            )

+

+        match = re.match(VERSION_FORMAT, crm_feature_set)

+        if not match:

+            raise unable_to_diff(

+                "the attribute 'crm_feature_set' of the element 'cib' has an"

+                " invalid value: '{0}'".format(crm_feature_set)

+            )

+        crm_feature_set_version = Version(

+            int(match.group("major")),

+            int(match.group("minor")),

+            int(match.group("rev")) if match.group("rev") else None

+        )

+

+        if crm_feature_set_version < MIN_FEATURE_SET_VERSION_FOR_DIFF:

+            raise unable_to_diff(

+                (

+                    "the 'crm_feature_set' version is '{0}'"

+                    " but at least version '{1}' is required"

+                ).format(

+                    crm_feature_set_version,

+                    MIN_FEATURE_SET_VERSION_FOR_DIFF,

+                )

+            )

+

         runner = utils.cmd_runner()

         command = [

             "crm_diff", "--original", diff_against, "--new", filename,

             "--no-version"

         ]

-        patch, error, dummy_retval = runner.run(command)

+        patch, stderr, dummy_retval = runner.run(command)

         # dummy_retval == 1 means one of two things:

         # a) an error has occured

         # b) --original and --new differ

         # therefore it's of no use to see if an error occurred

-        if error.strip():

-            utils.err("unable to diff the CIBs:\n" + error)

+        if stderr.strip():

+            utils.err("unable to diff the CIBs:\n" + stderr)

         if not patch.strip():

             print(

                 "The new CIB is the same as the original CIB, nothing to push."

@@ -1588,9 +1632,9 @@ def cluster_push(argv):

             sys.exit(0)

         command = ["cibadmin", "--patch", "--xml-pipe"]

-        output, error, retval = runner.run(command, patch)

+        output, stderr, retval = runner.run(command, patch)

         if retval != 0:

-            utils.err("unable to push cib\n" + error + output)

+            utils.err("unable to push cib\n" + stderr + output)

     else:

         command = ["cibadmin", "--replace", "--xml-file", filename]

diff --git a/pcs/lib/cib/test/test_tools.py b/pcs/lib/cib/test/test_tools.py

index fab39ce7..2bdc7695 100644

--- a/pcs/lib/cib/test/test_tools.py

+++ b/pcs/lib/cib/test/test_tools.py

@@ -436,6 +436,21 @@ class GetPacemakerVersionByWhichCibWasValidatedTest(TestCase):

             )

         )

+    def test_invalid_version_at_end(self):

+        assert_raise_library_error(

+            lambda: lib.get_pacemaker_version_by_which_cib_was_validated(

+                etree.XML('<cib validate-with="pacemaker-1.2.3x"/>')

+            ),

+            (

+                severities.ERROR,

+                report_codes.CIB_LOAD_ERROR_BAD_FORMAT,

+                {

+                    "reason": "the attribute 'validate-with' of the element"

+                        " 'cib' has an invalid value: 'pacemaker-1.2.3x'"

+                }

+            )

+        )

+

     def test_no_revision(self):

         self.assertEqual(

             Version(1, 2),

@@ -507,6 +522,20 @@ class getCibCrmFeatureSet(TestCase):

             )

         )

+    def test_invalid_version_at_end(self):

+        assert_raise_library_error(

+            lambda: lib.get_cib_crm_feature_set(

+                etree.XML('<cib crm_feature_set="3.0.9x" />')

+            ),

+            fixture.error(

+                report_codes.CIB_LOAD_ERROR_BAD_FORMAT,

+                reason=(

+                    "the attribute 'crm_feature_set' of the element 'cib' has "

+                    "an invalid value: '3.0.9x'"

+                )

+            )

+        )

+

 find_group = partial(lib.find_element_by_tag_and_id, "group")

 class FindTagWithId(TestCase):

diff --git a/pcs/lib/cib/tools.py b/pcs/lib/cib/tools.py

index 2cff96f3..ab2a9df5 100644

--- a/pcs/lib/cib/tools.py

+++ b/pcs/lib/cib/tools.py

@@ -16,7 +16,7 @@ from pcs.lib.pacemaker.values import (

 )

 from pcs.lib.xml_tools import get_root, get_sub_element

-_VERSION_FORMAT = r"(?P<major>\d+)\.(?P<minor>\d+)(\.(?P<rev>\d+))?"

+VERSION_FORMAT = r"(?P<major>\d+)\.(?P<minor>\d+)(\.(?P<rev>\d+))?$"

 class IdProvider(object):

     """

@@ -289,7 +289,7 @@ def get_pacemaker_version_by_which_cib_was_validated(cib):

     return _get_cib_version(

         cib,

         "validate-with",

-        re.compile(r"pacemaker-{0}".format(_VERSION_FORMAT))

+        re.compile(r"pacemaker-{0}".format(VERSION_FORMAT))

     )

 def get_cib_crm_feature_set(cib, none_if_missing=False):

@@ -303,6 +303,6 @@ def get_cib_crm_feature_set(cib, none_if_missing=False):

     return _get_cib_version(

         cib,

         "crm_feature_set",

-        re.compile(_VERSION_FORMAT),

+        re.compile(r"^{0}".format(VERSION_FORMAT)),

         none_if_missing=none_if_missing

     )

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

index 86f67b64..3b2c06b6 100644

--- a/pcs/lib/env.py

+++ b/pcs/lib/env.py

@@ -57,6 +57,8 @@ from pcs.lib.pacemaker.values import get_valid_timeout_seconds

 from pcs.lib.tools import write_tmpfile

 from pcs.lib.xml_tools import etree_to_str

+MIN_FEATURE_SET_VERSION_FOR_DIFF = Version(3, 0, 9)

+

 class LibraryEnvironment(object):

     # pylint: disable=too-many-instance-attributes

@@ -211,10 +213,14 @@ class LibraryEnvironment(object):

         # only check the version if a CIB has been loaded, otherwise the push

         # fails anyway. By my testing it seems that only the source CIB's

         # version matters.

-        if self.__loaded_cib_diff_source_feature_set < Version(3, 0, 9):

+        if(

+            self.__loaded_cib_diff_source_feature_set

+            <

+            MIN_FEATURE_SET_VERSION_FOR_DIFF

+        ):

             self.report_processor.process(

                 reports.cib_push_forced_full_due_to_crm_feature_set(

-                    Version(3, 0, 9),

+                    MIN_FEATURE_SET_VERSION_FOR_DIFF,

                     self.__loaded_cib_diff_source_feature_set

                 )

             )

-- 

2.13.6