Disallow setting order constraints on resources within a group

CHANGELOG.md

@@ -23,7 +23,10 @@
   ([rhbz#2058243])
 - Preventing fence-loop caused when stonith-watchdog-timeout is set
   with wrong value ([rhbz#2058246])
+- Do not allow to create an order constraint for resources in one group as that
+  may block Pacemaker ([ghpull#509])
 
+[ghpull#509]: https://github.com/ClusterLabs/pcs/pull/509
 [rhbz#2024522]: https://bugzilla.redhat.com/show_bug.cgi?id=2024522
 [rhbz#2053177]: https://bugzilla.redhat.com/show_bug.cgi?id=2053177
 [rhbz#2054671]: https://bugzilla.redhat.com/show_bug.cgi?id=2054671

pcs/common/reports/codes.py

@@ -102,6 +102,9 @@
     "CANNOT_MOVE_RESOURCE_STOPPED_NO_NODE_SPECIFIED"
 )
 CANNOT_REMOVE_ALL_CLUSTER_NODES = M("CANNOT_REMOVE_ALL_CLUSTER_NODES")
+CANNOT_SET_ORDER_CONSTRAINTS_FOR_RESOURCES_IN_THE_SAME_GROUP = M(
+    "CANNOT_SET_ORDER_CONSTRAINTS_FOR_RESOURCES_IN_THE_SAME_GROUP"
+)
 CANNOT_UNMOVE_UNBAN_RESOURCE_MASTER_RESOURCE_NOT_PROMOTABLE = M(
     "CANNOT_UNMOVE_UNBAN_RESOURCE_MASTER_RESOURCE_NOT_PROMOTABLE"
 )

pcs/common/reports/messages.py

@@ -347,6 +347,22 @@ def message(self) -> str:
         return "Resource set list is empty"
 
 
+@dataclass(frozen=True)
+class CannotSetOrderConstraintsForResourcesInTheSameGroup(ReportItemMessage):
+    """
+    Can't set order constraint for resources in one group because the
+    start sequence of the resources is determined by their location in the group
+    """
+
+    _code = codes.CANNOT_SET_ORDER_CONSTRAINTS_FOR_RESOURCES_IN_THE_SAME_GROUP
+
+    @property
+    def message(self) -> str:
+        return (
+            "Cannot create an order constraint for resources in the same group"
+        )
+
+
 @dataclass(frozen=True)
 class RequiredOptionsAreMissing(ReportItemMessage):
     """

pcs/constraint.py

@@ -188,6 +188,15 @@ def _validate_constraint_resource(cib_dom, resource_id):
         utils.err(resource_error)
 
 
+def _validate_resources_not_in_same_group(cib_dom, resource1, resource2):
+    if not utils.validate_resources_not_in_same_group(
+        cib_dom, resource1, resource2
+    ):
+        utils.err(
+            "Cannot create an order constraint for resources in the same group"
+        )
+
+
 # Syntax: colocation add [role] <src> with [role] <tgt> [score] [options]
 # possible commands:
 #        <src> with        <tgt> [score] [options]
@@ -478,6 +487,8 @@ def _order_add(resource1, resource2, options_list, modifiers):
     _validate_constraint_resource(cib_dom, resource1)
     _validate_constraint_resource(cib_dom, resource2)
 
+    _validate_resources_not_in_same_group(cib_dom, resource1, resource2)
+
     order_options = []
     id_specified = False
     sym = None

pcs/lib/cib/constraint/constraint.py

@@ -24,6 +24,7 @@
 from pcs.lib.xml_tools import (
     export_attributes,
     find_parent,
+    get_root,
 )
 
 
@@ -185,6 +186,13 @@ def create_with_set(constraint_section, tag_name, options, resource_set_list):
         )
     element = SubElement(constraint_section, tag_name)
     element.attrib.update(options)
+    if tag_name == "rsc_order":
+        all_resource_ids = []
+        for resource_set_item in resource_set_list:
+            all_resource_ids.extend(resource_set_item["ids"])
+        resource_set.is_resource_in_same_group(
+            get_root(constraint_section), all_resource_ids
+        )
     for resource_set_item in resource_set_list:
         resource_set.create(element, resource_set_item)
     return element

pcs/lib/cib/constraint/resource_set.py

@@ -5,10 +5,14 @@
     pacemaker,
     reports,
 )
+from pcs.common.reports.item import ReportItem
 from pcs.lib import validate
+from pcs.lib.cib.resource import group
+from pcs.lib.cib.resource.common import get_parent_resource
 from pcs.lib.cib.tools import (
     are_new_role_names_supported,
     find_unique_id,
+    get_elements_by_ids,
 )
 from pcs.lib.errors import LibraryError
 from pcs.lib.xml_tools import export_attributes
@@ -87,3 +91,23 @@ def export(element):
         "ids": get_resource_id_set_list(element),
         "options": export_attributes(element),
     }
+
+
+def is_resource_in_same_group(cib, resource_id_list):
+    # We don't care about not found elements here, that is a job of another
+    # validator. We do not care if the id doesn't belong to a resource either
+    # for the same reason.
+    element_list, _ = get_elements_by_ids(cib, set(resource_id_list))
+
+    parent_list = []
+    for element in element_list:
+        parent = get_parent_resource(element)
+        if parent is not None and group.is_group(parent):
+            parent_list.append(parent)
+
+    if len(set(parent_list)) != len(parent_list):
+        raise LibraryError(
+            ReportItem.error(
+                reports.messages.CannotSetOrderConstraintsForResourcesInTheSameGroup()
+            )
+        )

pcs/utils.py

@@ -1424,6 +1424,21 @@ def validate_constraint_resource(dom, resource_id):
     return True, "", resource_id
 
 
+def validate_resources_not_in_same_group(dom, resource_id1, resource_id2):
+    resource_el1 = dom_get_resource(dom, resource_id1)
+    resource_el2 = dom_get_resource(dom, resource_id2)
+    if not resource_el1 or not resource_el2:
+        # Only primitive resources can be in a group. If at least one of the
+        # resources is not a primitive (resource_el is None), then the
+        # resources are not in the same group.
+        return True
+    group1 = dom_get_parent_by_tag_names(resource_el1, ["group"])
+    group2 = dom_get_parent_by_tag_names(resource_el2, ["group"])
+    if not group1 or not group2:
+        return True
+    return group1 != group2
+
+
 def dom_get_resource_remote_node_name(dom_resource):
     """
     Commandline options: no options

pcs_test/Makefile.am

@@ -230,6 +230,7 @@ EXTRA_DIST		= \
 			  tier0/lib/commands/test_booth.py \
 			  tier0/lib/commands/test_cib_options.py \
 			  tier0/lib/commands/test_constraint_common.py \
+			  tier0/lib/commands/test_constraint_order.py \
 			  tier0/lib/commands/test_fencing_topology.py \
 			  tier0/lib/commands/test_node.py \
 			  tier0/lib/commands/test_pcsd.py \

pcs_test/tier0/common/reports/test_messages.py

@@ -118,6 +118,14 @@ def test_success(self):
         )
 
 
+class CannotSetOrderConstraintsForResourcesInTheSameGroup(NameBuildTest):
+    def test_success(self):
+        self.assert_message_from_report(
+            "Cannot create an order constraint for resources in the same group",
+            reports.CannotSetOrderConstraintsForResourcesInTheSameGroup(),
+        )
+
+
 class RequiredOptionsAreMissing(NameBuildTest):
     def test_build_message_with_type(self):
         self.assert_message_from_report(

pcs_test/tier0/lib/commands/test_constraint_order.py

@@ -0,0 +1,81 @@
+from unittest import TestCase
+
+from pcs.common import reports
+from pcs.lib.commands import constraint
+
+from pcs_test.tools import fixture
+from pcs_test.tools.command_env import get_env_tools
+
+
+class SetCreate(TestCase):
+    def setUp(self):
+        resources_xml = """
+            <resources>
+                <group id="grAB">
+                    <primitive class="ocf" id="A" provider="pacemaker" type="Dummy"/>
+                    <primitive class="ocf" id="B" provider="pacemaker" type="Dummy"/>
+                </group>
+                <group id="grC">
+                    <primitive class="ocf" id="C" provider="pacemaker" type="Dummy"/>
+                </group>
+                <primitive class="ocf" id="D" provider="pacemaker" type="Dummy"/>
+            </resources>
+        """
+        self.env_assist, self.config = get_env_tools(self)
+        self.config.runner.cib.load(resources=resources_xml)
+
+    def test_deny_resources_from_one_group_in_one_set(self):
+        self.env_assist.assert_raise_library_error(
+            lambda: constraint.order.create_with_set(
+                self.env_assist.get_env(),
+                [{"ids": ["A", "B"], "options": {}}],
+                {},
+            ),
+            [
+                fixture.error(
+                    reports.codes.CANNOT_SET_ORDER_CONSTRAINTS_FOR_RESOURCES_IN_THE_SAME_GROUP,
+                )
+            ],
+            expected_in_processor=False,
+        )
+
+    def test_deny_resources_from_one_group_in_different_sets(self):
+        self.env_assist.assert_raise_library_error(
+            lambda: constraint.order.create_with_set(
+                self.env_assist.get_env(),
+                [{"ids": ["A"], "options": {}}, {"ids": ["B"], "options": {}}],
+                {},
+            ),
+            [
+                fixture.error(
+                    reports.codes.CANNOT_SET_ORDER_CONSTRAINTS_FOR_RESOURCES_IN_THE_SAME_GROUP,
+                )
+            ],
+            expected_in_processor=False,
+        )
+
+    def test_allow_resources_from_different_groups(self):
+        constraints_xml = """
+            <constraints>
+                <rsc_order id="order_set_AACCAA">
+                    <resource_set id="order_set_AACCAA_set">
+                        <resource_ref id="A"/>
+                        <resource_ref id="C"/>
+                    </resource_set>
+                    <resource_set id="order_set_AACCAA_set-1">
+                        <resource_ref id="A"/>
+                        <resource_ref id="D"/>
+                    </resource_set>
+                </rsc_order>
+            </constraints>
+        """
+        self.config.env.push_cib(constraints=constraints_xml)
+
+        constraint.order.create_with_set(
+            self.env_assist.get_env(),
+            [
+                {"ids": ["A", "C"], "options": {}},
+                {"ids": ["A", "D"], "options": {}},
+            ],
+            {},
+        )

pcs_test/tier1/legacy/test_constraints.py

@@ -5377,3 +5377,49 @@ def test_complex_primitive_all(self):
                 """
             ),
         )
+
+
+class OrderVsGroup(unittest.TestCase, AssertPcsMixin):
+    empty_cib = rc("cib-empty.xml")
+
+    def setUp(self):
+        self.temp_cib = get_tmp_file("tier1_constraint_order_vs_group")
+        write_file_to_tmpfile(self.empty_cib, self.temp_cib)
+        self.pcs_runner = PcsRunner(self.temp_cib.name)
+        self.assert_pcs_success(
+            "resource create A ocf:heartbeat:Dummy --group grAB".split()
+        )
+        self.assert_pcs_success(
+            "resource create B ocf:heartbeat:Dummy --group grAB".split()
+        )
+        self.assert_pcs_success(
+            "resource create C ocf:heartbeat:Dummy --group grC".split()
+        )
+        self.assert_pcs_success("resource create D ocf:heartbeat:Dummy".split())
+
+    def tearDown(self):
+        self.temp_cib.close()
+
+    def test_deny_resources_in_one_group(self):
+        self.assert_pcs_fail(
+            "constraint order A then B".split(),
+            "Error: Cannot create an order constraint for resources in the same group\n",
+        )
+
+    def test_allow_resources_in_different_groups(self):
+        self.assert_pcs_success(
+            "constraint order A then C".split(),
+            "Adding A C (kind: Mandatory) (Options: first-action=start then-action=start)\n",
+        )
+
+    def test_allow_grouped_and_not_grouped_resource(self):
+        self.assert_pcs_success(
+            "constraint order A then D".split(),
+            "Adding A D (kind: Mandatory) (Options: first-action=start then-action=start)\n",
+        )
+
+    def test_allow_group_and_resource(self):
+        self.assert_pcs_success(
+            "constraint order grAB then C".split(),
+            "Adding grAB C (kind: Mandatory) (Options: first-action=start then-action=start)\n",
+        )