From e8d95b4ba03e62658ece669d6389b71b8553df1f Mon Sep 17 00:00:00 2001 From: Ivan Devat 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('') + ), + ( + 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('') + ), + 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\d+)\.(?P\d+)(\.(?P\d+))?" +VERSION_FORMAT = r"(?P\d+)\.(?P\d+)(\.(?P\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