diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 02fed1550e..b939a270be 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2500,28 +2500,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 895d4b54cb..2e64e071e5 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -4449,8 +4449,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)) @@ -4473,7 +4473,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', ] @@ -4485,6 +4486,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)) @@ -4548,6 +4554,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)) @@ -4576,6 +4584,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)) @@ -4593,8 +4603,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() @@ -4618,6 +4628,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)) @@ -4638,6 +4650,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.