From c85f907253f6379ad726a7f54635ccbe0b64af4c Mon Sep 17 00:00:00 2001

From: Ivan Devat <idevat@redhat.com>

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] <source resource id> with [master|slave] <target resource id> [score] [options] [id=constraint\-id]

-Request <source resource> to run on the same node where pacemaker has determined <target resource> 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 <source resource> to run (or not run) with <target resource> (score defaults to "INFINITY").  A role can be master or slave (if no role is specified, it defaults to 'started').

+colocation add [<role>] <source resource id> with [<role>] <target resource id> [score] [options] [id=constraint\-id]

+Request <source resource> to run on the same node where pacemaker has determined <target resource> 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 <source resource> to run (or not run) with <target resource> (score defaults to "INFINITY"). A role can be: 'Master', 'Slave', 'Started', 'Stopped' (if no role is specified, it defaults to 'Started').

 .TP

 colocation set <resource1> [resourceN]... [options] [set <resourceX> ... [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] <source resource id> with [master|slave]

+    colocation add [<role>] <source resource id> with [<role>]

                    <target resource id> [score] [options] [id=constraint-id]

         Request <source resource> to run on the same node where pacemaker has

         determined <target resource> 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 <source resource> to

         run (or not run) with <target resource> (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 <resource1> [resourceN]... [options]

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

-- 

2.21.0