diff --git a/nova/api/openstack/compute/server_groups.py b/nova/api/openstack/compute/server_groups.py index fc65caa8c6..69ad945a25 100644 --- a/nova/api/openstack/compute/server_groups.py +++ b/nova/api/openstack/compute/server_groups.py @@ -16,6 +16,7 @@ """The Server Group API Extension.""" import collections +import itertools from oslo_log import log as logging import webob @@ -87,7 +88,16 @@ def _should_enable_custom_max_server_rules(context, rules): class ServerGroupController(wsgi.Controller): """The Server group API controller for the OpenStack API.""" - def _format_server_group(self, context, group, req): + def _format_server_group(self, context, group, req, + not_deleted_inst_uuids=None): + """Format ServerGroup according to API version. + + Displays only not-deleted members. + + :param:not_deleted_inst_uuids: Pre-built set of instance-uuids for + multiple server-groups that are found to + be not deleted. + """ # the id field has its value as the uuid of the server group # There is no 'uuid' key in server_group seen by clients. # In addition, clients see policies as a ["policy-name"] list; @@ -106,7 +116,12 @@ class ServerGroupController(wsgi.Controller): members = [] if group.members: # Display the instances that are not deleted. - members = _get_not_deleted(context, group.members) + if not_deleted_inst_uuids is not None: + # short-cut if we already pre-built a list of not deleted + # instances to be more efficient + members = list(set(group.members) & not_deleted_inst_uuids) + else: + members = _get_not_deleted(context, group.members) server_group['members'] = members # Add project id information to the response data for # API version v2.13 @@ -181,7 +196,11 @@ class ServerGroupController(wsgi.Controller): sgs = objects.InstanceGroupList.get_by_project_id( context, project_id) limited_list = common.limited(sgs.objects, req) - result = [self._format_server_group(context, group, req) + + all_members = list(itertools.chain.from_iterable( + sg.members for sg in limited_list if sg.members)) + not_deleted = set(_get_not_deleted(context, all_members)) + result = [self._format_server_group(context, group, req, not_deleted) for group in limited_list] return {'server_groups': result} diff --git a/nova/tests/unit/api/openstack/compute/test_server_groups.py b/nova/tests/unit/api/openstack/compute/test_server_groups.py index 5c4c909576..d0a0e82bd9 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_groups.py +++ b/nova/tests/unit/api/openstack/compute/test_server_groups.py @@ -14,6 +14,7 @@ # under the License. import copy +import itertools from unittest import mock from oslo_utils.fixture import uuidsentinel @@ -389,6 +390,37 @@ class ServerGroupTestV21(test.NoDBTestCase): self.assertEqual(2, len(result_members)) self.assertIn(instances[0].uuid, result_members) + def test_display_active_members_only_when_listing(self): + ctx = context.RequestContext('fake_user', fakes.FAKE_PROJECT_ID, + roles=['member', 'reader']) + (ig1_uuid, ig1_instances, ig1_members) = \ + self._create_groups_and_instances(ctx) + (ig2_uuid, ig2_instances, ig2_members) = \ + self._create_groups_and_instances(ctx) + + # delete an instance per ig + for instance in (ig1_instances[1], ig2_instances[0]): + im = objects.InstanceMapping.get_by_instance_uuid( + ctx, instance.uuid) + with context.target_cell(ctx, im.cell_mapping) as cctxt: + instance._context = cctxt + instance.destroy() + # check that the instance does not exist + self.assertRaises(exception.InstanceNotFound, + objects.Instance.get_by_uuid, + ctx, instance.uuid) + + res_dict = self.controller.index(self.reader_req) + result_members = sorted(itertools.chain.from_iterable( + sg['members'] for sg in res_dict['server_groups'])) + + # check that only the active instances are displayed + self.assertEqual(4, len(result_members)) + expected_members = sorted([ + ig1_instances[0].uuid, ig1_instances[2].uuid, + ig2_instances[1].uuid, ig2_instances[2].uuid]) + self.assertEqual(expected_members, result_members) + def test_display_members_rbac_default(self): ctx = context.RequestContext('fake_user', fakes.FAKE_PROJECT_ID) ig_uuid = self._create_groups_and_instances(ctx)[0] diff --git a/releasenotes/notes/bug-2122109-dac145799fc8f58c.yaml b/releasenotes/notes/bug-2122109-dac145799fc8f58c.yaml new file mode 100644 index 0000000000..f69c4f227a --- /dev/null +++ b/releasenotes/notes/bug-2122109-dac145799fc8f58c.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + `Bug #2122109`_: Fix API performance when getting multiple server groups. + We now pre-fetch the not-deleted members of all requested server groups in + a single query per cell instead of executing a query per server group per + cell. + + .. _Bug #2122109: https://launchpad.net/bugs/2122109