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 84e927d4bb)

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)

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)

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)

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)

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)

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)

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)

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)

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)

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)

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.