Attachment 1475608 Details for Bug 1488044 – fix for "pcs cluster cib-push"

[patch] fix for "pcs cluster cib-push"

0001-fix-pcs-cluster-cib-push-for-old-feature-set.patch (text/plain), 9.10 KB, created by Tomas Jelinek on 2018-08-13 15:31:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomas Jelinek

Created: 2018-08-13 15:31:21 UTC

Size: 9.10 KB

patch

obsolete

>From 68b1d94ac66a0b217d8303e2839115d0df77e528 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
>
>---
> CHANGELOG.md                   |  3 +++
> pcs/cluster.py                 | 56 +++++++++++++++++++++++++++++++++++++-----
> pcs/lib/cib/test/test_tools.py | 29 ++++++++++++++++++++++
> pcs/lib/cib/tools.py           |  6 ++---
> pcs/lib/env.py                 | 10 ++++++--
> 5 files changed, 93 insertions(+), 11 deletions(-)
>
>diff --git a/CHANGELOG.md b/CHANGELOG.md
>index bf0699d4..94407798 100644
>--- a/CHANGELOG.md
>+++ b/CHANGELOG.md
>@@ -7,9 +7,12 @@
> - Test of watchdog devices ([rhbz#1475318])
> - Possible race condition causing an HTTP 408 error when sending larger files
>   via pcs ([rhbz#1600169])
>+- Support `diff-against` option of `pcs cluster cib-push` only for
>+  crm\_feature\_set > 3.0.8 ([rhbz#1488044])
> 
> [rhbz#1462248]: https://bugzilla.redhat.com/show_bug.cgi?id=1462248
> [rhbz#1475318]: https://bugzilla.redhat.com/show_bug.cgi?id=1475318
>+[rhbz#1488044]: https://bugzilla.redhat.com/show_bug.cgi?id=1488044
> [rhbz#1600169]: https://bugzilla.redhat.com/show_bug.cgi?id=1600169
> 
> 
>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.11.0
>

Actions: View | Diff

Attachments on bug 1488044: 1380384 | 1475608