From c2aa20175c425b5ace32ec6862dba1189ae1ad5a Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 21 May 2021 18:58:16 +0000 Subject: [PATCH] Improve the client performance on large clouds This patch fixes the client performance when used on large clouds. Previously the client was pulling down a full list of resources when mapping names to IDs and filtering on the client side. This is inefficient as it is transfering many more records than are necessary. This patch uses the filter capabilities of the OpenStack APIs to target the search for a resource ID. Change-Id: I6e68113aea9ce27beb6691bd2a398bf276a23ae2 (cherry picked from commit 84e927d4bb3dc38fb3717dfec602e5625e7f882f) --- octaviaclient/osc/v2/utils.py | 91 ++++++++++++++----- .../unit/osc/v2/test_availabilityzone.py | 2 + .../osc/v2/test_availabilityzoneprofile.py | 2 + .../tests/unit/osc/v2/test_flavor.py | 1 + .../tests/unit/osc/v2/test_flavorprofile.py | 2 + .../tests/unit/osc/v2/test_health_monitor.py | 2 + .../tests/unit/osc/v2/test_l7policy.py | 1 + .../tests/unit/osc/v2/test_listener.py | 1 + .../tests/unit/osc/v2/test_load_balancer.py | 2 + octaviaclient/tests/unit/osc/v2/test_pool.py | 1 + ...a-name-instead-of-ID-51a6637050386a90.yaml | 5 + 11 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/Improve-client-performance-when-using-a-name-instead-of-ID-51a6637050386a90.yaml diff --git a/octaviaclient/osc/v2/utils.py b/octaviaclient/osc/v2/utils.py index b99090e..d27f6ac 100644 --- a/octaviaclient/osc/v2/utils.py +++ b/octaviaclient/osc/v2/utils.py @@ -16,6 +16,7 @@ import munch from openstackclient.identity import common as identity_common from osc_lib import exceptions as osc_exc from osc_lib import utils +from oslo_utils import uuidutils from octaviaclient.api import exceptions from octaviaclient.osc.v2 import constants @@ -62,6 +63,62 @@ def _map_attrs(args, source_attr_map): return res +def _find_resource(list_funct, resource_name, root_tag, name, parent=None): + """Search for a resource by name and ID. + + This function will search for a resource by both the name and ID, + returning the resource once it finds a match. If no match is found, + an exception will be raised. + + :param list_funct: The resource list method to call during searches. + :param resource_name: The name of the resource type we are searching for. + :param root_tag: The root tag of the resource returned from the API. + :param name: The value we are searching for, a resource name or ID. + :param parent: The parent resource ID, when required. + :return: The resource found for the name or ID. + :raises osc_exc.CommandError: If more than one match or none are found. + """ + if parent: + parent_args = [parent] + else: + parent_args = [] + # Optimize the API call order if we got a UUID-like name or not + if uuidutils.is_uuid_like(name): + # Try by ID first + resource = list_funct(*parent_args, id=name)[root_tag] + if len(resource) == 1: + return resource[0] + + # Try by name next + resource = list_funct(*parent_args, name=name)[root_tag] + if len(resource) == 1: + return resource[0] + if len(resource) > 1: + msg = ("{0} {1} found with name or ID of {2}. Please try " + "again with UUID".format(len(resource), resource_name, + name)) + raise osc_exc.CommandError(msg) + else: + # Try by name first + resource = list_funct(*parent_args, name=name)[root_tag] + if len(resource) == 1: + return resource[0] + if len(resource) > 1: + msg = ("{0} {1} found with name or ID of {2}. Please try " + "again with UUID".format(len(resource), resource_name, + name)) + raise osc_exc.CommandError(msg) + + # Try by ID next + resource = list_funct(*parent_args, id=name)[root_tag] + if len(resource) == 1: + return resource[0] + + # We didn't find what we were looking for, raise a consistent error. + msg = "Unable to locate {0} in {1}".format(name, resource_name) + raise osc_exc.CommandError(msg) + + def get_resource_id(resource, resource_name, name): """Converts a resource name into a UUID for consumption for the API @@ -94,30 +151,20 @@ def get_resource_id(resource, resource_name, name): ).id return project_id return 'non-uuid' + if resource_name == 'members': - names = [re for re in resource(name['pool_id'])['members'] - if re.get('id') == name['member_id'] or - re.get('name') == name['member_id']] - name = name['member_id'] - if len(names) > 1: - msg = ("{0} {1} found with name or ID of {2}. Please try " - "again with UUID".format(len(names), resource_name, - name)) - raise osc_exc.CommandError(msg) - return names[0].get('id') + member = _find_resource(resource, resource_name, 'members', + name['member_id'], parent=name['pool_id']) + return member.get('id') + if resource_name == 'l7rules': - names = [re for re in resource(name['l7policy_id'])['rules'] - if re.get('id') == name['l7rule_id']] - name = name['l7rule_id'] - return names[0].get('id') - names = [re for re in resource()[resource_name] - if re.get('name') == name or re.get('id') == name] - if len(names) > 1: - msg = ("{0} {1} found with name or ID of {2}. Please try " - "again with UUID".format(len(names), resource_name, - name)) - raise osc_exc.CommandError(msg) - return names[0].get(primary_key) + l7rule = _find_resource(resource, resource_name, 'rules', + name['l7rule_id'], + parent=name['l7policy_id']) + return l7rule.get('id') + + resource = _find_resource(resource, resource_name, resource_name, name) + return resource.get(primary_key) except IndexError as e: msg = "Unable to locate {0} in {1}".format(name, resource_name) diff --git a/octaviaclient/tests/unit/osc/v2/test_availabilityzone.py b/octaviaclient/tests/unit/osc/v2/test_availabilityzone.py index 8141b4b..27a5911 100644 --- a/octaviaclient/tests/unit/osc/v2/test_availabilityzone.py +++ b/octaviaclient/tests/unit/osc/v2/test_availabilityzone.py @@ -94,6 +94,8 @@ class TestAvailabilityzoneDelete(TestAvailabilityzone): verifylist = [ ('availabilityzone', 'unknown_availabilityzone') ] + self.api_mock.availabilityzone_list.return_value = { + 'availability_zones': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_availabilityzoneprofile.py b/octaviaclient/tests/unit/osc/v2/test_availabilityzoneprofile.py index b7fe78d..27e8ec8 100644 --- a/octaviaclient/tests/unit/osc/v2/test_availabilityzoneprofile.py +++ b/octaviaclient/tests/unit/osc/v2/test_availabilityzoneprofile.py @@ -99,6 +99,8 @@ class TestAvailabilityzoneProfileDelete(TestAvailabilityzoneProfile): verifylist = [ ('availabilityzoneprofile', 'unknown_availabilityzoneprofile') ] + self.api_mock.availabilityzoneprofile_list.return_value = { + 'availability_zone_profiles': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_flavor.py b/octaviaclient/tests/unit/osc/v2/test_flavor.py index 9a4fe99..1d58319 100644 --- a/octaviaclient/tests/unit/osc/v2/test_flavor.py +++ b/octaviaclient/tests/unit/osc/v2/test_flavor.py @@ -92,6 +92,7 @@ class TestFlavorDelete(TestFlavor): verifylist = [ ('flavor', 'unknown_flavor') ] + self.api_mock.flavor_list.return_value = {'flavors': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_flavorprofile.py b/octaviaclient/tests/unit/osc/v2/test_flavorprofile.py index 9bd2019..43cbcca 100644 --- a/octaviaclient/tests/unit/osc/v2/test_flavorprofile.py +++ b/octaviaclient/tests/unit/osc/v2/test_flavorprofile.py @@ -94,6 +94,8 @@ class TestFlavorProfileDelete(TestFlavorProfile): verifylist = [ ('flavorprofile', 'unknown_flavorprofile') ] + self.api_mock.flavorprofile_list.return_value = { + 'flavorprofiles': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_health_monitor.py b/octaviaclient/tests/unit/osc/v2/test_health_monitor.py index 15c4558..d4dbe9c 100644 --- a/octaviaclient/tests/unit/osc/v2/test_health_monitor.py +++ b/octaviaclient/tests/unit/osc/v2/test_health_monitor.py @@ -101,6 +101,8 @@ class TestHealthMonitorDelete(TestHealthMonitor): verifylist = [ ('health_monitor', 'unknown_hm') ] + self.api_mock.health_monitor_list.return_value = { + 'healthmonitors': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_l7policy.py b/octaviaclient/tests/unit/osc/v2/test_l7policy.py index 152c0c3..ef0d27d 100644 --- a/octaviaclient/tests/unit/osc/v2/test_l7policy.py +++ b/octaviaclient/tests/unit/osc/v2/test_l7policy.py @@ -117,6 +117,7 @@ class TestL7PolicyDelete(TestL7Policy): verifylist = [ ('l7policy', 'unknown_policy') ] + self.api_mock.l7policy_list.return_value = {'l7policies': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_listener.py b/octaviaclient/tests/unit/osc/v2/test_listener.py index 32167f3..9ada164 100644 --- a/octaviaclient/tests/unit/osc/v2/test_listener.py +++ b/octaviaclient/tests/unit/osc/v2/test_listener.py @@ -108,6 +108,7 @@ class TestListenerDelete(TestListener): verifylist = [ ('listener', 'unknown_lb') ] + self.api_mock.listener_list.return_value = {'listeners': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_load_balancer.py b/octaviaclient/tests/unit/osc/v2/test_load_balancer.py index 1393ba2..a8a134a 100644 --- a/octaviaclient/tests/unit/osc/v2/test_load_balancer.py +++ b/octaviaclient/tests/unit/osc/v2/test_load_balancer.py @@ -237,6 +237,8 @@ class TestLoadBalancerDelete(TestLoadBalancer): verifylist = [ ('loadbalancer', 'unknown_lb') ] + self.api_mock.load_balancer_list.return_value = { + 'loadbalancers': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/octaviaclient/tests/unit/osc/v2/test_pool.py b/octaviaclient/tests/unit/osc/v2/test_pool.py index 9f72736..e7199a1 100644 --- a/octaviaclient/tests/unit/osc/v2/test_pool.py +++ b/octaviaclient/tests/unit/osc/v2/test_pool.py @@ -99,6 +99,7 @@ class TestPoolDelete(TestPool): verifylist = [ ('pool', 'unknown_pool') ] + self.api_mock.pool_list.return_value = {'pools': []} parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) diff --git a/releasenotes/notes/Improve-client-performance-when-using-a-name-instead-of-ID-51a6637050386a90.yaml b/releasenotes/notes/Improve-client-performance-when-using-a-name-instead-of-ID-51a6637050386a90.yaml new file mode 100644 index 0000000..bd298f2 --- /dev/null +++ b/releasenotes/notes/Improve-client-performance-when-using-a-name-instead-of-ID-51a6637050386a90.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Improved the client performance when using a name instead of the + resource ID.