From 4a986e8ee0610b1c85a04e38042e4073d41207a4 Mon Sep 17 00:00:00 2001
From: Miroslav Lisik <mlisik@redhat.com>
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 = """
+ <constraints>
+ <rsc_location id="location-C-clone-rh7-1-INFINITY" node="rh7-1"
+ rsc="C-clone" score="INFINITY"/>
+ <rsc_location id="location-TagCloneOnly-rh7-1-INFINITY"
+ node="rh7-1" rsc="TagCloneOnly" score="INFINITY"/>
+ </constraints>
+"""
+
+
+FIXTURE_TAGS_CONFIG_XML = """
+ <tags>
+ <tag id="TagCloneOnly">
+ <obj_ref id="C-clone"/>
+ </tag>
+ <tag id="TagNotCloneOnly">
+ <obj_ref id="C-clone"/>
+ <obj_ref id="Dummy"/>
+ </tag>
+ </tags>
+"""
+
+
+FIXTURE_TAGS_RESULT_XML = """
+ <tags>
+ <tag id="TagNotCloneOnly">
+ <obj_ref id="Dummy"/>
+ </tag>
+ </tags>
+"""
+
+
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",
- """
- <tags>
- <tag id="T1">
- <obj_ref id="C-clone"/>
- <obj_ref id="Dummy"/>
- </tag>
- <tag id="T2">
- <obj_ref id="C-clone"/>
- </tag>
- </tags>
- """,
+ "configuration", FIXTURE_TAGS_CONFIG_XML,
)
xml_manip.append_to_first_tag_name(
"constraints",
@@ -95,8 +132,8 @@ class Unclone(
rsc="C-clone" score="INFINITY"/>
""",
"""
- <rsc_location id="location-T1-rh7-1-INFINITY" node="rh7-1" rsc="T1"
- score="INFINITY"/>
+ <rsc_location id="location-TagCloneOnly-rh7-1-INFINITY"
+ node="rh7-1" rsc="TagCloneOnly" 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("<constraints/>")
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("<constraints/>")
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 = """
+ <constraints>
+ <rsc_location id="location-AGroup-rh7-1-INFINITY" node="rh7-1"
+ rsc="AGroup" score="INFINITY"/>
+ <rsc_location id="location-TagGroupOnly-rh7-1-INFINITY"
+ node="rh7-1" rsc="TagGroupOnly" score="INFINITY"/>
+ </constraints>
+"""
+
+FIXTURE_CLONE_TAG_CONSTRAINTS = """
+ <constraints>
+ <rsc_location id="location-AGroup-rh7-1-INFINITY" node="rh7-1"
+ rsc="AGroup-clone" score="INFINITY"
+ />
+ <rsc_location id="location-TagGroupOnly-rh7-1-INFINITY"
+ node="rh7-1" rsc="TagGroupOnly" score="INFINITY"
+ />
+ </constraints>
+"""
+
+
+FIXTURE_CLONE_CONSTRAINT = """
+ <constraints>
+ <rsc_location id="location-AGroup-rh7-1-INFINITY" node="rh7-1"
+ rsc="AGroup-clone" score="INFINITY"
+ />
+ </constraints>
+"""
+
+
+FIXTURE_TAGS_CONFIG_XML = """
+ <tags>
+ <tag id="TagGroupOnly">
+ <obj_ref id="AGroup"/>
+ </tag>
+ <tag id="TagNotGroupOnly">
+ <obj_ref id="AGroup"/>
+ <obj_ref id="A1"/>
+ <obj_ref id="A2"/>
+ <obj_ref id="A3"/>
+ </tag>
+ </tags>
+"""
+
+
+FIXTURE_TAGS_RESULT_XML = """
+ <tags>
+ <tag id="TagNotGroupOnly">
+ <obj_ref id="A1"/>
+ <obj_ref id="A2"/>
+ <obj_ref id="A3"/>
+ </tag>
+ </tags>
+"""
+
+
+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",
- """
- <tags>
- <tag id="T1">
- <obj_ref id="AGroup"/>
- </tag>
- <tag id="T2">
- <obj_ref id="AGroup"/>
- </tag>
- </tags>
- """,
+ "configuration", FIXTURE_TAGS_CONFIG_XML,
)
xml_manip.append_to_first_tag_name(
"constraints",
@@ -100,8 +139,8 @@ class TestGroupMixin(
rsc="AGroup" score="INFINITY"/>
""",
"""
- <rsc_location id="location-T1-rh7-1-INFINITY" node="rh7-1" rsc="T1"
- score="INFINITY"/>
+ <rsc_location id="location-TagGroupOnly-rh7-1-INFINITY"
+ node="rh7-1" rsc="TagGroupOnly" 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("<constraints/>")
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("<constraints/>")
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