From deac91b1fc74065d01342420accfd1af88237237 Mon Sep 17 00:00:00 2001

From: Ivan Devat <idevat@redhat.com>

Date: Tue, 20 Sep 2016 08:20:29 +0200

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

07ae6704fff5 fail when no matching ticket constraint for remove

d25ed3d9bc65 fix help for constraint ticket commands

66b91ba0da7e fix manpage for booth ticket add command

2710bc2e15c2 fix manpage for constraint ticket set

---

 pcs/cli/constraint_ticket/command.py       |  4 ++-

 pcs/lib/cib/constraint/ticket.py           |  4 +++

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

 pcs/lib/commands/constraint/ticket.py      | 15 +++++++--

 pcs/lib/commands/test/test_ticket.py       | 20 +++++++++++

 pcs/pcs.8                                  |  6 ++--

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

 pcs/usage.py                               | 10 +++---

 8 files changed, 105 insertions(+), 15 deletions(-)

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

index 0ed4fdd..583ba9e 100644

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

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

@@ -8,6 +8,7 @@ from __future__ import (

 from pcs.cli.common.errors import CmdLineInputError

 from pcs.cli.constraint import command

 from pcs.cli.constraint_ticket import parse_args, console_report

+from pcs.cli.common.console_report import error

 def create_with_set(lib, argv, modificators):

     """

@@ -56,7 +57,8 @@ def remove(lib, argv, modificators):

     if len(argv) != 2:

         raise CmdLineInputError()

     ticket, resource_id = argv

-    lib.constraint_ticket.remove(ticket, resource_id)

+    if not lib.constraint_ticket.remove(ticket, resource_id):

+        raise error("no matching ticket constraint found")

 def show(lib, argv, modificators):

     """

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

index c708794..85d045c 100644

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

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

@@ -113,6 +113,8 @@ def remove_plain(constraint_section, ticket_key, resource_id):

     for ticket_element in ticket_element_list:

         ticket_element.getparent().remove(ticket_element)

+    return len(ticket_element_list) > 0

+

 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}"]'

@@ -128,6 +130,8 @@ def remove_with_resource_set(constraint_section, ticket_key, resource_id):

             if not len(ticket_element):

                 ticket_element.getparent().remove(ticket_element)

+    return len(ref_element_list) > 0

+

 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 d3da004..b720b55 100644

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

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

@@ -324,11 +324,34 @@ class RemovePlainTest(TestCase):

             </constraints>

         """)

-        ticket.remove_plain(

+        self.assertTrue(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>

+        """)

+

+    def test_remove_nothing_when_no_matching_found(self):

+        constraint_section = etree.fromstring("""

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

+        """)

+

+        self.assertFalse(ticket.remove_plain(

+            constraint_section,

+            ticket_key="tA",

+            resource_id="rA",

+        ))

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

             <constraints>

@@ -369,11 +392,11 @@ class RemoveWithSetTest(TestCase):

             </constraints>

         """)

-        ticket.remove_with_resource_set(

+        self.assertTrue(ticket.remove_with_resource_set(

             constraint_section,

             ticket_key="tA",

             resource_id="rA"

-        )

+        ))

         assert_xml_equal(

             """

@@ -393,3 +416,25 @@ class RemoveWithSetTest(TestCase):

             """,

             etree.tostring(constraint_section).decode()

         )

+

+    def test_remove_nothing_when_no_matching_found(self):

+        constraint_section = etree.fromstring("""

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

+        """)

+        self.assertFalse(ticket.remove_with_resource_set(

+            constraint_section,

+            ticket_key="tA",

+            resource_id="rA"

+        ))

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

index 2ea7afc..a14c5ad 100644

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

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

@@ -77,6 +77,17 @@ def remove(env, ticket_key, resource_id):

     """

     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)

+    any_plain_removed = ticket.remove_plain(

+        constraint_section,

+        ticket_key,

+        resource_id

+    )

+    any_with_resource_set_removed = ticket.remove_with_resource_set(

+        constraint_section,

+        ticket_key,

+        resource_id

+    )

+

     env.push_cib(cib)

+

+    return any_plain_removed or any_with_resource_set_removed

diff --git a/pcs/lib/commands/test/test_ticket.py b/pcs/lib/commands/test/test_ticket.py

index 586ca4b..edf592a 100644

--- a/pcs/lib/commands/test/test_ticket.py

