From c85f907253f6379ad726a7f54635ccbe0b64af4c Mon Sep 17 00:00:00 2001 From: Ivan Devat Date: Mon, 4 Nov 2019 14:52:03 +0100 Subject: [PATCH 1/4] squash bz1500012 provide a hint about the set opti d2754533 doc: fix `pcs constraint colocation add` command abb8f0ea don't allow empty value for constraint options --- pcs/cli/common/parse_args.py | 9 +++++++-- pcs/constraint.py | 7 +++++-- pcs/pcs.8 | 4 ++-- pcs/test/test_constraints.py | 9 +++++++++ pcs/usage.py | 6 +++--- 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pcs/cli/common/parse_args.py b/pcs/cli/common/parse_args.py index 6a3210ac..23d8799d 100644 --- a/pcs/cli/common/parse_args.py +++ b/pcs/cli/common/parse_args.py @@ -46,7 +46,7 @@ def split_list(arg_list, separator): bounds = zip([0]+[i+1 for i in separator_indexes], separator_indexes+[None]) return [arg_list[i:j] for i, j in bounds] -def split_option(arg): +def split_option(arg, allow_empty_value=True): """ Get (key, value) from a key=value commandline argument. @@ -54,12 +54,17 @@ def split_option(arg): CmdLineInputError if the argument cannot be splitted. string arg -- commandline argument + allow_empty_value -- if True, empty value is allowed. Otherwise, + CmdLineInputError exception is raised """ if "=" not in arg: raise CmdLineInputError("missing value of '{0}' option".format(arg)) if arg.startswith("="): raise CmdLineInputError("missing key in '{0}' option".format(arg)) - return arg.split("=", 1) + key, value = arg.split("=", 1) + if not (value or allow_empty_value): + raise CmdLineInputError("value of '{0}' option is empty".format(key)) + return key, value def prepare_options(cmdline_args): """return dictionary of options from commandline key=value args""" diff --git a/pcs/constraint.py b/pcs/constraint.py index c9f6fd5f..ca12aaa7 100644 --- a/pcs/constraint.py +++ b/pcs/constraint.py @@ -208,7 +208,10 @@ def colocation_add(argv): return SCORE_INFINITY, [] score = SCORE_INFINITY if "=" in argv[0] else argv.pop(0) # create a list of 2-tuples (name, value) - arg_array = [parse_args.split_option(arg) for arg in argv] + arg_array = [ + parse_args.split_option(arg, allow_empty_value=False) + for arg in argv + ] return score, arg_array def _validate_and_prepare_role(role): @@ -444,7 +447,7 @@ def _order_add(resource1, resource2, options_list): elif arg == "nonsymmetrical": sym = "false" else: - name, value = parse_args.split_option(arg) + name, value = parse_args.split_option(arg, allow_empty_value=False) if name == "id": id_valid, id_error = utils.validate_xml_id( value, "constraint id" diff --git a/pcs/pcs.8 b/pcs/pcs.8 index 5670f458..f08b5e46 100644 --- a/pcs/pcs.8 +++ b/pcs/pcs.8 @@ -562,8 +562,8 @@ Remove resource from any ordering constraint colocation [show] [\fB\-\-full\fR] List all current colocation constraints (if \fB\-\-full\fR is specified show the internal constraint id's as well). .TP -colocation add [master|slave] with [master|slave] [score] [options] [id=constraint\-id] -Request to run on the same node where pacemaker has determined should run. Positive values of score mean the resources should be run on the same node, negative values mean the resources should not be run on the same node. Specifying 'INFINITY' (or '\-INFINITY') for the score forces to run (or not run) with (score defaults to "INFINITY"). A role can be master or slave (if no role is specified, it defaults to 'started'). +colocation add [] with [] [score] [options] [id=constraint\-id] +Request to run on the same node where pacemaker has determined should run. Positive values of score mean the resources should be run on the same node, negative values mean the resources should not be run on the same node. Specifying 'INFINITY' (or '\-INFINITY') for the score forces to run (or not run) with (score defaults to "INFINITY"). A role can be: 'Master', 'Slave', 'Started', 'Stopped' (if no role is specified, it defaults to 'Started'). .TP colocation set [resourceN]... [options] [set ... [options]] [setoptions [constraint_options]] Create a colocation constraint with a resource set. Available options are sequential=true/false and role=Stopped/Started/Master/Slave. Available constraint_options are id and either of: score, score\-attribute, score\-attribute\-mangle. diff --git a/pcs/test/test_constraints.py b/pcs/test/test_constraints.py index 36bcb652..3feaa053 100644 --- a/pcs/test/test_constraints.py +++ b/pcs/test/test_constraints.py @@ -262,6 +262,11 @@ Ticket Constraints: ac(o,"Location Constraints:\nOrdering Constraints:\n stop D1 then stop D2 (kind:Mandatory) (id:order-D1-D2-mandatory)\n start D1 then start D2 (kind:Mandatory) (id:order-D1-D2-mandatory-1)\nColocation Constraints:\nTicket Constraints:\n") assert r == 0 + def test_order_options_empty_value(self): + o, r = pcs("constraint order D1 then D2 option1=") + self.assertIn("value of 'option1' option is empty", o) + self.assertEqual(r, 1) + def test_order_too_many_resources(self): msg = ( "Error: Multiple 'then's cannot be specified.\n" @@ -525,6 +530,10 @@ Ticket Constraints: self.assertIn(msg, o) self.assertEqual(r, 1) + def test_colocation_options_empty_value(self): + o, r = pcs("constraint colocation add D1 with D2 option1=") + self.assertIn("value of 'option1' option is empty", o) + self.assertEqual(r, 1) def testColocationSets(self): # see also BundleColocation diff --git a/pcs/usage.py b/pcs/usage.py index 18baaf0e..37c92d26 100644 --- a/pcs/usage.py +++ b/pcs/usage.py @@ -1200,7 +1200,7 @@ Commands: List all current colocation constraints (if --full is specified show the internal constraint id's as well). - colocation add [master|slave] with [master|slave] + colocation add [] with [] [score] [options] [id=constraint-id] Request to run on the same node where pacemaker has determined should run. Positive values of score @@ -1208,8 +1208,8 @@ Commands: mean the resources should not be run on the same node. Specifying 'INFINITY' (or '-INFINITY') for the score forces to run (or not run) with (score defaults to "INFINITY"). - A role can be master or slave (if no role is specified, it defaults to - 'started'). + A role can be: 'Master', 'Slave', 'Started', 'Stopped' (if no role is + specified, it defaults to 'Started'). colocation set [resourceN]... [options] [set ... [options]] -- 2.21.0