From be8876832da345e7e16827bd6c50e262380d6979 Mon Sep 17 00:00:00 2001

From: Ivan Devat <idevat@redhat.com>

Date: Wed, 14 Sep 2016 09:04:57 +0200

Subject: [PATCH] squash bz1305049 pcs does not support "ticket" con

d147ba4a51d0 do not use suffix "no-role" in ticket constraints

066cf217ec45 add constraint ticket remove command

---

 pcs/cli/common/lib_wrapper.py                  |  1 +

 pcs/cli/constraint_ticket/command.py           |  6 ++

 pcs/cli/constraint_ticket/test/test_command.py | 22 +++++++

 pcs/constraint.py                              |  1 +

 pcs/lib/cib/constraint/ticket.py               | 29 ++++++++-

 pcs/lib/cib/test/test_constraint_ticket.py     | 89 +++++++++++++++++++++++++-

 pcs/lib/commands/constraint/ticket.py          | 12 ++++

 pcs/pcs.8                                      |  3 +

 pcs/test/test_constraints.py                   | 36 +++++++++++

 pcs/usage.py                                   |  3 +

 10 files changed, 199 insertions(+), 3 deletions(-)

diff --git a/pcs/cli/common/lib_wrapper.py b/pcs/cli/common/lib_wrapper.py

index 94a1311..99bfe35 100644

--- a/pcs/cli/common/lib_wrapper.py

+++ b/pcs/cli/common/lib_wrapper.py

@@ -132,6 +132,7 @@ def load_module(env, middleware_factory, name):

                 'set': constraint_ticket.create_with_set,

                 'show': constraint_ticket.show,

                 'add': constraint_ticket.create,

+                'remove': constraint_ticket.remove,

             }

         )

diff --git a/pcs/cli/constraint_ticket/command.py b/pcs/cli/constraint_ticket/command.py

index ab70434..0ed4fdd 100644

--- a/pcs/cli/constraint_ticket/command.py

+++ b/pcs/cli/constraint_ticket/command.py

@@ -52,6 +52,12 @@ def add(lib, argv, modificators):

         duplication_alowed=modificators["force"],

     )

+def remove(lib, argv, modificators):

+    if len(argv) != 2:

+        raise CmdLineInputError()

+    ticket, resource_id = argv

+    lib.constraint_ticket.remove(ticket, resource_id)

