List instances for secgroup without joining on rules

Make db.security_group_get only join rules if specified in
the columns_to_join. This works around a performance issue
with lots of instances and security groups.

Co-Authored-By: Dan Smith <dansmith@redhat.com>
Change-Id: Ie3daed133419c41ed22646f9a790570ff47f0eec
Closes-Bug: #1552971

nova/compute/api.py

@@ -4099,13 +4099,16 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
 
     def get(self, context, name=None, id=None, map_exception=False):
         self.ensure_default(context)
+        cols = ['rules']
         try:
             if name:
                 return self.db.security_group_get_by_name(context,
                                                           context.project_id,
-                                                          name)
+                                                          name,
+                                                          columns_to_join=cols)
             elif id:
-                return self.db.security_group_get(context, id)
+                return self.db.security_group_get(context, id,
+                                                  columns_to_join=cols)
         except exception.NotFound as exp:
             if map_exception:
                 msg = exp.format_message()

nova/db/sqlalchemy/api.py

@@ -4301,7 +4301,11 @@ def security_group_get_all(context):
 @require_context
 @main_context_manager.reader
 def security_group_get(context, security_group_id, columns_to_join=None):
-    query = _security_group_get_query(context, project_only=True).\
+    join_rules = columns_to_join and 'rules' in columns_to_join
+    if join_rules:
+        columns_to_join.remove('rules')
+    query = _security_group_get_query(context, project_only=True,
+                                      join_rules=join_rules).\
                     filter_by(id=security_group_id)
 
     if columns_to_join is None:

nova/tests/unit/api/openstack/compute/test_security_groups.py

@@ -114,7 +114,8 @@ def return_non_running_server(context, server_id, columns_to_join=None):
            'uuid': server_id, 'host': "localhost", 'name': 'asdf'})
 
 
-def return_security_group_by_name(context, project_id, group_name):
+def return_security_group_by_name(context, project_id, group_name,
+                                  columns_to_join=None):
     return {'id': 1, 'name': group_name,
             "instances": [{'id': 1, 'uuid': UUID_SERVER}]}
 
@@ -473,7 +474,7 @@ class TestSecurityGroupsV21(test.TestCase):
     def test_get_security_group_by_id(self):
         sg = security_group_template(id=2, rules=[])
 
-        def return_security_group(context, group_id):
+        def return_security_group(context, group_id, columns_to_join=None):
             self.assertEqual(sg['id'], group_id)
             return security_group_db(sg)
 
@@ -498,7 +499,7 @@ class TestSecurityGroupsV21(test.TestCase):
         sg_update = security_group_template(id=2, rules=[],
                         name='update_name', description='update_desc')
 
-        def return_security_group(context, group_id):
+        def return_security_group(context, group_id, columns_to_join=None):
             self.assertEqual(sg['id'], group_id)
             return security_group_db(sg)
 
@@ -523,7 +524,7 @@ class TestSecurityGroupsV21(test.TestCase):
     def test_update_security_group_name_to_default(self):
         sg = security_group_template(id=2, rules=[], name='default')
 
-        def return_security_group(context, group_id):
+        def return_security_group(context, group_id, columns_to_join=None):
             self.assertEqual(sg['id'], group_id)
             return security_group_db(sg)
 
@@ -548,7 +549,7 @@ class TestSecurityGroupsV21(test.TestCase):
         def security_group_destroy(context, id):
             self.called = True
 
-        def return_security_group(context, group_id):
+        def return_security_group(context, group_id, columns_to_join=None):
             self.assertEqual(sg['id'], group_id)
             return security_group_db(sg)
 
@@ -592,7 +593,7 @@ class TestSecurityGroupsV21(test.TestCase):
         def security_group_in_use(context, id):
             return True
 
-        def return_security_group(context, group_id):
+        def return_security_group(context, group_id, columns_to_join=None):
             self.assertEqual(sg['id'], group_id)
             return security_group_db(sg)
 

nova/tests/unit/db/test_db_api.py

@@ -2004,14 +2004,16 @@ class SecurityGroupTestCase(test.TestCase, ModelsObjectComparatorMixin):
                           self.ctxt, security_group1['id'])
         self._assertEqualObjects(db.security_group_get(
                 self.ctxt, security_group2['id'],
-                columns_to_join=['instances']), security_group2)
+                columns_to_join=['instances',
+                                 'rules']), security_group2)
 
     def test_security_group_get(self):
         security_group1 = self._create_security_group({})
         self._create_security_group({'name': 'fake_sec_group2'})
         real_security_group = db.security_group_get(self.ctxt,
                                               security_group1['id'],
-                                              columns_to_join=['instances'])
+                                              columns_to_join=['instances',
+                                                               'rules'])
         self._assertEqualObjects(security_group1,
                                  real_security_group)