From 84e927d4bb3dc38fb3717dfec602e5625e7f882f 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 --- 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 a12cca5..2809722 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 58ad54e..0c812bc 100644 --- a/octaviaclient/tests/unit/osc/v2/test_health_monitor.py +++ b/octaviaclient/tests/unit/osc/v2/test_health_monitor.py @@ -157,6 +157,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 ac6b3a5..6f89b47 100644 --- a/octaviaclient/tests/unit/osc/v2/test_l7policy.py +++ b/octaviaclient/tests/unit/osc/v2/test_l7policy.py @@ -173,6 +173,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 e40f9e9..2da8c65 100644 --- a/octaviaclient/tests/unit/osc/v2/test_listener.py +++ b/octaviaclient/tests/unit/osc/v2/test_listener.py @@ -162,6 +162,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 f37d96d..7687a43 100644 --- a/octaviaclient/tests/unit/osc/v2/test_load_balancer.py +++ b/octaviaclient/tests/unit/osc/v2/test_load_balancer.py @@ -305,6 +305,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 5606499..2e5724f 100644 --- a/octaviaclient/tests/unit/osc/v2/test_pool.py +++ b/octaviaclient/tests/unit/osc/v2/test_pool.py @@ -155,6 +155,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.