From 0b642f384198d9df09c33ba1473909b7aeb4a572 Mon Sep 17 00:00:00 2001 From: Ivan Devat Date: Mon, 5 Jun 2017 17:13:41 +0200 Subject: [PATCH] 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 7d6f6006..5c709245 100644 --- a/pcs/cli/common/parse_args.py +++ b/pcs/cli/common/parse_args.py @@ -299,7 +299,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 c6bb0aca..f615f682 100644 --- a/pcs/resource.py +++ b/pcs/resource.py @@ -391,6 +391,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", ], """ @@ -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", """ @@ -680,13 +680,55 @@ class SuccessMaster(ResourceTestLocal): """ ) - 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" + # , + # """ + # + # + # + # + # + # + # + # + # + # + # + # + # + # + # + # """ + # ) + + 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" , """ @@ -694,6 +736,9 @@ class SuccessMaster(ResourceTestLocal): type="Dummy" > + @@ -719,22 +764,58 @@ class SuccessMaster(ResourceTestLocal): /> - - - """ - ) - - 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" + # , + # """ + # + # + # + # + # + # + # + # + # + # + # + # + # + # """ + # ) + + 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" , """ @@ -749,22 +830,53 @@ class SuccessMaster(ResourceTestLocal): - - - """ - ) - - 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" + # , + # """ + # + # + # + # + # + # + # + # + # + # + # + # + # + # """ + # ) + 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): - - - + """ + , + 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" , """ @@ -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 d2278e08..142fb6f7 100644 --- a/pcs/test/test_constraints.py +++ b/pcs/test/test_constraints.py @@ -349,43 +349,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 == "" @@ -939,7 +939,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 @@ -960,7 +960,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 @@ -999,7 +999,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 """) @@ -1120,7 +1120,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 3091517c..f514f9ed 100644 --- a/pcs/test/test_resource.py +++ b/pcs/test/test_resource.py @@ -2738,7 +2738,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] @@ -2851,7 +2851,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 @@ -3077,7 +3077,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) @@ -3742,7 +3742,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" ) @@ -5355,7 +5355,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 8515aae9..c5e3c171 100644 --- a/pcs/utils.py +++ b/pcs/utils.py @@ -2962,6 +2962,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.17.0