From b56bf42759b18ed7b98dc98d3897b3cdf7b3ce5a Mon Sep 17 00:00:00 2001

From: Ondrej Mular <omular@redhat.com>

Date: Thu, 11 Jun 2020 16:53:09 +0200

Subject: [PATCH 1/6] GUI: fix race-condition when removing resource

---

 pcs/lib/cib/tools.py      | 30 ++++++++++++++++++++----------

 pcs/test/test_acl.py      |  4 ++--

 pcs/test/test_resource.py | 33 +++++++++++++++++++++++++++++++++

 pcsd/public/js/pcsd.js    |  2 +-

 4 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/pcs/lib/cib/tools.py b/pcs/lib/cib/tools.py

index ab2a9df5..e9599c43 100644

--- a/pcs/lib/cib/tools.py

+++ b/pcs/lib/cib/tools.py

@@ -56,11 +56,14 @@ class IdProvider(object):

         return report_list

-def does_id_exist(tree, check_id):

+def get_configuration_elements_by_id(tree, check_id):

     """

-    Checks to see if id exists in the xml dom passed

-    tree cib etree node

-    check_id id to check

+    Return any configuration elements (not in status section of cib) with value

+    of attribte id specified as 'check_id'

+

+    tree -- any element in xml tree, whole tree (not only its subtree) will be

+        searched

+    check_id -- id to find

     """

     # do not search in /cib/status, it may contain references to previously

@@ -70,7 +73,7 @@ def does_id_exist(tree, check_id):

     #which will be named the same as the value of the remote-node attribute of

     #the explicit resource. So the value of nvpair named "remote-node" is

     #considered to be id

-    existing = get_root(tree).xpath("""

+    return get_root(tree).xpath("""

         (

             /cib/*[name()!="status"]

             |

@@ -96,7 +99,15 @@ def does_id_exist(tree, check_id):

             )

         ]

     """.format(check_id))

-    return len(existing) > 0

+

+

+def does_id_exist(tree, check_id):

+    """

+    Checks to see if id exists in the xml dom passed

+    tree cib etree node

+    check_id id to check

+    """

+    return len(get_configuration_elements_by_id(tree, check_id)) > 0

 def validate_id_does_not_exist(tree, id):

     """

@@ -155,11 +166,10 @@ def find_element_by_tag_and_id(

     if element_list:

         return element_list[0]

-    element = get_root(context_element).find(

-        './/*[@id="{0}"]'.format(element_id)

-    )

+    elements = get_configuration_elements_by_id(context_element, element_id)

-    if element is not None:

+    if elements:

+        element = elements[0]

         raise LibraryError(

             reports.id_belongs_to_unexpected_type(

                 element_id,

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

index a59beb0a..2a3b6d66 100644

--- a/pcs/test/test_acl.py

+++ b/pcs/test/test_acl.py

@@ -395,7 +395,7 @@ Group: group2

         o,r = pcs("acl group delete user1")

         assert r == 1

-        ac(o,"Error: 'user1' is not an ACL group\n")

+        ac(o,"Error: ACL group 'user1' does not exist\n")

         o,r = pcs("acl")

         ac(o, """\

@@ -861,7 +861,7 @@ Role: role4

         self.assert_pcs_success("acl user create user1")

         self.assert_pcs_fail(

             "acl role assign role1 to group user1",

-            "Error: 'user1' is not an ACL group\n"

+            "Error: ACL group 'user1' does not exist\n"

         )

     def test_assign_unassign_role_to_group_with_to(self):

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

index 972323f9..fd1a0731 100644

--- a/pcs/test/test_resource.py

+++ b/pcs/test/test_resource.py

@@ -3542,6 +3542,39 @@ Error: Cannot remove more than one resource from cloned group

             )

         )

+    def testResourceDisableNonExistingWithFailedAction(self):

+        with open(temp_cib, "w") as f:

+            f.truncate()

+            f.write(

+                """

+                <cib epoch="557" num_updates="122" admin_epoch="0" crm_feature_set="3.0.14" validate-with="pacemaker-2.10" update-origin="rh7-3" update-client="crmd" cib-last-written="Thu Aug 23 16:49:17 2012" have-quorum="0" dc-uuid="2">

+                  <configuration>

+                    <crm_config/>

+                    <nodes>

+                    </nodes>

+                    <resources/>

+                    <constraints/>

+                  </configuration>

+                  <status>

+                    <node_state id="1" uname="rh7-1" in_ccm="true" crmd="online" crm-debug-origin="do_update_resource" join="member" expected="member">

+                      <lrm id="1">

+                        <lrm_resources>

+                          <lrm_resource id="A" type="apache" class="ocf" provider="heartbeat">

+                            <lrm_rsc_op id="A_last_0" operation_key="A_monitor_0" operation="monitor" crm-debug-origin="do_update_resource" crm_feature_set="3.0.14" transition-key="1:1104:7:817ee250-d179-483a-819e-9be9cb0e06df" transition-magic="0:7;1:1104:7:817ee250-d179-483a-819e-9be9cb0e06df" exit-reason="" on_node="rh7-1" call-id="1079" rc-code="7" op-status="0" interval="0" last-run="1591791198" last-rc-change="1591791198" exec-time="275" queue-time="0" op-digest="f2317cad3d54cec5d7d7aa7d0bf35cf8"/>

+                          </lrm_resource>

+                        </lrm_resources>

+                      </lrm>

+                    </node_state>

+                  </status>

+                </cib>

+                """

+            )

+        self.assert_pcs_fail(

+            "resource disable A",

+            "Error: bundle/clone/group/master/resource 'A' does not exist\n"

+        )

+

+

     def testResourceEnable(self):

         # These tests were moved to

         # pcs/lib/commands/test/resource/test_resource_enable_disable.py .

diff --git a/pcsd/public/js/pcsd.js b/pcsd/public/js/pcsd.js

index 20c19434..60c3da59 100644

--- a/pcsd/public/js/pcsd.js

+++ b/pcsd/public/js/pcsd.js

@@ -1441,7 +1441,7 @@ function remove_resource(ids, force) {

         error == "timeout" ||

         xhr.responseText == '{"noresponse":true}'

       ) {

-        message = "Operation takes longer to complete than expected.";

+        message = "Operation takes longer to complete than expected but it will continue in the background.";

       } else {

         message = "Unable to remove resources (" + error + ")";

         if (

-- 

2.21.0