From 8707ba13053e172d148ec12820a4259ffa371000 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 bz1308514 Wider support for booth configura

de5edd583d82 correct booth documentation (ticket grant/revoke)

d98e6b04da8d make forceable uknown booth ticket option

---

 pcs/cli/booth/command.py                    |   6 +-

 pcs/cli/booth/test/test_command.py          |   3 +-

 pcs/lib/booth/config_structure.py           |  31 ++++++--

 pcs/lib/booth/test/test_config_structure.py | 107 ++++++++++++++++++++++++----

 pcs/lib/commands/booth.py                   |   7 +-

 pcs/pcs.8                                   |   7 +-

 pcs/test/test_booth.py                      |  17 +++++

 pcs/usage.py                                |  11 +--

 8 files changed, 161 insertions(+), 28 deletions(-)

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

index 0b71a01..72b2c73 100644

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

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

@@ -64,7 +64,11 @@ def config_ticket_add(lib, arg_list, modifiers):

     """

     if not arg_list:

         raise CmdLineInputError

-    lib.booth.config_ticket_add(arg_list[0], prepare_options(arg_list[1:]))

+    lib.booth.config_ticket_add(

+        arg_list[0],

+        prepare_options(arg_list[1:]),

+        allow_unknown_options=modifiers["force"]

+    )

 def config_ticket_remove(lib, arg_list, modifiers):

     """

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

index 44d7a12..8ba2c0e 100644

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

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

@@ -45,9 +45,10 @@ class ConfigTicketAddTest(TestCase):

         command.config_ticket_add(

             lib,

             arg_list=["TICKET_A", "timeout=10"],

-            modifiers={}

+            modifiers={"force": True}

         )

         lib.booth.config_ticket_add.assert_called_once_with(

             "TICKET_A",

             {"timeout": "10"},

+            allow_unknown_options=True

         )

diff --git a/pcs/lib/booth/config_structure.py b/pcs/lib/booth/config_structure.py

index 8977b7a..09ff1a7 100644

--- a/pcs/lib/booth/config_structure.py

+++ b/pcs/lib/booth/config_structure.py

@@ -9,7 +9,8 @@ import re

 import pcs.lib.reports as common_reports

 from pcs.lib.booth import reports

-from pcs.lib.errors import LibraryError

+from pcs.lib.errors import LibraryError, ReportItemSeverity as severities

+from pcs.common import report_codes

 from collections import namedtuple

 GLOBAL_KEYS = (

@@ -83,10 +84,13 @@ def remove_ticket(booth_configuration, ticket_name):

         if config_item.key != "ticket" or config_item.value != ticket_name

     ]

-def add_ticket(booth_configuration, ticket_name, options):

+def add_ticket(

+    report_processor, booth_configuration, ticket_name, options,

+    allow_unknown_options

+):

     validate_ticket_name(ticket_name)

     validate_ticket_unique(booth_configuration, ticket_name)

-    validate_ticket_options(options)

