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