Attachment 946487 Details for Bug 1149677 – proposed fix

[patch] proposed fix

0001-Fixed-resource-move-ban-clear-for-clone-and-master.patch (text/plain), 7.15 KB, created by Tomas Jelinek on 2014-10-13 16:41:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomas Jelinek

Created: 2014-10-13 16:41:10 UTC

Size: 7.15 KB

patch

obsolete

>From 7f08a1fb2f7f49543583dc09d0cab904c35d5a52 Mon Sep 17 00:00:00 2001
>From: Tomas Jelinek <tojeline@redhat.com>
>Date: Mon, 13 Oct 2014 18:28:07 +0200
>Subject: [PATCH] Fixed resource move/ban/clear for clone and master
>
>* Added support for clones
>* Added check for group clones and group master/slave resources
>* Fixed finding the master id when --master is used and the specified
>  resource is not master
>---
> pcs/resource.py           | 59 +++++++++++++++++++++++++++++-----
> pcs/test/test_resource.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 131 insertions(+), 8 deletions(-)
>
>diff --git a/pcs/resource.py b/pcs/resource.py
>index e8e18f0..0956b50 100644
>--- a/pcs/resource.py
>+++ b/pcs/resource.py
>@@ -457,19 +457,62 @@ def resource_move(argv,clear=False,ban=False):
>         usage.resource()
>         sys.exit(1)
> 
>-    if not utils.does_exist("//primitive[@id='"+resource_id+"']") and not utils.does_exist("//group[@id='"+resource_id+"']") and not utils.does_exist("//master[@id='"+resource_id+"']"):
>+    dom = utils.get_cib_dom()
>+    if (
>+        not utils.dom_get_resource(dom, resource_id)
>+        and
>+        not utils.dom_get_group(dom, resource_id)
>+        and
>+        not utils.dom_get_master(dom, resource_id)
>+        and
>+        not utils.dom_get_clone(dom, resource_id)
>+    ):
>         utils.err("%s is not a valid resource" % resource_id)
> 
>-    if utils.is_resource_clone(resource_id) and not clear and not ban:
>+    if (
>+        not clear and not ban
>+        and
>+        (
>+            utils.dom_get_clone(dom, resource_id)
>+            or
>+            utils.dom_get_resource_clone(dom, resource_id)
>+            or
>+            utils.dom_get_group_clone(dom, resource_id)
>+        )
>+    ):
>         utils.err("cannot move cloned resources")
> 
>-    if utils.is_resource_masterslave(resource_id) and not clear and not ban and not "--master" in utils.pcs_options:
>-        master_id = utils.get_resource_master_id(resource_id)
>-        utils.err("to move Master/Slave resources you must use --master and the master id (%s)" % master_id)
>+    if (
>+        not clear and not ban
>+        and
>+        "--master" not in utils.pcs_options
>+        and
>+        (
>+            utils.dom_get_resource_masterslave(dom, resource_id)
>+            or
>+            utils.dom_get_group_masterslave(dom, resource_id)
>+        )
>+    ):
>+        master = utils.dom_get_resource_clone_ms_parent(dom, resource_id)
>+        utils.err(
>+            "to move Master/Slave resources you must use --master "
>+                "and the master id (%s)"
>+            % master.getAttribute("id")
>+        )
> 
>-    if "--master" in utils.pcs_options and not utils.does_exist("//master[@id='"+resource_id+"']"):
>-        master_id = utils.get_resource_master_id(resource_id)
>-        utils.err("when specifying --master you must use the master id (%s)" % master_id)
>+    if (
>+        "--master" in utils.pcs_options
>+        and
>+        not utils.dom_get_master(dom, resource_id)
>+    ):
>+        master_clone = utils.dom_get_resource_clone_ms_parent(dom, resource_id)
>+        if master_clone and master_clone.tagName == "master":
>+            utils.err(
>+                "when specifying --master you must use the master id (%s)"
>+                % master_clone.getAttribute("id")
>+            )
>+        else:
>+            utils.err("when specifying --master you must use the master id")
> 
>     if "--master" in utils.pcs_options:
>         other_options.append("--master")
>diff --git a/pcs/test/test_resource.py b/pcs/test/test_resource.py
>index 31a6e8a..ec8db1b 100644
>--- a/pcs/test/test_resource.py
>+++ b/pcs/test/test_resource.py
>@@ -1467,6 +1467,82 @@ Colocation Constraints:
>         )
>         self.assertEquals(1, returnVal)
> 
>+        output, returnVal = pcs(temp_cib, "resource move dummy rh7-1 --master")
>+        ac(output, """\
>+Error: when specifying --master you must use the master id
>+""")
>+        self.assertEquals(1, returnVal)
>+
>+    def testCloneMoveBanClear(self):
>+        # Load nodes into cib so move will work
>+        utils.usefile = True
>+        utils.filename = temp_cib
>+        output, returnVal = utils.run(["cibadmin", "-M", '--xml-text', '<nodes><node id="1" uname="rh7-1"><instance_attributes id="nodes-1"/></node><node id="2" uname="rh7-2"><instance_attributes id="nodes-2"/></node></nodes>'])
>+        ac(output, "")
>+        self.assertEquals(0, returnVal)
>+
>+        output, returnVal = pcs(
>+            temp_cib, "resource create --no-default-ops D1 Dummy --clone"
>+        )
>+        ac(output, "")
>+        self.assertEquals(0, returnVal)
>+
>+        output, returnVal = pcs(
>+            temp_cib, "resource create --no-default-ops D2 Dummy --group DG"
>+        )
>+        ac(output, "")
>+        self.assertEquals(0, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource clone DG")
>+        ac(output, "")
>+        self.assertEquals(0, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource move D1")
>+        ac(output, "Error: cannot move cloned resources\n")
>+        self.assertEquals(1, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource move D1-clone")
>+        ac(output, "Error: cannot move cloned resources\n")
>+        self.assertEquals(1, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource move D2")
>+        ac(output, "Error: cannot move cloned resources\n")
>+        self.assertEquals(1, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource move DG")
>+        ac(output, "Error: cannot move cloned resources\n")
>+        self.assertEquals(1, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource move DG-clone")
>+        ac(output, "Error: cannot move cloned resources\n")
>+        self.assertEquals(1, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource ban DG-clone rh7-1")
>+        ac(output, "")
>+        self.assertEquals(0, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "constraint --full")
>+        ac(output, """\
>+Location Constraints:
>+  Resource: DG-clone
>+    Disabled on: rh7-1 (score:-INFINITY) (role: Started) (id:cli-ban-DG-clone-on-rh7-1)
>+Ordering Constraints:
>+Colocation Constraints:
>+""")
>+        self.assertEquals(0, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "resource clear DG-clone")
>+        ac(output, "")
>+        self.assertEquals(0, returnVal)
>+
>+        output, returnVal = pcs(temp_cib, "constraint --full")
>+        ac(output, """\
>+Location Constraints:
>+Ordering Constraints:
>+Colocation Constraints:
>+""")
>+        self.assertEquals(0, returnVal)
>+
>     def testNoMoveMSClone(self):
>         output, returnVal  = pcs(temp_cib, "resource create --no-default-ops D0 Dummy")
>         assert returnVal == 0
>@@ -1484,6 +1560,10 @@ Colocation Constraints:
>         assert returnVal == 1
>         assert output == "Error: cannot move cloned resources\n", [output]
> 
>+        output, returnVal  = pcs(temp_cib, "resource move D1-clone")
>+        assert returnVal == 1
>+        assert output == "Error: cannot move cloned resources\n", [output]
>+
>         output, returnVal  = pcs(temp_cib, "resource move D2")
>         assert returnVal == 1
>         assert output == "Error: to move Master/Slave resources you must use --master and the master id (D2-master)\n", [output]
>-- 
>1.9.1
>

Actions: View | Diff

Attachments on bug 1149677: 946487