+

 def show(lib, argv, modificators):

     """

     show all ticket constraints

diff --git a/pcs/cli/constraint_ticket/test/test_command.py b/pcs/cli/constraint_ticket/test/test_command.py

index d40d421..9ca7817 100644

--- a/pcs/cli/constraint_ticket/test/test_command.py

+++ b/pcs/cli/constraint_ticket/test/test_command.py

@@ -65,3 +65,25 @@ class AddTest(TestCase):

             resource_in_clone_alowed=True,

             duplication_alowed=True,

         )

+

+class RemoveTest(TestCase):

+    def test_refuse_args_count(self):

+        self.assertRaises(CmdLineInputError, lambda: command.remove(

+            mock.MagicMock(),

+            ["TICKET"],

+            {},

+        ))

+        self.assertRaises(CmdLineInputError, lambda: command.remove(

+            mock.MagicMock(),

+            ["TICKET", "RESOURCE", "SOMETHING_ELSE"],

+            {},

+        ))

+

+    def test_call_library_remove_with_correct_attrs(self):

+        lib = mock.MagicMock(

+            constraint_ticket=mock.MagicMock(remove=mock.Mock())

+        )

+        command.remove(lib, ["TICKET", "RESOURCE"], {})

+        lib.constraint_ticket.remove.assert_called_once_with(

+            "TICKET", "RESOURCE",

+        )

diff --git a/pcs/constraint.py b/pcs/constraint.py

index e32f1a3..d8415b6 100644

--- a/pcs/constraint.py

+++ b/pcs/constraint.py

@@ -90,6 +90,7 @@ def constraint_cmd(argv):

             command_map = {

                 "set": ticket_command.create_with_set,

                 "add": ticket_command.add,

+                "remove": ticket_command.remove,

                 "show": ticket_command.show,

             }

             sub_command = argv[0] if argv else "show"

diff --git a/pcs/lib/cib/constraint/ticket.py b/pcs/lib/cib/constraint/ticket.py

index 4154aac..c708794 100644

--- a/pcs/lib/cib/constraint/ticket.py

+++ b/pcs/lib/cib/constraint/ticket.py

@@ -39,7 +39,8 @@ def _validate_options_common(options):

 def _create_id(cib, ticket, resource_id, resource_role):

     return tools.find_unique_id(

         cib,

-        "-".join(('ticket', ticket, resource_id, resource_role))

+        "-".join(('ticket', ticket, resource_id))

+        +("-{0}".format(resource_role) if resource_role else "")

     )

 def prepare_options_with_set(cib, options, resource_set_list):

@@ -93,7 +94,7 @@ def prepare_options_plain(cib, options, ticket, resource_id):

             cib,

             options["ticket"],

             resource_id,

-            options["rsc-role"] if "rsc-role" in options else "no-role"

+            options.get("rsc-role", "")

         ),

         partial(tools.check_new_id_applicable, cib, DESCRIPTION)

     )

@@ -103,6 +104,30 @@ def create_plain(constraint_section, options):

     element.attrib.update(options)

     return element

+def remove_plain(constraint_section, ticket_key, resource_id):

+    ticket_element_list = constraint_section.xpath(

+        './/rsc_ticket[@ticket="{0}" and @rsc="{1}"]'

+        .format(ticket_key, resource_id)

+    )

+

+    for ticket_element in ticket_element_list:

+        ticket_element.getparent().remove(ticket_element)

+

+def remove_with_resource_set(constraint_section, ticket_key, resource_id):

+    ref_element_list = constraint_section.xpath(

+        './/rsc_ticket[@ticket="{0}"]/resource_set/resource_ref[@id="{1}"]'

+        .format(ticket_key, resource_id)

+    )

+

+    for ref_element in ref_element_list:

+        set_element = ref_element.getparent()

+        set_element.remove(ref_element)

+        if not len(set_element):

+            ticket_element = set_element.getparent()

+            ticket_element.remove(set_element)

+            if not len(ticket_element):

+                ticket_element.getparent().remove(ticket_element)

+

 def are_duplicate_plain(element, other_element):

     return all(

         element.attrib.get(name, "") == other_element.attrib.get(name, "")

diff --git a/pcs/lib/cib/test/test_constraint_ticket.py b/pcs/lib/cib/test/test_constraint_ticket.py

index ede748e..d3da004 100644

--- a/pcs/lib/cib/test/test_constraint_ticket.py

+++ b/pcs/lib/cib/test/test_constraint_ticket.py

@@ -8,10 +8,15 @@ from __future__ import (

 from functools import partial

 from pcs.test.tools.pcs_unittest import TestCase

+from lxml import etree

+

 from pcs.common import report_codes

 from pcs.lib.cib.constraint import ticket

 from pcs.lib.errors import ReportItemSeverity as severities

-from pcs.test.tools.assertions import assert_raise_library_error

+from pcs.test.tools.assertions import (

+    assert_raise_library_error,

+    assert_xml_equal,

+)

 from pcs.test.tools.pcs_unittest import mock

@@ -306,3 +311,85 @@ class AreDuplicateWithResourceSet(TestCase):

             Element({"ticket": "ticket_key"}),

             Element({"ticket": "X"}),

         ))

+

+class RemovePlainTest(TestCase):

+    def test_remove_tickets_constraints_for_resource(self):

+        constraint_section = etree.fromstring("""

+            <constraints>

+                <rsc_ticket id="t1" ticket="tA" rsc="rA"/>

+                <rsc_ticket id="t2" ticket="tA" rsc="rB"/>

+                <rsc_ticket id="t3" ticket="tA" rsc="rA"/>

+                <rsc_ticket id="t4" ticket="tB" rsc="rA"/>

+                <rsc_ticket id="t5" ticket="tB" rsc="rB"/>

+            </constraints>

+        """)

+

+        ticket.remove_plain(

+            constraint_section,

+            ticket_key="tA",

+            resource_id="rA",

+        )

+

+        assert_xml_equal(etree.tostring(constraint_section).decode(), """

+            <constraints>

+                <rsc_ticket id="t2" ticket="tA" rsc="rB"/>

+                <rsc_ticket id="t4" ticket="tB" rsc="rA"/>

+                <rsc_ticket id="t5" ticket="tB" rsc="rB"/>

+            </constraints>

+        """)

+

+class RemoveWithSetTest(TestCase):

+    def test_remove_resource_references_and_empty_remaining_parents(self):

+        constraint_section = etree.fromstring("""

+            <constraints>

+                <rsc_ticket id="t1" ticket="tA">

+                    <resource_set id="rs1">

+                        <resource_ref id="rA"/>

+                    </resource_set>

+                    <resource_set id="rs2">

+                        <resource_ref id="rA"/>

+                    </resource_set>

+                </rsc_ticket>

+

+                <rsc_ticket id="t2" ticket="tA">

+                    <resource_set id="rs3">

+                        <resource_ref id="rA"/>

+                        <resource_ref id="rB"/>

+                    </resource_set>

+                    <resource_set id="rs4">

+                        <resource_ref id="rA"/>

+                    </resource_set>

+                </rsc_ticket>

+

+                <rsc_ticket id="t3" ticket="tB">

+                    <resource_set id="rs5">

+                        <resource_ref id="rA"/>

+                    </resource_set>

+                </rsc_ticket>

+            </constraints>

