From 9783358accb4002f30b9f7990e9facd68e22331e Mon Sep 17 00:00:00 2001
From: Ivan Devat <idevat@redhat.com>
Date: Mon, 5 Jun 2017 17:13:41 +0200
Subject: [PATCH 2/6] give back orig. --master behav. (resource create)
---
pcs/cli/common/parse_args.py | 8 +-
pcs/cli/common/test/test_parse_args.py | 34 ++++-
pcs/resource.py | 19 +++
pcs/test/cib_resource/test_create.py | 181 ++++++++++++++++++++-----
pcs/test/test_constraints.py | 28 ++--
pcs/test/test_resource.py | 10 +-
pcs/utils.py | 7 +
7 files changed, 228 insertions(+), 59 deletions(-)
diff --git a/pcs/cli/common/parse_args.py b/pcs/cli/common/parse_args.py
index 19a658e5..7235835b 100644
--- a/pcs/cli/common/parse_args.py
+++ b/pcs/cli/common/parse_args.py
@@ -307,7 +307,13 @@ def upgrade_args(arg_list):
and
args_without_options[:2] == ["resource", "create"]
):
- upgraded_args.append("master")
+ #upgraded_args.append("master")
+
+ #We do not replace flag --master with keyword "manster" here because
+ #we want to give grace period to openstack that uses original
+ #missbehaviour.
+ #see https://bugzilla.redhat.com/show_bug.cgi?id=1458153
+ upgraded_args.append(arg)
else:
upgraded_args.append(arg)
return upgraded_args
diff --git a/pcs/cli/common/test/test_parse_args.py b/pcs/cli/common/test/test_parse_args.py
index efe38d0e..900094c9 100644
--- a/pcs/cli/common/test/test_parse_args.py
+++ b/pcs/cli/common/test/test_parse_args.py
@@ -486,9 +486,21 @@ class UpgradeArgs(TestCase):
upgrade_args(["first", "--cloneopt=1", "second"])
)
- def test_upgrade_2dash_master_in_resource_create(self):
- self.assertEqual(
- ["resource", "create", "master", "second"],
+ # def test_upgrade_2dash_master_in_resource_create(self):
+ # self.assertEqual(
+ # ["resource", "create", "master", "second"],
+ # upgrade_args(["resource", "create", "--master", "second"])
+ # )
+
+ def test_do_not_upgrade_2dash_master_in_resource_create__original_behaviour(
+ self
+ ):
+ """
+ downstream temporary behaviour
+ fixes bz 1458153
+ """
+ self.assertEqual(
+ ["resource", "create", "--master", "second"],
upgrade_args(["resource", "create", "--master", "second"])
)
@@ -498,10 +510,22 @@ class UpgradeArgs(TestCase):
upgrade_args(["first", "--master", "second"])
)
- def test_upgrade_2dash_master_in_resource_create_with_complications(self):
+ # def test_upgrade_2dash_master_in_resource_create_with_complications(self):
+ # self.assertEqual(
+ # [
+ # "-f", "path/to/file", "resource", "-V", "create", "master",
+ # "second"
+ # ],
+ # upgrade_args([
+ # "-f", "path/to/file", "resource", "-V", "create", "--master",
+ # "second"
+ # ])
+ # )
+
+ def test_no_upgrade_2dash_master_complications__original_behaviour(self):
self.assertEqual(
[
- "-f", "path/to/file", "resource", "-V", "create", "master",
+ "-f", "path/to/file", "resource", "-V", "create", "--master",
"second"
],
upgrade_args([
diff --git a/pcs/resource.py b/pcs/resource.py
index 3274910a..ac24cf4a 100644
--- a/pcs/resource.py
+++ b/pcs/resource.py
@@ -397,6 +397,25 @@ def resource_create(lib, argv, modifiers):
ra_type = argv[1]
parts = parse_create_args(argv[2:])
+
+ if modifiers["master"] and "master" in parts:
+ raise error("you cannot specify both --master and master")
+
+ #This is for `pcs resource create`. Fix of the bug
+ #https://bugzilla.redhat.com/show_bug.cgi?id=1378107
+ #caused problems in openstack which uses `pcs resource create`
+ #see https://bugzilla.redhat.com/show_bug.cgi?id=1458153
+ #so we give back the original misbehavior of master here temporarily.
+ #When user uses `--master` she gets the original behaviour. With `master`
+ #she gets new behaviour.
+ if modifiers["master"]:
+ warn(
+ "flag '--master' is deprecated, use keyword 'master' instead (see"
+ " the usage)"
+ )
+ parts["master"] = parts["meta"]
+ parts["meta"] = {}
+
parts_sections = ["clone", "master", "bundle"]
defined_options = [opt for opt in parts_sections if opt in parts]
if modifiers["group"]:
diff --git a/pcs/test/cib_resource/test_create.py b/pcs/test/cib_resource/test_create.py
index ecb16384..57d95350 100644
--- a/pcs/test/cib_resource/test_create.py
+++ b/pcs/test/cib_resource/test_create.py
@@ -238,7 +238,7 @@ class Success(ResourceTestLocal):
def test_with_master(self):
self.assert_effect(
[
- "resource create R ocf:heartbeat:Dummy --no-default-ops --master",
+ # "resource create R ocf:heartbeat:Dummy --no-default-ops --master",
"resource create R ocf:heartbeat:Dummy --no-default-ops master",
],
"""<resources>
@@ -659,7 +659,7 @@ class SuccessGroup(ResourceTestLocal):
class SuccessMaster(ResourceTestLocal):
def test_disable_is_on_master_element(self):
self.assert_effect(
- "resource create R ocf:heartbeat:Dummy --no-default-ops --disabled --master",
+ "resource create R ocf:heartbeat:Dummy --no-default-ops --disabled master",
"""<resources>
<master id="R-master">
<meta_attributes id="R-master-meta_attributes">
@@ -680,13 +680,55 @@ class SuccessMaster(ResourceTestLocal):
</resources>"""
)
- def test_put_options_after_master_as_its_meta_fix_1(self):
+ # def test_put_options_after_master_as_its_meta_fix_1(self):
+ # """
+ # fixes bz 1378107 (do not use master options as primitive options)
+ # """
+ # self.assert_effect(
+ # "resource create R ocf:heartbeat:Dummy state=a"
+ # " --master is-managed=false --force"
+ # ,
+ # """<resources>
+ # <master id="R-master">
+ # <primitive class="ocf" id="R" provider="heartbeat"
+ # type="Dummy"
+ # >
+ # <instance_attributes id="R-instance_attributes">
+ # <nvpair id="R-instance_attributes-state"
+ # name="state" value="a"
+ # />
+ # </instance_attributes>
+ # <operations>
+ # <op id="R-monitor-interval-10" interval="10"
+ # name="monitor" timeout="20"
+ # />
+ # <op id="R-start-interval-0s" interval="0s"
+ # name="start" timeout="20"
+ # />
+ # <op id="R-stop-interval-0s" interval="0s"
+ # name="stop" timeout="20"
+ # />
+ # </operations>
+ # </primitive>
+ # <meta_attributes id="R-master-meta_attributes">
+ # <nvpair id="R-master-meta_attributes-is-managed"
+ # name="is-managed" value="false"
+ # />
+ # </meta_attributes>
+ # </master>
+ # </resources>"""
+ # )
+
+ def test_put_options_after_master_as_primitive_options__original_behaviour(
+ self
+ ):
"""
- fixes bz 1378107 (do not use master options as primitive options)
+ downstream temporary behaviour
+ fixes bz 1458153
"""
self.assert_effect(
"resource create R ocf:heartbeat:Dummy state=a"
- " --master is-managed=false --force"
+ " --master fake=false --force"
,
"""<resources>
<master id="R-master">
@@ -694,6 +736,9 @@ class SuccessMaster(ResourceTestLocal):
type="Dummy"
>
<instance_attributes id="R-instance_attributes">
+ <nvpair id="R-instance_attributes-fake" name="fake"
+ value="false"
+ />
<nvpair id="R-instance_attributes-state"
name="state" value="a"
/>
@@ -719,22 +764,58 @@ class SuccessMaster(ResourceTestLocal):
/>
</operations>
</primitive>
- <meta_attributes id="R-master-meta_attributes">
- <nvpair id="R-master-meta_attributes-is-managed"
- name="is-managed" value="false"
- />
- </meta_attributes>
</master>
</resources>"""
- )
-
- def test_put_options_after_master_as_its_meta_fix_2(self):
+ ,
+ output="Warning: flag '--master' is deprecated, use keyword"
+ " 'master' instead (see the usage)\n"
+ )
+
+
+ # def test_put_options_after_master_as_its_meta_fix_2(self):
+ # """
+ # fixes bz 1378107 (do not use master options as operations)
+ # """
+ # self.assert_effect(
+ # "resource create R ocf:heartbeat:Dummy state=a op monitor"
+ # " interval=10s --master is-managed=false --force"
+ # " --no-default-ops"
+ # ,
+ # """<resources>
+ # <master id="R-master">
+ # <primitive class="ocf" id="R" provider="heartbeat"
+ # type="Dummy"
+ # >
+ # <instance_attributes id="R-instance_attributes">
+ # <nvpair id="R-instance_attributes-state"
+ # name="state" value="a"
+ # />
+ # </instance_attributes>
+ # <operations>
+ # <op id="R-monitor-interval-10s" interval="10s"
+ # name="monitor"
+ # />
+ # </operations>
+ # </primitive>
+ # <meta_attributes id="R-master-meta_attributes">
+ # <nvpair id="R-master-meta_attributes-is-managed"
+ # name="is-managed" value="false"
+ # />
+ # </meta_attributes>
+ # </master>
+ # </resources>"""
+ # )
+
+ def test_put_options_after_master_as_operation_opts__original_behaviour(
+ self
+ ):
"""
- fixes bz 1378107 (do not use master options as operations)
+ downstream temporary behaviour
+ fixes bz 1458153
"""
self.assert_effect(
"resource create R ocf:heartbeat:Dummy state=a op monitor"
- " interval=10s --master is-managed=false --force"
+ " interval=10s --master timeout=3m --force"
" --no-default-ops"
,
"""<resources>
@@ -749,22 +830,53 @@ class SuccessMaster(ResourceTestLocal):
</instance_attributes>
<operations>
<op id="R-monitor-interval-10s" interval="10s"
- name="monitor"
+ name="monitor" timeout="3m"
/>
</operations>
</primitive>
- <meta_attributes id="R-master-meta_attributes">
- <nvpair id="R-master-meta_attributes-is-managed"
- name="is-managed" value="false"
- />
- </meta_attributes>
</master>
</resources>"""
- )
-
- def test_do_not_steal_primitive_meta_options(self):
+ ,
+ output="Warning: flag '--master' is deprecated, use keyword"
+ " 'master' instead (see the usage)\n"
+ )
+
+ # def test_do_not_steal_primitive_meta_options(self):
+ # """
+ # fixes bz 1378107
+ # """
+ # self.assert_effect(
+ # "resource create R ocf:heartbeat:Dummy meta a=b --master b=c"
+ # " --no-default-ops"
+ # ,
+ # """<resources>
+ # <master id="R-master">
+ # <primitive class="ocf" id="R" provider="heartbeat"
+ # type="Dummy"
+ # >
+ # <meta_attributes id="R-meta_attributes">
+ # <nvpair id="R-meta_attributes-a" name="a"
+ # value="b"
+ # />
+ # </meta_attributes>
+ # <operations>
+ # <op id="R-monitor-interval-10" interval="10"
+ # name="monitor" timeout="20"
+ # />
+ # </operations>
+ # </primitive>
+ # <meta_attributes id="R-master-meta_attributes">
+ # <nvpair id="R-master-meta_attributes-b" name="b"
+ # value="c"
+ # />
+ # </meta_attributes>
+ # </master>
+ # </resources>"""
+ # )
+ def test_steals_primitive_meta_options__original_behaviour(self):
"""
- fixes bz 1378107
+ downstream temporary behaviour
+ fixes bz 1458153
"""
self.assert_effect(
"resource create R ocf:heartbeat:Dummy meta a=b --master b=c"
@@ -775,11 +887,6 @@ class SuccessMaster(ResourceTestLocal):
<primitive class="ocf" id="R" provider="heartbeat"
type="Dummy"
>
- <meta_attributes id="R-meta_attributes">
- <nvpair id="R-meta_attributes-a" name="a"
- value="b"
- />
- </meta_attributes>
<operations>
<op id="R-monitor-interval-10s" interval="10s"
name="monitor" timeout="20s"
@@ -787,18 +894,24 @@ class SuccessMaster(ResourceTestLocal):
</operations>
</primitive>
<meta_attributes id="R-master-meta_attributes">
+ <nvpair id="R-master-meta_attributes-a" name="a"
+ value="b"
+ />
<nvpair id="R-master-meta_attributes-b" name="b"
value="c"
/>
</meta_attributes>
</master>
</resources>"""
+ ,
+ output="Warning: flag '--master' is deprecated, use keyword"
+ " 'master' instead (see the usage)\n"
)
def test_takes_master_meta_attributes(self):
self.assert_effect(
"resource create --no-default-ops R ocf:heartbeat:IPaddr2"
- " ip=192.168.0.99 --master cidr_netmask=32"
+ " ip=192.168.0.99 master cidr_netmask=32"
,
"""<resources>
<master id="R-master">
@@ -1048,7 +1161,7 @@ class FailOrWarn(ResourceTestLocal):
def test_error_master_clone_combination(self):
self.assert_pcs_fail(
"resource create R ocf:heartbeat:Dummy --no-default-ops --clone"
- " --master"
+ " master"
,
"Error: you can specify only one of clone, master, bundle or"
" --group\n"
@@ -1056,7 +1169,7 @@ class FailOrWarn(ResourceTestLocal):
def test_error_master_group_combination(self):
self.assert_pcs_fail(
- "resource create R ocf:heartbeat:Dummy --no-default-ops --master"
+ "resource create R ocf:heartbeat:Dummy --no-default-ops master"
" --group G"
,
"Error: you can specify only one of clone, master, bundle or"
@@ -1074,7 +1187,7 @@ class FailOrWarn(ResourceTestLocal):
def test_error_bundle_master_combination(self):
self.assert_pcs_fail(
- "resource create R ocf:heartbeat:Dummy --no-default-ops --master"
+ "resource create R ocf:heartbeat:Dummy --no-default-ops master"
" bundle bundle_id"
,
"Error: you can specify only one of clone, master, bundle or"
diff --git a/pcs/test/test_constraints.py b/pcs/test/test_constraints.py
index df8594a5..671f0122 100644
--- a/pcs/test/test_constraints.py
+++ b/pcs/test/test_constraints.py
@@ -393,43 +393,43 @@ Ticket Constraints:
def testColocationConstraints(self):
# see also BundleColocation
- line = "resource create M1 ocf:heartbeat:Dummy --master"
+ line = "resource create M1 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == ""
- line = "resource create M2 ocf:heartbeat:Dummy --master"
+ line = "resource create M2 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == ""
- line = "resource create M3 ocf:heartbeat:Dummy --master"
+ line = "resource create M3 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == "",[returnVal, output]
- line = "resource create M4 ocf:heartbeat:Dummy --master"
+ line = "resource create M4 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == "",[returnVal, output]
- line = "resource create M5 ocf:heartbeat:Dummy --master"
+ line = "resource create M5 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == "",[returnVal, output]
- line = "resource create M6 ocf:heartbeat:Dummy --master"
+ line = "resource create M6 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == "",[returnVal, output]
- line = "resource create M7 ocf:heartbeat:Dummy --master"
+ line = "resource create M7 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == "",[returnVal, output]
- line = "resource create M8 ocf:heartbeat:Dummy --master"
+ line = "resource create M8 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == "",[returnVal, output]
- line = "resource create M9 ocf:heartbeat:Dummy --master"
+ line = "resource create M9 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == "",[returnVal, output]
- line = "resource create M10 ocf:heartbeat:Dummy --master"
+ line = "resource create M10 ocf:heartbeat:Dummy master"
output, returnVal = pcs(temp_cib, line)
assert returnVal == 0 and output == ""
@@ -1066,7 +1066,7 @@ Ticket Constraints:
assert returnVal == 1
def testLocationBadRules(self):
- o,r = pcs("resource create stateful0 ocf:heartbeat:Dummy --master")
+ o,r = pcs("resource create stateful0 ocf:heartbeat:Dummy master")
ac(o,"")
assert r == 0
@@ -1087,7 +1087,7 @@ Ticket Constraints:
""")
assert r == 0
- o,r = pcs("resource create stateful1 ocf:heartbeat:Dummy --master")
+ o,r = pcs("resource create stateful1 ocf:heartbeat:Dummy master")
ac(o,"")
assert r == 0
@@ -1126,7 +1126,7 @@ Ticket Constraints:
ac(o,"")
assert r == 0
- o,r = pcs("resource create stateful1 ocf:pacemaker:Stateful --master")
+ o,r = pcs("resource create stateful1 ocf:pacemaker:Stateful master")
ac(o, """\
Warning: changing a monitor operation interval from 10 to 11 to make the operation unique
""")
@@ -1247,7 +1247,7 @@ Ticket Constraints:
self.assertEqual(0, returnVal)
output, returnVal = pcs(
- "resource create stateful1 ocf:pacemaker:Stateful --master"
+ "resource create stateful1 ocf:pacemaker:Stateful master"
)
ac(output, """\
Warning: changing a monitor operation interval from 10 to 11 to make the operation unique
diff --git a/pcs/test/test_resource.py b/pcs/test/test_resource.py
index fd1a0731..3a47053f 100644
--- a/pcs/test/test_resource.py
+++ b/pcs/test/test_resource.py
@@ -2898,7 +2898,7 @@ Ticket Constraints:
output, returnVal = pcs(
temp_cib,
- "resource create --no-default-ops D2 ocf:heartbeat:Dummy --master"
+ "resource create --no-default-ops D2 ocf:heartbeat:Dummy master"
)
assert returnVal == 0
assert output == "", [output]
@@ -3011,7 +3011,7 @@ Warning: changing a monitor operation interval from 10 to 11 to make the operati
ac(o,"")
assert r == 0
- o,r = pcs("resource create D3 ocf:heartbeat:Dummy --master")
+ o,r = pcs("resource create D3 ocf:heartbeat:Dummy master")
ac(o,"")
assert r == 0
@@ -3237,7 +3237,7 @@ Warning: changing a monitor operation interval from 10 to 11 to make the operati
output, returnVal = pcs(
temp_cib,
- "resource create --no-default-ops dummy ocf:heartbeat:Dummy --master"
+ "resource create --no-default-ops dummy ocf:heartbeat:Dummy master"
)
ac(output, "")
self.assertEqual(0, returnVal)
@@ -3935,7 +3935,7 @@ Error: Cannot remove more than one resource from cloned group
# However those test the pcs library. I'm leaving these tests here to
# test the cli part for now.
self.assert_pcs_success(
- "resource create --no-default-ops dummy ocf:pacemaker:Stateful --master",
+ "resource create --no-default-ops dummy ocf:pacemaker:Stateful master",
"Warning: changing a monitor operation interval from 10 to 11 to make the operation unique\n"
)
@@ -5548,7 +5548,7 @@ class CloneMasterUpdate(unittest.TestCase, AssertPcsMixin):
def test_no_op_allowed_in_master_update(self):
self.assert_pcs_success(
- "resource create dummy ocf:heartbeat:Dummy --master"
+ "resource create dummy ocf:heartbeat:Dummy master"
)
self.assert_pcs_success(
"resource show dummy-master",
diff --git a/pcs/utils.py b/pcs/utils.py
index e56a1e8b..793f0b5e 100644
--- a/pcs/utils.py
+++ b/pcs/utils.py
@@ -2982,6 +2982,13 @@ def get_modifiers():
"wait": pcs_options.get("--wait", False),
"watchdog": pcs_options.get("--watchdog", []),
"no_watchdog_validation": "--no-watchdog-validation" in pcs_options,
+
+ #This is for `pcs resource create`. Fix of the bug
+ #https://bugzilla.redhat.com/show_bug.cgi?id=1378107
+ #caused problems in openstack which uses `pcs resource create`
+ #see https://bugzilla.redhat.com/show_bug.cgi?id=1458153
+ #so we give back the original misbehavior of master here temporarily.
+ "master": "--master" in pcs_options,
}
def exit_on_cmdline_input_errror(error, main_name, usage_name):
--
2.21.0