From 725b7de13cf00da386132a42b2738f4c57026184 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 12 Apr 2022 20:16:59 +0100 Subject: [PATCH] compute: Only retrieve necessary images The Glance API allows us to filter by multiple IDs using the 'in:' operator. Take advantage of this to speed up listing of server in larger deployments where image counts in the hundreds (or even thousands) are not uncommon. Unfortunately the Nova API does not support something similar for listing flavors. Boo. Change-Id: I7d3222d0b0b8bf72b4ff3e429bc49e621b569979 Signed-off-by: Stephen Finucane Depends-On: https://review.opendev.org/c/openstack/openstacksdk/+/837613 --- openstackclient/compute/v2/server.py | 31 ++++++++++++++----- .../tests/unit/compute/v2/test_server.py | 24 +++++++++++--- ...list-restrict-images-c0b2c4de6f93df33.yaml | 8 +++++ 3 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/server-list-restrict-images-c0b2c4de6f93df33.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 69aaa3c57b..803887359c 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2479,28 +2479,45 @@ class ListServer(command.Lister): data = compute_client.servers.list( search_opts=search_opts, marker=marker_id, - limit=parsed_args.limit) + limit=parsed_args.limit, + ) images = {} flavors = {} if data and not parsed_args.no_name_lookup: + # partial responses from down cells will not have an image + # attribute so we use getattr + image_ids = { + s.image['id'] for s in data + if getattr(s, 'image', None) and s.image.get('id') + } + # create a dict that maps image_id to image object, which is used # to display the "Image Name" column. Note that 'image.id' can be # empty for BFV instances and 'image' can be missing entirely if # there are infra failures if parsed_args.name_lookup_one_by_one or image_id: - for i_id in set( - s.image['id'] for s in data - if s.image and s.image.get('id') - ): + for image_id in image_ids: # "Image Name" is not crucial, so we swallow any exceptions try: - images[i_id] = image_client.get_image(i_id) + images[image_id] = image_client.get_image(image_id) except Exception: pass else: try: - images_list = image_client.images() + # some deployments can have *loads* of images so we only + # want to list the ones we care about. It would be better + # to only retrun the *fields* we care about (name) but + # glance doesn't support that + # NOTE(stephenfin): This could result in super long URLs + # but it seems unlikely to cause issues. Apache supports + # URL lengths of up to 8190 characters by default, which + # should allow for more than 220 unique image ID (different + # servers are likely use the same image ID) in the filter. + # Who'd need more than that in a single command? + images_list = image_client.images( + id=f"in:{','.join(image_ids)}" + ) for i in images_list: images[i.id] = i except Exception: diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 46ace57903..207daf4b3c 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -4441,8 +4441,8 @@ class TestServerList(_TestServerList): columns, data = self.cmd.take_action(parsed_args) self.servers_mock.list.assert_called_with(**self.kwargs) - self.assertEqual(0, self.images_mock.list.call_count) - self.assertEqual(0, self.flavors_mock.list.call_count) + self.images_mock.assert_not_called() + self.flavors_mock.list.assert_not_called() self.assertEqual(self.columns, columns) self.assertEqual(self.data, tuple(data)) @@ -4465,7 +4465,8 @@ class TestServerList(_TestServerList): getattr(s, 'OS-EXT-AZ:availability_zone'), getattr(s, 'OS-EXT-SRV-ATTR:host'), s.Metadata, - ) for s in self.servers) + ) for s in self.servers + ) arglist = [ '--long', ] @@ -4477,6 +4478,11 @@ class TestServerList(_TestServerList): columns, data = self.cmd.take_action(parsed_args) self.servers_mock.list.assert_called_with(**self.kwargs) + image_ids = {s.image['id'] for s in self.servers if s.image} + self.images_mock.assert_called_once_with( + id=f'in:{",".join(image_ids)}', + ) + self.flavors_mock.list.assert_called_once_with(is_public=None) self.assertEqual(self.columns_long, columns) self.assertEqual(self.data, tuple(data)) @@ -4526,6 +4532,8 @@ class TestServerList(_TestServerList): columns, data = self.cmd.take_action(parsed_args) self.servers_mock.list.assert_called_with(**self.kwargs) + self.images_mock.assert_not_called() + self.flavors_mock.list.assert_not_called() self.assertEqual(self.columns, columns) self.assertEqual(self.data, tuple(data)) @@ -4554,6 +4562,8 @@ class TestServerList(_TestServerList): columns, data = self.cmd.take_action(parsed_args) self.servers_mock.list.assert_called_with(**self.kwargs) + self.images_mock.assert_not_called() + self.flavors_mock.list.assert_not_called() self.assertEqual(self.columns, columns) self.assertEqual(self.data, tuple(data)) @@ -4571,8 +4581,8 @@ class TestServerList(_TestServerList): 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.assert_not_called() + self.flavors_mock.list.assert_not_called() self.get_image_mock.assert_called() self.flavors_mock.get.assert_called() @@ -4596,6 +4606,8 @@ class TestServerList(_TestServerList): self.search_opts['image'] = self.image.id self.servers_mock.list.assert_called_with(**self.kwargs) + self.images_mock.assert_not_called() + self.flavors_mock.list.assert_called_once() self.assertEqual(self.columns, columns) self.assertEqual(self.data, tuple(data)) @@ -4616,6 +4628,8 @@ class TestServerList(_TestServerList): self.search_opts['flavor'] = self.flavor.id self.servers_mock.list.assert_called_with(**self.kwargs) + self.images_mock.assert_called_once() + self.flavors_mock.list.assert_not_called() self.assertEqual(self.columns, columns) self.assertEqual(self.data, tuple(data)) diff --git a/releasenotes/notes/server-list-restrict-images-c0b2c4de6f93df33.yaml b/releasenotes/notes/server-list-restrict-images-c0b2c4de6f93df33.yaml new file mode 100644 index 0000000000..2ee2e4d87b --- /dev/null +++ b/releasenotes/notes/server-list-restrict-images-c0b2c4de6f93df33.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + The ``server list`` needs to query the image service API to retrieve + image names as part of the response. This command will now retrieve only + the images that are relevant, i.e. those used by the server included in + the output. This should result in signficantly faster responses when + using a deployment with a large number of public images.