From 6805a235de50925ed7f30ac79b3d96be3f5d71df Mon Sep 17 00:00:00 2001

From: Tomas Jelinek <tojeline@redhat.com>

Date: Mon, 25 Jul 2016 14:23:23 +0200

Subject: [PATCH 1/2] remove dead code

Function resource_master_remove could not ever be called because conditions

with xpath queries were never True. In the case when the resource_id was

an id of a master resource, it got changed to the id of the master's child

right at the beginning of resource_remove function.

---

 pcs/resource.py | 49 ++++++-------------------------------------------

 1 file changed, 6 insertions(+), 43 deletions(-)

diff --git a/pcs/resource.py b/pcs/resource.py

index 9384a21..24128ba 100644

--- a/pcs/resource.py

+++ b/pcs/resource.py

@@ -1618,45 +1618,9 @@ def resource_master_create(dom, argv, update=False, master_id=None):

     return dom, master_element.getAttribute("id")

-def resource_master_remove(argv):

-    if len(argv) < 1:

-        usage.resource()

-        sys.exit(1)

-

-    dom = utils.get_cib_dom()

-    master_id = argv.pop(0)

-

-    master_found = False

-# Check to see if there's a resource/group with the master_id if so, we remove the parent

-    for rg in (dom.getElementsByTagName("primitive") + dom.getElementsByTagName("group")):

-        if rg.getAttribute("id") == master_id and rg.parentNode.tagName == "master":

-            master_id = rg.parentNode.getAttribute("id")

-

-    resources_to_cleanup = []

-    for master in dom.getElementsByTagName("master"):

-        if master.getAttribute("id") == master_id:

-            childNodes = master.getElementsByTagName("primitive")

-            for child in childNodes:

-                resources_to_cleanup.append(child.getAttribute("id"))

-            master_found = True

-            break

-

-    if not master_found:

-        utils.err("Unable to find multi-state resource with id %s" % master_id)

-

-    constraints_element = dom.getElementsByTagName("constraints")

-    if len(constraints_element) > 0:

-        constraints_element = constraints_element[0]

-        for resource_id in resources_to_cleanup:

-            remove_resource_references(

-                dom, resource_id, constraints_element=constraints_element

-            )

-    master.parentNode.removeChild(master)

-    print("Removing Master - " + master_id)

-    utils.replace_cib_configuration(dom)

-

 def resource_remove(resource_id, output = True):

     dom = utils.get_cib_dom()

+    # if resource is a clone or a master, work with its child instead

     cloned_resource = utils.dom_get_clone_ms_resource(dom, resource_id)

     if cloned_resource:

         resource_id = cloned_resource.getAttribute("id")

@@ -1704,16 +1668,15 @@ def resource_remove(resource_id, output = True):

             resource_remove(res.getAttribute("id"))

         sys.exit(0)

+    # now we know resource is not a group, a clone nor a master

+    # because of the conditions above

+    if not utils.does_exist('//resources/descendant::primitive[@id="'+resource_id+'"]'):

+        utils.err("Resource '{0}' does not exist.".format(resource_id))

+

     group_xpath = '//group/primitive[@id="'+resource_id+'"]/..'

     group = utils.get_cib_xpath(group_xpath)

     num_resources_in_group = 0

-    if not utils.does_exist('//resources/descendant::primitive[@id="'+resource_id+'"]'):

-        if utils.does_exist('//resources/master[@id="'+resource_id+'"]'):

-            return resource_master_remove([resource_id])

-

-        utils.err("Resource '{0}' does not exist.".format(resource_id))

-

     if (group != ""):

         num_resources_in_group = len(parseString(group).documentElement.getElementsByTagName("primitive"))

-- 

1.8.3.1

From cd96c34c7ad1f4f767c0d14475b683a70c3b0862 Mon Sep 17 00:00:00 2001

From: Tomas Jelinek <tojeline@redhat.com>

Date: Mon, 25 Jul 2016 17:34:35 +0200

Subject: [PATCH 2/2] when removing a remote node remove it from pacemaker's

 caches as well

---

 pcs/cluster.py         |  6 +++++

 pcs/resource.py        | 64 ++++++++++++++++++++++++++++----------------------

 pcs/test/test_utils.py |  9 +++++++

 pcs/utils.py           |  8 +++++++

 4 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/pcs/cluster.py b/pcs/cluster.py

index 7a8615d..1c3b425 100644

--- a/pcs/cluster.py

+++ b/pcs/cluster.py

@@ -1922,6 +1922,12 @@ def cluster_remote_node(argv):

             nvpair.parentNode.removeChild(nvpair)

         dom = constraint.remove_constraints_containing_node(dom, hostname)

         utils.replace_cib_configuration(dom)

+        if not utils.usefile:

+            output, retval = utils.run([

+                "crm_node", "--force", "--remove", hostname

+            ])

+            if retval != 0:

+                utils.err("unable to remove: {0}".fomat(output))

     else:

         usage.cluster(["remote-node"])

         sys.exit(1)

diff --git a/pcs/resource.py b/pcs/resource.py

index 24128ba..a85f46f 100644

--- a/pcs/resource.py

+++ b/pcs/resource.py

@@ -929,31 +929,11 @@ def resource_update(res_id,args):

             ia.setAttribute("value", val)

             instance_attributes.appendChild(ia)

-    meta_attributes = resource.getElementsByTagName("meta_attributes")

-    if len(meta_attributes) == 0:

-        meta_attributes = dom.createElement("meta_attributes")

-        meta_attributes.setAttribute("id", res_id + "-meta_attributes")

