From e782f49927a6b142a35f85a1a4a759fd2b1ceedf Mon Sep 17 00:00:00 2001
From: Pavlo Shchelokovskyy <shchelokovskyy@gmail.com>
Date: Mon, 14 May 2018 18:13:28 +0000
Subject: [PATCH] Add --name-lookup-one-by-one option to server list

usually in a big cloud there are many images and flavors,
while each given project might use only some of those.

This patch introduces '--name-lookup-one-by-one' argument to
server list command (mutually exclusive with '--no-name-lookup')

When provided (or either '--image' or '--flavor' is specified) to the
`server list` command, name resolving for
corresponding entity is now using targeted GET commands instead of
full entities list.

In some situations this can significantly speedup the execution of the
`server list` command by reducing the number of API requests performed.

Change-Id: I59cbf3f75c55e5d3747654edcc9be86ad954cf40
Story: #2002039
Task: #19682
---
 openstackclient/compute/v2/server.py          | 70 +++++++++++++------
 .../tests/unit/compute/v2/test_server.py      | 30 +++++++-
 ...me-lookup-one-by-one-e0f15f4eab329b19.yaml | 14 ++++
 3 files changed, 91 insertions(+), 23 deletions(-)
 create mode 100644 releasenotes/notes/name-lookup-one-by-one-e0f15f4eab329b19.yaml

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 777f7744e7..9247c40e53 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -1044,11 +1044,22 @@ class ListServer(command.Lister):
             default=False,
             help=_('List additional fields in output'),
         )
-        parser.add_argument(
+        name_lookup_group = parser.add_mutually_exclusive_group()
+        name_lookup_group.add_argument(
             '-n', '--no-name-lookup',
             action='store_true',
             default=False,
-            help=_('Skip flavor and image name lookup.'),
+            help=_('Skip flavor and image name lookup.'
+                   'Mutually exclusive with "--name-lookup-one-by-one"'
+                   ' option.'),
+        )
+        name_lookup_group.add_argument(
+            '--name-lookup-one-by-one',
+            action='store_true',
+            default=False,
+            help=_('When looking up flavor and image names, look them up'
+                   'one by one as needed instead of all together (default). '
+                   'Mutually exclusive with "--no-name-lookup|-n" option.'),
         )
         parser.add_argument(
             '--marker',
@@ -1223,28 +1234,43 @@ class ListServer(command.Lister):
                                            limit=parsed_args.limit)
 
         images = {}
-        # Create a dict that maps image_id to image object.
-        # Needed so that we can display the "Image Name" column.
-        # "Image Name" is not crucial, so we swallow any exceptions.
-        if data and not parsed_args.no_name_lookup:
-            try:
-                images_list = self.app.client_manager.image.images.list()
-                for i in images_list:
-                    images[i.id] = i
-            except Exception:
-                pass
-
         flavors = {}
-        # Create a dict that maps flavor_id to flavor object.
-        # Needed so that we can display the "Flavor Name" column.
-        # "Flavor Name" is not crucial, so we swallow any exceptions.
         if data and not parsed_args.no_name_lookup:
-            try:
-                flavors_list = compute_client.flavors.list(is_public=None)
-                for i in flavors_list:
-                    flavors[i.id] = i
-            except Exception:
-                pass
+            # Create a dict that maps image_id to image object.
+            # Needed so that we can display the "Image Name" column.
+            # "Image Name" is not crucial, so we swallow any exceptions.
+            if parsed_args.name_lookup_one_by_one or image_id:
+                for i_id in set(filter(lambda x: x is not None,
+                                       (s.image.get('id') for s in data))):
+                    try:
+                        images[i_id] = image_client.images.get(i_id)
+                    except Exception:
+                        pass
+            else:
+                try:
+                    images_list = image_client.images.list()
+                    for i in images_list:
+                        images[i.id] = i
+                except Exception:
+                    pass
+
+            # Create a dict that maps flavor_id to flavor object.
+            # Needed so that we can display the "Flavor Name" column.
+            # "Flavor Name" is not crucial, so we swallow any exceptions.
+            if parsed_args.name_lookup_one_by_one or flavor_id:
+                for f_id in set(filter(lambda x: x is not None,
+                                       (s.flavor.get('id') for s in data))):
+                    try:
+                        flavors[f_id] = compute_client.flavors.get(f_id)
+                    except Exception:
+                        pass
+            else:
+                try:
+                    flavors_list = compute_client.flavors.list(is_public=None)
+                    for i in flavors_list:
+                        flavors[i.id] = i
+                except Exception:
+                    pass
 
         # Populate image_name, image_id, flavor_name and flavor_id attributes
         # of server objects so that we can display those columns.
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 46d4c24114..6243b49ddf 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -1938,12 +1938,18 @@ class TestServerList(TestServer):
             ('all_projects', False),
             ('long', False),
             ('deleted', False),
+            ('name_lookup_one_by_one', False),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
 
         self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.images_mock.list.assert_called()
+        self.flavors_mock.list.assert_called()
+        # we did not pass image or flavor, so gets on those must be absent
+        self.assertFalse(self.flavors_mock.get.call_count)
+        self.assertFalse(self.images_mock.get.call_count)
         self.assertEqual(self.columns, columns)
         self.assertEqual(tuple(self.data), tuple(data))
 
@@ -2014,6 +2020,28 @@ class TestServerList(TestServer):
         self.assertEqual(self.columns, columns)
         self.assertEqual(tuple(self.data_no_name_lookup), tuple(data))
 
+    def test_server_list_name_lookup_one_by_one(self):
+        arglist = [
+            '--name-lookup-one-by-one'
+        ]
+        verifylist = [
+            ('all_projects', False),
+            ('no_name_lookup', False),
+            ('name_lookup_one_by_one', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+        self.assertFalse(self.images_mock.list.call_count)
+        self.assertFalse(self.flavors_mock.list.call_count)
+        self.images_mock.get.assert_called()
+        self.flavors_mock.get.assert_called()
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
     def test_server_list_with_image(self):
 
         arglist = [
@@ -2046,7 +2074,7 @@ class TestServerList(TestServer):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.flavors_mock.get.assert_called_with(self.flavor.id)
+        self.flavors_mock.get.has_calls(self.flavor.id)
 
         self.search_opts['flavor'] = self.flavor.id
         self.servers_mock.list.assert_called_with(**self.kwargs)
diff --git a/releasenotes/notes/name-lookup-one-by-one-e0f15f4eab329b19.yaml b/releasenotes/notes/name-lookup-one-by-one-e0f15f4eab329b19.yaml
new file mode 100644
index 0000000000..b572b75d5e
--- /dev/null
+++ b/releasenotes/notes/name-lookup-one-by-one-e0f15f4eab329b19.yaml
@@ -0,0 +1,14 @@
+---
+features:
+  - |
+    Add ``--name-lookup-one-by-one`` option to the ``server list`` command
+    that is (mutually exclusive with ``-n | --no-name-lookup``).
+    When provided, the names of images and flavors will be resolved one by one
+    only for those images and flavors that are needed to display the obtained
+    list of servers instead of fetching all the images and flavors.
+    Depending on amount of images in your deployment this can speed up the
+    execution of this command.
+  - |
+    When given ``--image`` or ``--flavor`` argument, the ``server list``
+    command now resolves only single image or flavor instead of fetching
+    all the images or flavors for name lookup purposes.