+    validate_ticket_options(report_processor, options, allow_unknown_options)

     return booth_configuration + [

         ConfigItem("ticket", ticket_name, [

             ConfigItem(key, value) for key, value in options.items()

@@ -101,7 +105,7 @@ def validate_ticket_unique(booth_configuration, ticket_name):

     if ticket_exists(booth_configuration, ticket_name):

         raise LibraryError(reports.booth_ticket_duplicate(ticket_name))

-def validate_ticket_options(options):

+def validate_ticket_options(report_processor, options, allow_unknown_options):

     reports = []

     for key in sorted(options):

         if key in GLOBAL_KEYS:

@@ -109,6 +113,22 @@ def validate_ticket_options(options):

                 common_reports.invalid_option(key, TICKET_KEYS, "booth ticket")

             )

+        elif key not in TICKET_KEYS:

+            reports.append(

+                common_reports.invalid_option(

+                    key, TICKET_KEYS,

+                    "booth ticket",

+                    severity=(

+                        severities.WARNING if allow_unknown_options

+                        else severities.ERROR

+                    ),

+                    forceable=(

+                        None if allow_unknown_options

+                        else report_codes.FORCE_OPTIONS

+                    ),

+                )

+            )

+

         if not options[key].strip():

             reports.append(common_reports.invalid_option_value(

                 key,

@@ -116,8 +136,7 @@ def validate_ticket_options(options):

                 "no-empty",

             ))

-    if reports:

-        raise LibraryError(*reports)

+    report_processor.process_list(reports)

 def ticket_exists(booth_configuration, ticket_name):

     return any(

diff --git a/pcs/lib/booth/test/test_config_structure.py b/pcs/lib/booth/test/test_config_structure.py

index 5e7ac68..40618b2 100644

--- a/pcs/lib/booth/test/test_config_structure.py

+++ b/pcs/lib/booth/test/test_config_structure.py

@@ -10,7 +10,11 @@ from pcs.test.tools.pcs_unittest import TestCase

 from pcs.common import report_codes

 from pcs.lib.booth import config_structure

 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_report_item_list_equal,

+)

+from pcs.test.tools.custom_mock import MockLibraryReportProcessor

 from pcs.test.tools.pcs_unittest import mock

@@ -49,12 +53,8 @@ class ValidateTicketUniqueTest(TestCase):

 class ValidateTicketOptionsTest(TestCase):

     def test_raises_on_invalid_options(self):

-        assert_raise_library_error(

-            lambda: config_structure.validate_ticket_options({

-                "site": "a",

-                "port": "b",

-                "timeout": " ",

-            }),

+        report_processor = MockLibraryReportProcessor()

+        expected_errors = [

             (

                 severities.ERROR,

                 report_codes.INVALID_OPTION,

@@ -82,10 +82,81 @@ class ValidateTicketOptionsTest(TestCase):

                     "allowed_values": "no-empty",

                 },

             ),

+            (

+                severities.ERROR,

+                report_codes.INVALID_OPTION,

+                {

+                    "option_name": "unknown",

+                    "option_type": "booth ticket",

+                    "allowed": list(config_structure.TICKET_KEYS),

+                },

+                report_codes.FORCE_OPTIONS

+            ),

+        ]

+        assert_raise_library_error(

+            lambda: config_structure.validate_ticket_options(

+                report_processor,

+                {

+                    "site": "a",

+                    "port": "b",

+                    "timeout": " ",

+                    "unknown": "c",

+                },

+                allow_unknown_options=False,

+            ),

+            *expected_errors

+        )

+        assert_report_item_list_equal(

+            report_processor.report_item_list,

+            expected_errors

+        )

+

+    def test_unknown_options_are_forceable(self):

+        report_processor = MockLibraryReportProcessor()

+        expected_errors = [

+            (

+                severities.ERROR,

+                report_codes.INVALID_OPTION,

+                {

+                    "option_name": "site",

+                    "option_type": "booth ticket",

+                    "allowed": list(config_structure.TICKET_KEYS),

+                },

+            ),

+        ]

+        assert_raise_library_error(

+            lambda: config_structure.validate_ticket_options(

+                report_processor, {

+                    "site": "a",

+                    "unknown": "c",

+                },

+                allow_unknown_options=True,

+            ),

+            *expected_errors

+        )

+        assert_report_item_list_equal(

+            report_processor.report_item_list,

+            expected_errors + [

+                (

+                    severities.WARNING,

+                    report_codes.INVALID_OPTION,

+                    {

+                        "option_name": "unknown",

+                        "option_type": "booth ticket",

+                        "allowed": list(config_structure.TICKET_KEYS),

+                    },

+                ),

+            ]

         )

     def test_success_on_valid_options(self):

-        config_structure.validate_ticket_options({"timeout": "10"})

+        report_processor = MockLibraryReportProcessor()

+        config_structure.validate_ticket_options(

+            report_processor,

+            {"timeout": "10"},

+            allow_unknown_options=False,

+        )

+        assert_report_item_list_equal(report_processor.report_item_list, [])

 class TicketExistsTest(TestCase):

     def test_returns_true_if_ticket_in_structure(self):

@@ -214,18 +285,25 @@ class RemoveTicketTest(TestCase):

         )

 class AddTicketTest(TestCase):

+    @mock.patch("pcs.lib.booth.config_structure.validate_ticket_options")

     @mock.patch("pcs.lib.booth.config_structure.validate_ticket_unique")

     @mock.patch("pcs.lib.booth.config_structure.validate_ticket_name")

     def test_successfully_add_ticket(

-        self, mock_validate_name, mock_validate_uniq

+        self, mock_validate_name, mock_validate_uniq, mock_validate_options

     ):

         configuration = [

             config_structure.ConfigItem("ticket", "some-ticket"),

         ]

+

         self.assertEqual(

-            config_structure.add_ticket(configuration, "new-ticket", {

-                "timeout": "10",

-            }),

+            config_structure.add_ticket(

+                None, configuration,

+                "new-ticket",

+                {

+                    "timeout": "10",

+                },

+                allow_unknown_options=False,

+            ),

             [

                 config_structure.ConfigItem("ticket", "some-ticket"),

                 config_structure.ConfigItem("ticket", "new-ticket", [

@@ -236,6 +314,11 @@ class AddTicketTest(TestCase):

         mock_validate_name.assert_called_once_with("new-ticket")

         mock_validate_uniq.assert_called_once_with(configuration, "new-ticket")

+        mock_validate_options.assert_called_once_with(

+            None,

+            {"timeout": "10"},

+            False

+        )

 class SetAuthfileTest(TestCase):

     def test_add_authfile(self):

diff --git a/pcs/lib/commands/booth.py b/pcs/lib/commands/booth.py

index bea966c..705900a 100644

--- a/pcs/lib/commands/booth.py

+++ b/pcs/lib/commands/booth.py

@@ -119,14 +119,19 @@ def config_text(env, name, node_name=None):

         raise LibraryError(reports.invalid_response_format(node_name))

-def config_ticket_add(env, ticket_name, options):

+def config_ticket_add(env, ticket_name, options, allow_unknown_options):

     """

     add ticket to booth configuration

+    dict options contains options for ticket

+    bool allow_unknown_options decide if can be used options not listed in

+        ticket options nor global options

     """

     booth_configuration = config_structure.add_ticket(

+        env.report_processor,

         parse(env.booth.get_config_content()),

         ticket_name,

         options,

+        allow_unknown_options,

     )

     env.booth.push_config(build(booth_configuration))

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

index 270ad2d..61abe67 100644

--- a/pcs/pcs.8

+++ b/pcs/pcs.8

@@ -585,7 +585,8 @@ destroy

 Remove booth configuration files.

 .TP

 ticket add <ticket>

-Add new ticket to the current configuration.

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

+

 .TP

 ticket remove <ticket>

 Remove the specified ticket from the current configuration.

@@ -603,10 +604,10 @@ restart

 Restart booth resources created by the "pcs booth create" command.

 .TP

 ticket grant <ticket> [<site address>]

-Grant the ticket for the site specified by address.  Site address which has been specified with 'pcs booth create' command is used if 'site address' is omitted. Cannot be run on an arbitrator.

+Grant the ticket for the site specified by address.  Site address which has been specified with 'pcs booth create' command is used if 'site address' is omitted.  Specifying site address is mandatory when running this command on an arbitrator.

 .TP

 ticket revoke <ticket> [<site address>]

-Revoke the ticket for the site specified by address.  Site address which has been specified with 'pcs booth create' command is used if 'site address' is omitted. Cannot be run on an arbitrator.

+Revoke the ticket for the site specified by address.  Site address which has been specified with 'pcs booth create' command is used if 'site address' is omitted.  Specifying site address is mandatory when running this command on an arbitrator.

 .TP

 status

 Print current status of booth on the local node.

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

index 3356e71..c12391b 100644

--- a/pcs/test/test_booth.py

+++ b/pcs/test/test_booth.py

@@ -223,6 +223,23 @@ class AddTicketTest(BoothTest):

             )

         )

+    def test_forceable_fail_on_unknown_options(self):

+        msg = (

+            "invalid booth ticket option 'unknown', allowed options"

+            " are: acquire-after, attr-prereq, before-acquire-handler,"

+            " expire, renewal-freq, retries, timeout, weights"

+        )

+        self.assert_pcs_fail(

+            "booth ticket add TicketA unknown=a", console_report(

+                "Error: "+msg+", use --force to override",

+            )

+        )

+        self.assert_pcs_success(

+            "booth ticket add TicketA unknown=a --force",

+            "Warning: {0}\n".format(msg),

+        )

+

+

 class RemoveTicketTest(BoothTest):

     def test_success_remove_ticket(self):

         self.assert_pcs_success("booth ticket add TicketA")

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

index 088dec9..9d4617f 100644

--- a/pcs/usage.py

+++ b/pcs/usage.py

@@ -1433,8 +1433,9 @@ Commands:

     destroy

         Remove booth configuration files.

-    ticket add <ticket>

-        Add new ticket to the current configuration.

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

+        Add new ticket to the current configuration. Ticket options are

+        specified in booth manpage.

     ticket remove <ticket>

         Remove the specified ticket from the current configuration.

@@ -1456,12 +1457,14 @@ Commands:

     ticket grant <ticket> [<site address>]

         Grant the ticket for the site specified by address.  Site address which

         has been specified with 'pcs booth create' command is used if

-        'site address' is omitted. Cannot be run on an arbitrator.

+        'site address' is omitted.  Specifying site address is mandatory when

+        running this command on an arbitrator.

     ticket revoke <ticket> [<site address>]

         Revoke the ticket for the site specified by address.  Site address which

         has been specified with 'pcs booth create' command is used if

-        'site address' is omitted. Cannot be run on an arbitrator.

+        'site address' is omitted.  Specifying site address is mandatory when

+        running this command on an arbitrator.

     status

         Print current status of booth on the local node.

-- 

1.8.3.1