-        resource.appendChild(meta_attributes)

-    else:

-        meta_attributes = meta_attributes[0]

-

-    meta_attrs = utils.convert_args_to_tuples(meta_values)

-    for (key,val) in meta_attrs:

-        meta_found = False

-        for ma in meta_attributes.getElementsByTagName("nvpair"):

-            if ma.getAttribute("name") == key:

-                meta_found = True

-                if val == "":

-                    meta_attributes.removeChild(ma)

-                else:

-                    ma.setAttribute("value", val)

-                break

-        if not meta_found:

-            ma = dom.createElement("nvpair")

-            ma.setAttribute("id", res_id + "-meta_attributes-" + key)

-            ma.setAttribute("name", key)

-            ma.setAttribute("value", val)

-            meta_attributes.appendChild(ma)

+    remote_node_name = utils.dom_get_resource_remote_node_name(resource)

+    utils.dom_update_meta_attr(

+        resource,

+        utils.convert_args_to_tuples(meta_values)

+    )

     operations = resource.getElementsByTagName("operations")

     if len(operations) == 0:

@@ -1005,6 +985,17 @@ def resource_update(res_id,args):

     utils.replace_cib_configuration(dom)

+    if (

+        remote_node_name

+        and

+        remote_node_name != utils.dom_get_resource_remote_node_name(resource)

+    ):

+        # if the resource was a remote node and it is not anymore, (or its name

+        # changed) we need to tell pacemaker about it

+        output, retval = utils.run([

+            "crm_node", "--force", "--remove", remote_node_name

+        ])

+

     if "--wait" in utils.pcs_options:

         args = ["crm_resource", "--wait"]

         if wait_timeout:

@@ -1231,10 +1222,22 @@ def resource_meta(res_id, argv):

     if "--wait" in utils.pcs_options:

         wait_timeout = utils.validate_wait_get_timeout()

+    remote_node_name = utils.dom_get_resource_remote_node_name(resource_el)

     utils.dom_update_meta_attr(resource_el, utils.convert_args_to_tuples(argv))

     utils.replace_cib_configuration(dom)

+    if (

+        remote_node_name

+        and

+        remote_node_name != utils.dom_get_resource_remote_node_name(resource_el)

+    ):

+        # if the resource was a remote node and it is not anymore, (or its name

+        # changed) we need to tell pacemaker about it

+        output, retval = utils.run([

+            "crm_node", "--force", "--remove", remote_node_name

+        ])

+

     if "--wait" in utils.pcs_options:

         args = ["crm_resource", "--wait"]

         if wait_timeout:

@@ -1714,11 +1717,12 @@ def resource_remove(resource_id, output = True):

     )

     dom = utils.get_cib_dom()

     resource_el = utils.dom_get_resource(dom, resource_id)

+    remote_node_name = None

     if resource_el:

-        remote_node = utils.dom_get_resource_remote_node_name(resource_el)

-        if remote_node:

+        remote_node_name = utils.dom_get_resource_remote_node_name(resource_el)

+        if remote_node_name:

             dom = constraint.remove_constraints_containing_node(

-                dom, remote_node, output

+                dom, remote_node_name, output

             )

             utils.replace_cib_configuration(dom)

             dom = utils.get_cib_dom()

@@ -1784,6 +1788,10 @@ def resource_remove(resource_id, output = True):

             if output == True:

                 utils.err("Unable to remove resource '%s' (do constraints exist?)" % (resource_id))

             return False

+    if remote_node_name and not utils.usefile:

+        output, retval = utils.run([

+            "crm_node", "--force", "--remove", remote_node_name

+        ])

     return True

 def stonith_level_rm_device(cib_dom, stn_id):

diff --git a/pcs/test/test_utils.py b/pcs/test/test_utils.py

index 819f8ee..192048e 100644

--- a/pcs/test/test_utils.py

+++ b/pcs/test/test_utils.py

@@ -273,6 +273,9 @@ class UtilsTest(unittest.TestCase):

                             name="remote-node" value="guest2"/>

                     </instance_attributes>

                 </primitive>

+                

+                        class="ocf" provider="pacemaker" type="remote">

+                </primitive>

             </resources>

         """).documentElement

         resources = dom.getElementsByTagName("resources")[0]

@@ -296,6 +299,12 @@ class UtilsTest(unittest.TestCase):

                 utils.dom_get_resource(dom, "vm-guest1")

             )

         )

+        self.assertEqual(

+            "dummy3",

+            utils.dom_get_resource_remote_node_name(

+                utils.dom_get_resource(dom, "dummy3")

+            )

+        )

     def test_dom_get_meta_attr_value(self):

         dom = self.get_cib_empty()

diff --git a/pcs/utils.py b/pcs/utils.py

index a7ed975..25274dc 100644

--- a/pcs/utils.py

+++ b/pcs/utils.py

@@ -1252,6 +1252,14 @@ def validate_constraint_resource(dom, resource_id):

 def dom_get_resource_remote_node_name(dom_resource):

     if dom_resource.tagName != "primitive":

         return None

+    if (

+        dom_resource.getAttribute("class").lower() == "ocf"

+        and

+        dom_resource.getAttribute("provider").lower() == "pacemaker"

+        and

+        dom_resource.getAttribute("type").lower() == "remote"

+    ):

+        return dom_resource.getAttribute("id")

     return dom_get_meta_attr_value(dom_resource, "remote-node")

 def dom_get_meta_attr_value(dom_resource, meta_name):

-- 

1.8.3.1