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