Fix server_group_members quota check

For example there are 3 instances in a server group (quota is 5).
When doing multi-creating of 3 more instances in this group
(would have 6 members), current quota checking scheme will fail to
prevent this happening, which is not expected.

This is due to the server_group_members quota check previously
only counting group members that existed as instance records in
cell databases and not accounting for build requests which are
the temporary representation of the instance in the API database
before the instance is scheduled to a cell.

Co-Authored-By: Matt Riedemann <mriedem.os@gmail.com>

Change-Id: If439f4486b8fe157c436c47aa408608e639a3e15
Closes-Bug: #1780373
(cherry picked from commit bbee9a26a5)

nova/quota.py

@@ -1287,7 +1287,20 @@ def _server_group_count_members_by_user(context, group, user_id):
     for greenthread in greenthreads:
         found = greenthread.wait()
         instances = instances + found
-    return {'user': {'server_group_members': len(instances)}}
+    # Count build requests using the same filters to catch group members
+    # that are not yet creatd in a cell.
+    # NOTE(mriedem): BuildRequestList.get_by_filters is not very efficient for
+    # what we need and we can optimize this with a new query method.
+    build_requests = objects.BuildRequestList.get_by_filters(context, filters)
+    # Ignore any duplicates since build requests and instances can co-exist
+    # for a short window of time after the instance is created in a cell but
+    # before the build request is deleted.
+    instance_uuids = [inst.uuid for inst in instances]
+    count = len(instances)
+    for build_request in build_requests:
+        if build_request.instance_uuid not in instance_uuids:
+            count += 1
+    return {'user': {'server_group_members': count}}
 
 
 def _fixed_ip_count(context, project_id):

nova/tests/functional/regressions/test_bug_1780373.py

@@ -15,7 +15,6 @@ from nova.tests import fixtures as nova_fixtures
 from nova.tests.functional import integrated_helpers
 from nova.tests.unit.image import fake as fake_image
 from nova.tests.unit import policy_fixture
-from nova.virt import fake as fake_virt
 
 
 class TestMultiCreateServerGroupMemberOverQuota(
@@ -59,14 +58,6 @@ class TestMultiCreateServerGroupMemberOverQuota(
         server group and then create 3 servers in the group using a
         multi-create POST /servers request.
         """
-        # TODO(mriedem): We don't need a compute service when the bug is fixed
-        # because we won't be able to get past nova-api validation.
-        self.start_service('conductor')
-        self.start_service('scheduler')
-        fake_virt.set_nodes(['host1'])
-        self.addCleanup(fake_virt.restore_nodes)
-        self.start_service('compute', host='host1')
-
         server_req = self._build_minimal_create_server_request(
             self.api, 'test_multi_create_server_group_members_over_quota',
             image_uuid=fake_image.AUTO_DISK_CONFIG_ENABLED_IMAGE_UUID,
@@ -74,20 +65,15 @@ class TestMultiCreateServerGroupMemberOverQuota(
         server_req['min_count'] = 3
         server_req['return_reservation_id'] = True
         hints = {'group': self.created_group['id']}
-        # FIXME(mriedem): When bug 1780373 is fixed this should result in a
-        # 403 error response and 0 members in the group.
-        reservation_id = self.api.post_server(
-            {'server': server_req,
-             'os:scheduler_hints': hints})['reservation_id']
-        # Assert that three servers were created regardless of the
-        # [quota]/server_group_members=2 quota limit.
-        servers = self.api.get_servers(
-            detail=False, search_opts={'reservation_id': reservation_id})
-        self.assertEqual(3, len(servers))
+        # We should get a 403 response due to going over quota on server
+        # group members in a single request.
+        self.api.api_post(
+            '/servers', {'server': server_req, 'os:scheduler_hints': hints},
+            check_response_status=[403])
         group = self.api.api_get(
             '/os-server-groups/%s' %
             self.created_group['id']).body['server_group']
-        self.assertEqual(3, len(group['members']))
+        self.assertEqual(0, len(group['members']))
 
     def test_concurrent_request_server_group_members_over_quota(self):
         """Recreate scenario for the bug where we create 3 servers in the
@@ -105,17 +91,20 @@ class TestMultiCreateServerGroupMemberOverQuota(
                 image_uuid=fake_image.AUTO_DISK_CONFIG_ENABLED_IMAGE_UUID,
                 networks='none')
             hints = {'group': self.created_group['id']}
-            # FIXME(mriedem): When bug 1780373 is fixed this should result in a
-            # 403 error response on the 3rd create server request.
-            self.api.post_server(
-                {'server': server_req, 'os:scheduler_hints': hints})
-        # Assert that three servers were created regardless of the
-        # [quota]/server_group_members=2 quota limit.
+            # This should result in a 403 response on the 3rd server.
+            if x == 2:
+                self.api.api_post(
+                    '/servers',
+                    {'server': server_req, 'os:scheduler_hints': hints},
+                    check_response_status=[403])
+            else:
+                self.api.post_server(
+                    {'server': server_req, 'os:scheduler_hints': hints})
+        # There should only be two servers created which are both members of
+        # the same group.
         servers = self.api.get_servers(detail=False)
-        # FIXME(mriedem): When the bug is fixed, there should only be 2 servers
-        # created and 2 members in the group.
-        self.assertEqual(3, len(servers))
+        self.assertEqual(2, len(servers))
         group = self.api.api_get(
             '/os-server-groups/%s' %
             self.created_group['id']).body['server_group']
-        self.assertEqual(3, len(group['members']))
+        self.assertEqual(2, len(group['members']))