+        """)

+

+        ticket.remove_with_resource_set(

+            constraint_section,

+            ticket_key="tA",

+            resource_id="rA"

+        )

+

+        assert_xml_equal(

+            """

+                <constraints>

+                    <rsc_ticket id="t2" ticket="tA">

+                        <resource_set id="rs3">

+                            <resource_ref id="rB"/>

+                        </resource_set>

+                    </rsc_ticket>

+

+                    <rsc_ticket id="t3" ticket="tB">

+                        <resource_set id="rs5">

+                            <resource_ref id="rA"/>

+                        </resource_set>

+                    </rsc_ticket>

+                </constraints>

+            """,

+            etree.tostring(constraint_section).decode()

+        )

diff --git a/pcs/lib/commands/constraint/ticket.py b/pcs/lib/commands/constraint/ticket.py

index e6960d5..2ea7afc 100644

--- a/pcs/lib/commands/constraint/ticket.py

+++ b/pcs/lib/commands/constraint/ticket.py

@@ -68,3 +68,15 @@ def create(

     )

     env.push_cib(cib)

+

+def remove(env, ticket_key, resource_id):

+    """

+    remove all ticket constraint from resource

+    If resource is in resource set with another resources then only resource ref

+    is removed. If resource is alone in resource set whole constraint is removed.

+    """

+    cib = env.get_cib()

+    constraint_section = get_constraints(cib)

+    ticket.remove_plain(constraint_section, ticket_key, resource_id)

+    ticket.remove_with_resource_set(constraint_section, ticket_key, resource_id)

+    env.push_cib(cib)

diff --git a/pcs/pcs.8 b/pcs/pcs.8

index 61abe67..40b146f 100644

--- a/pcs/pcs.8

+++ b/pcs/pcs.8

@@ -490,6 +490,9 @@ Create a ticket constraint for <resource id>. Available option is loss-policy=fe

 ticket set <resource1> [resourceN]... [options] [set <resourceX> ... [options]] [setoptions [constraint_options]]

 Create a ticket constraint with a resource set. Available options are sequential=true/false, require-all=true/false, action=start/promote/demote/stop and role=Stopped/Started/Master/Slave. Required constraint option is ticket=<ticket>. Optional constraint options are id=<constraint-id> and loss-policy=fence/stop/freeze/demote.

 .TP

+ticket remove <ticket> <resource id>

+Remove all ticket constraints with <ticket> from <resource id>.

+.TP

 remove [constraint id]...

 Remove constraint(s) or constraint rules with the specified id(s).

 .TP

diff --git a/pcs/test/test_constraints.py b/pcs/test/test_constraints.py

index 7c76e09..4007e90 100644

--- a/pcs/test/test_constraints.py

+++ b/pcs/test/test_constraints.py

@@ -2686,6 +2686,42 @@ class TicketAdd(ConstraintBaseTest):

             "  Master A loss-policy=fence ticket=T",

         ])

+class TicketRemoveTest(ConstraintBaseTest):

+    def test_remove_multiple_tickets(self):

+        #fixture

+        self.assert_pcs_success('constraint ticket add T A')

+        self.assert_pcs_success(

+            'constraint ticket add T A --force',

+            stdout_full=[

+                "Warning: duplicate constraint already exists",

+                "  A ticket=T (id:ticket-T-A)"

+            ]

+        )

+        self.assert_pcs_success(

+            'constraint ticket set A B setoptions ticket=T'

+        )

+        self.assert_pcs_success(

+            'constraint ticket set A setoptions ticket=T'

+        )

+        self.assert_pcs_success("constraint ticket show", stdout_full=[

+            "Ticket Constraints:",

+            "  A ticket=T",

+            "  A ticket=T",

+            "  Resource Sets:",

+            "    set A B setoptions ticket=T",

+            "    set A setoptions ticket=T",

+        ])

+

+        #test

+        self.assert_pcs_success("constraint ticket remove T A")

+

+        self.assert_pcs_success("constraint ticket show", stdout_full=[

+            "Ticket Constraints:",

+            "  Resource Sets:",

+            "    set B setoptions ticket=T",

+        ])

+

+

 class TicketShow(ConstraintBaseTest):

     def test_show_set(self):

         self.assert_pcs_success('constraint ticket set A B setoptions ticket=T')

diff --git a/pcs/usage.py b/pcs/usage.py

index 9d4617f..764e3fc 100644

--- a/pcs/usage.py

+++ b/pcs/usage.py

@@ -1011,6 +1011,9 @@ Commands:

         Required constraint option is ticket=<ticket>. Optional constraint

         options are id=<constraint-id> and loss-policy=fence/stop/freeze/demote.

+    ticket remove <ticket> <resource id>

+        Remove all ticket constraints with <ticket> from <resource id>.

+

     remove [constraint id]...

         Remove constraint(s) or constraint rules with the specified id(s).

-- 

1.8.3.1