From b56bf42759b18ed7b98dc98d3897b3cdf7b3ce5a Mon Sep 17 00:00:00 2001 From: Ondrej Mular 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( + """ + + + + + + + + + + + + + + + + + + + + + """ + ) + 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