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