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="C-clone" score="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"/>

             """,

             """

-            

-                score="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="AGroup" score="INFINITY"/>

+        

+            node="rh7-1" rsc="TagGroupOnly" score="INFINITY"/>

+    </constraints>

+"""

+

+FIXTURE_CLONE_TAG_CONSTRAINTS = """

+    <constraints>

+        

+            rsc="AGroup-clone" score="INFINITY"

+        />

+        

+            node="rh7-1" rsc="TagGroupOnly" score="INFINITY"

+        />

+    </constraints>

+"""

+

+

+FIXTURE_CLONE_CONSTRAINT = """

+    <constraints>

+        

+            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"/>

             """,

             """

-            

-                score="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