From 4a986e8ee0610b1c85a04e38042e4073d41207a4 Mon Sep 17 00:00:00 2001 From: Miroslav Lisik Date: Mon, 13 Jul 2020 12:59:09 +0200 Subject: [PATCH 2/3] Fix tag removal in resource 'unclone/ungroup' commands and extend test coverage --- pcs/resource.py | 2 +- .../tier1/cib_resource/test_clone_unclone.py | 73 +++++++-- .../tier1/cib_resource/test_group_ungroup.py | 143 +++++++++++++++--- pcs_test/tools/cib.py | 10 +- 4 files changed, 187 insertions(+), 41 deletions(-) diff --git a/pcs/resource.py b/pcs/resource.py index 9a3bd0ee..49d28ef0 100644 --- a/pcs/resource.py +++ b/pcs/resource.py @@ -2027,7 +2027,7 @@ def remove_resource_references( if obj_ref.getAttribute("id") == resource_id: tag = obj_ref.parentNode tag.removeChild(obj_ref) - if tag.getElementsByTagName(obj_ref).length == 0: + if tag.getElementsByTagName("obj_ref").length == 0: remove_resource_references( dom, tag.getAttribute("id"), output=output, ) diff --git a/pcs_test/tier1/cib_resource/test_clone_unclone.py b/pcs_test/tier1/cib_resource/test_clone_unclone.py index c9c6a29e..2633801a 100644 --- a/pcs_test/tier1/cib_resource/test_clone_unclone.py +++ b/pcs_test/tier1/cib_resource/test_clone_unclone.py @@ -55,6 +55,38 @@ FIXTURE_RESOURCES = """ ) +FIXTURE_CONSTRAINTS_CONFIG_XML = """ + + + + +""" + + +FIXTURE_TAGS_CONFIG_XML = """ + + + + + + + + + +""" + + +FIXTURE_TAGS_RESULT_XML = """ + + + + + +""" + + class Unclone( TestCase, get_assert_pcs_effect_mixin( @@ -66,6 +98,22 @@ class Unclone( ): empty_cib = rc("cib-empty.xml") + def assert_tags_xml(self, expected_xml): + self.assert_resources_xml_in_cib( + expected_xml, + get_cib_part_func=lambda cib: etree.tostring( + etree.parse(cib).findall(".//tags")[0], + ), + ) + + def assert_constraint_xml(self, expected_xml): + self.assert_resources_xml_in_cib( + expected_xml, + get_cib_part_func=lambda cib: etree.tostring( + etree.parse(cib).findall(".//constraints")[0], + ), + ) + def setUp(self): # pylint: disable=invalid-name self.temp_cib = get_tmp_file("tier1_cib_resource_group_ungroup") @@ -75,18 +123,7 @@ class Unclone( "resources", FIXTURE_CLONE, FIXTURE_DUMMY, ) xml_manip.append_to_first_tag_name( - "configuration", - """ - - - - - - - - - - """, + "configuration", FIXTURE_TAGS_CONFIG_XML, ) xml_manip.append_to_first_tag_name( "constraints", @@ -95,8 +132,8 @@ class Unclone( rsc="C-clone" score="INFINITY"/> """, """ - + """, ) write_data_to_tmpfile(str(xml_manip), self.temp_cib) @@ -111,6 +148,8 @@ class Unclone( "Error: could not find resource: NonExistentClone\n", ) self.assert_resources_xml_in_cib(FIXTURE_CLONE_AND_RESOURCE) + self.assert_tags_xml(FIXTURE_TAGS_CONFIG_XML) + self.assert_constraint_xml(FIXTURE_CONSTRAINTS_CONFIG_XML) def test_not_clone_resource(self): self.assert_pcs_fail( @@ -118,9 +157,15 @@ class Unclone( "Error: 'Dummy' is not a clone resource\n", ) self.assert_resources_xml_in_cib(FIXTURE_CLONE_AND_RESOURCE) + self.assert_tags_xml(FIXTURE_TAGS_CONFIG_XML) + self.assert_constraint_xml(FIXTURE_CONSTRAINTS_CONFIG_XML) def test_unclone_clone_id(self): self.assert_effect("resource unclone C-clone", FIXTURE_RESOURCES) + self.assert_tags_xml(FIXTURE_TAGS_RESULT_XML) + self.assert_constraint_xml("") def test_unclone_resoruce_id(self): self.assert_effect("resource unclone C", FIXTURE_RESOURCES) + self.assert_tags_xml(FIXTURE_TAGS_RESULT_XML) + self.assert_constraint_xml("") diff --git a/pcs_test/tier1/cib_resource/test_group_ungroup.py b/pcs_test/tier1/cib_resource/test_group_ungroup.py index f86e9890..88cc315d 100644 --- a/pcs_test/tier1/cib_resource/test_group_ungroup.py +++ b/pcs_test/tier1/cib_resource/test_group_ungroup.py @@ -64,14 +64,63 @@ FIXTURE_AGROUP_XML = fixture_group_xml( ) -class TestGroupMixin( - get_assert_pcs_effect_mixin( - lambda cib: etree.tostring( - # pylint:disable=undefined-variable - etree.parse(cib).findall(".//resources")[0] - ) - ), -): +FIXTURE_CONSTRAINTS_CONFIG_XML = """ + + + + +""" + +FIXTURE_CLONE_TAG_CONSTRAINTS = """ + + + + +""" + + +FIXTURE_CLONE_CONSTRAINT = """ + + + +""" + + +FIXTURE_TAGS_CONFIG_XML = """ + + + + + + + + + + + +""" + + +FIXTURE_TAGS_RESULT_XML = """ + + + + + + + +""" + + +class TestGroupMixin: empty_cib = rc("cib-empty.xml") def setUp(self): @@ -81,17 +130,7 @@ class TestGroupMixin( xml_manip = XmlManipulation.from_file(self.empty_cib) xml_manip.append_to_first_tag_name("resources", FIXTURE_AGROUP_XML) xml_manip.append_to_first_tag_name( - "configuration", - """ - - - - - - - - - """, + "configuration", FIXTURE_TAGS_CONFIG_XML, ) xml_manip.append_to_first_tag_name( "constraints", @@ -100,8 +139,8 @@ class TestGroupMixin( rsc="AGroup" score="INFINITY"/> """, """ - + """, ) write_data_to_tmpfile(str(xml_manip), self.temp_cib) @@ -111,9 +150,33 @@ class TestGroupMixin( self.temp_cib.close() -class GroupDeleteRemoveUngroupBase(TestGroupMixin): +class GroupDeleteRemoveUngroupBase( + get_assert_pcs_effect_mixin( + lambda cib: etree.tostring( + # pylint:disable=undefined-variable + etree.parse(cib).findall(".//resources")[0] + ) + ), + TestGroupMixin, +): command = None + def assert_tags_xml(self, expected_xml): + self.assert_resources_xml_in_cib( + expected_xml, + get_cib_part_func=lambda cib: etree.tostring( + etree.parse(cib).findall(".//tags")[0], + ), + ) + + def assert_constraint_xml(self, expected_xml): + self.assert_resources_xml_in_cib( + expected_xml, + get_cib_part_func=lambda cib: etree.tostring( + etree.parse(cib).findall(".//constraints")[0], + ), + ) + def test_nonexistent_group(self): self.assert_pcs_fail( f"resource {self.command} NonExistentGroup", @@ -122,6 +185,8 @@ class GroupDeleteRemoveUngroupBase(TestGroupMixin): self.assert_resources_xml_in_cib( fixture_resources_xml([FIXTURE_AGROUP_XML]), ) + self.assert_tags_xml(FIXTURE_TAGS_CONFIG_XML) + self.assert_constraint_xml(FIXTURE_CONSTRAINTS_CONFIG_XML) def test_not_a_group_id(self): self.assert_pcs_fail( @@ -130,6 +195,8 @@ class GroupDeleteRemoveUngroupBase(TestGroupMixin): self.assert_resources_xml_in_cib( fixture_resources_xml([FIXTURE_AGROUP_XML]), ) + self.assert_tags_xml(FIXTURE_TAGS_CONFIG_XML) + self.assert_constraint_xml(FIXTURE_CONSTRAINTS_CONFIG_XML) def test_whole_group(self): self.assert_effect( @@ -142,10 +209,12 @@ class GroupDeleteRemoveUngroupBase(TestGroupMixin): ], ), output=( - "Removing Constraint - location-T1-rh7-1-INFINITY\n" + "Removing Constraint - location-TagGroupOnly-rh7-1-INFINITY\n" "Removing Constraint - location-AGroup-rh7-1-INFINITY\n" ), ) + self.assert_tags_xml(FIXTURE_TAGS_RESULT_XML) + self.assert_constraint_xml("") def test_specified_resources(self): self.assert_effect( @@ -160,6 +229,26 @@ class GroupDeleteRemoveUngroupBase(TestGroupMixin): ], ), ) + self.assert_tags_xml(FIXTURE_TAGS_CONFIG_XML) + self.assert_constraint_xml(FIXTURE_CONSTRAINTS_CONFIG_XML) + + def test_all_resources(self): + self.assert_effect( + f"resource {self.command} AGroup A1 A2 A3", + fixture_resources_xml( + [ + fixture_primitive_xml("A1"), + fixture_primitive_xml("A2"), + fixture_primitive_xml("A3"), + ], + ), + output=( + "Removing Constraint - location-TagGroupOnly-rh7-1-INFINITY\n" + "Removing Constraint - location-AGroup-rh7-1-INFINITY\n" + ), + ) + self.assert_tags_xml(FIXTURE_TAGS_RESULT_XML) + self.assert_constraint_xml("") def test_cloned_group(self): self.assert_pcs_success("resource clone AGroup") @@ -172,6 +261,8 @@ class GroupDeleteRemoveUngroupBase(TestGroupMixin): [fixture_clone_xml("AGroup-clone", FIXTURE_AGROUP_XML)], ) ) + self.assert_tags_xml(FIXTURE_TAGS_CONFIG_XML) + self.assert_constraint_xml(FIXTURE_CLONE_TAG_CONSTRAINTS) def test_cloned_group_all_resorces_specified(self): self.assert_pcs_success("resource clone AGroup") @@ -184,6 +275,8 @@ class GroupDeleteRemoveUngroupBase(TestGroupMixin): [fixture_clone_xml("AGroup-clone", FIXTURE_AGROUP_XML)], ) ) + self.assert_tags_xml(FIXTURE_TAGS_CONFIG_XML) + self.assert_constraint_xml(FIXTURE_CLONE_TAG_CONSTRAINTS) def test_cloned_group_with_one_resource(self): self.assert_pcs_success("resource clone AGroup") @@ -199,8 +292,10 @@ class GroupDeleteRemoveUngroupBase(TestGroupMixin): fixture_primitive_xml("A2"), ], ), - output="Removing Constraint - location-T1-rh7-1-INFINITY\n", + output="Removing Constraint - location-TagGroupOnly-rh7-1-INFINITY\n", ) + self.assert_tags_xml(FIXTURE_TAGS_RESULT_XML) + self.assert_constraint_xml(FIXTURE_CLONE_CONSTRAINT) class ResourceUngroup(GroupDeleteRemoveUngroupBase, TestCase): diff --git a/pcs_test/tools/cib.py b/pcs_test/tools/cib.py index d52176cf..5eaaa92e 100644 --- a/pcs_test/tools/cib.py +++ b/pcs_test/tools/cib.py @@ -30,8 +30,14 @@ def xml_format(xml_string): def get_assert_pcs_effect_mixin(get_cib_part): class AssertPcsEffectMixin(AssertPcsMixin): - def assert_resources_xml_in_cib(self, expected_xml_resources): - xml = get_cib_part(self.temp_cib) + def assert_resources_xml_in_cib( + self, expected_xml_resources, get_cib_part_func=None, + ): + self.temp_cib.seek(0) + if get_cib_part_func is not None: + xml = get_cib_part_func(self.temp_cib) + else: + xml = get_cib_part(self.temp_cib) try: assert_xml_equal(expected_xml_resources, xml.decode()) except AssertionError as e: -- 2.25.4