+++ b/pcs/lib/commands/test/test_ticket.py

@@ -6,6 +6,8 @@ from __future__ import (

 )

 from pcs.test.tools.pcs_unittest import TestCase

+from pcs.test.tools.pcs_unittest import mock

+from pcs.test.tools.misc import create_patcher

 from pcs.common import report_codes

 from pcs.lib.commands.constraint import ticket as ticket_command

@@ -18,6 +20,7 @@ from pcs.test.tools.assertions import (

 from pcs.test.tools.misc import get_test_resource as rc

 from pcs.test.tools.xml import get_xml_manipulation_creator_from_file

+patch_commands = create_patcher("pcs.lib.commands.constraint.ticket")

 class CreateTest(TestCase):

     def setUp(self):

@@ -65,3 +68,20 @@ class CreateTest(TestCase):

                 {"resource_id": "resourceA"},

             ),

         )

+

+@patch_commands("get_constraints", mock.Mock)

+class RemoveTest(TestCase):

+    @patch_commands("ticket.remove_plain", mock.Mock(return_value=1))

+    @patch_commands("ticket.remove_with_resource_set",mock.Mock(return_value=0))

+    def test_successfully_remove_plain(self):

+        self.assertTrue(ticket_command.remove(mock.MagicMock(), "T", "R"))

+

+    @patch_commands("ticket.remove_plain", mock.Mock(return_value=0))

+    @patch_commands("ticket.remove_with_resource_set",mock.Mock(return_value=1))

+    def test_successfully_remove_with_resource_set(self):

+        self.assertTrue(ticket_command.remove(mock.MagicMock(), "T", "R"))

+

+    @patch_commands("ticket.remove_plain", mock.Mock(return_value=0))

+    @patch_commands("ticket.remove_with_resource_set",mock.Mock(return_value=0))

+    def test_raises_library_error_when_no_matching_constraint_found(self):

+        self.assertFalse(ticket_command.remove(mock.MagicMock(), "T", "R"))

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

index 40b146f..1efe8f4 100644

--- a/pcs/pcs.8

+++ b/pcs/pcs.8

@@ -484,10 +484,10 @@ Remove colocation constraints with specified resources.

 ticket [show] [\fB\-\-full\fR]

 List all current ticket constraints (if \fB\-\-full\fR is specified show the internal constraint id's as well).

 .TP

-ticket add <ticket> [<role>] <resource id> [options] [id=constraint\-id]

+ticket add <ticket> [<role>] <resource id> [<options>] [id=<constraint\-id>]

 Create a ticket constraint for <resource id>. Available option is loss-policy=fence/stop/freeze/demote. A role can be master, slave, started or stopped.

 .TP

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

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

@@ -587,7 +587,7 @@ Write new booth configuration with specified sites and arbitrators.  Total numbe

 destroy

 Remove booth configuration files.

 .TP

-ticket add <ticket>

+ticket add <ticket> [<name>=<value> ...]

 Add new ticket to the current configuration. Ticket options are specified in booth manpage.

 .TP

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

index 4007e90..fee7093 100644

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

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

@@ -2721,6 +2721,14 @@ class TicketRemoveTest(ConstraintBaseTest):

             "    set B setoptions ticket=T",

         ])

+    def test_fail_when_no_matching_ticket_constraint_here(self):

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

+            "Ticket Constraints:",

+        ])

+        self.assert_pcs_fail("constraint ticket remove T A", [

+            "Error: no matching ticket constraint found"

+        ])

+

 class TicketShow(ConstraintBaseTest):

     def test_show_set(self):

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

index 764e3fc..ea407c3 100644

--- a/pcs/usage.py

+++ b/pcs/usage.py

@@ -996,15 +996,15 @@ Commands:

         List all current ticket constraints (if --full is specified show

         the internal constraint id's as well).

-    ticket add <ticket> [<role>] <resource id> [options]

-               [id=constraint-id]

+    ticket add <ticket> [<role>] <resource id> [<options>]

+               [id=<constraint-id>]

         Create a ticket constraint for <resource id>.

         Available option is loss-policy=fence/stop/freeze/demote.

         A role can be master, slave, started or stopped.

-    ticket set <resource1> [resourceN]... [options]

-               [set <resourceX> ... [options]]

-               [setoptions [constraint_options]]

+    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.

-- 

1.8.3.1