Query quantum once for instance's security groups

This patch adds the method get_instance_security_group_bindings()
which returns all the instance_id's and security groups for a given
tenant. This method is then used to avoid querying quantum for it's
security groups when using GET /v2/<project_id>/servers/detail.

Fixes bug 1161518

Change-Id: Ia28d5573c264f0316ab9fb257ca8e7ae35679883

4 files changed, 82 insertions, 24 deletions

diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index c019011f5..700d9b71e 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -482,10 +482,11 @@ class SecurityGroupsOutputController(wsgi.Controller):
             # quantum security groups the requested security groups for the
             # instance are not in the db and have not been sent to quantum yet.
             if req.method != 'POST':
+                sg_instance_bindings = (
+                    self.security_group_api
+                    .get_instances_security_groups_bindings(context))
                 for server in servers:
-                    groups = (
-                        self.security_group_api.get_instance_security_groups(
-                            context, server['id']))
+                    groups = sg_instance_bindings.get(server['id'])
                     if groups:
                         server[key] = groups
             # In this section of code len(servers) == 1 as you can only POST
diff --git a/nova/network/security_group/quantum_driver.py b/nova/network/security_group/quantum_driver.py
index d8eede44f..aecafe8d1 100644
--- a/nova/network/security_group/quantum_driver.py
+++ b/nova/network/security_group/quantum_driver.py
@@ -245,6 +245,36 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
                 raise e
         return self._convert_to_nova_security_group_rule_format(rule)
 
+    def get_instances_security_groups_bindings(self, context):
+        """Returns a dict(instance_id, [security_groups]) to allow obtaining
+        all of the instances and their security groups in one shot."""
+        quantum = quantumv2.get_client(context)
+        ports = quantum.list_ports().get('ports')
+        security_groups = quantum.list_security_groups().get('security_groups')
+        security_group_lookup = {}
+        instances_security_group_bindings = {}
+        for security_group in security_groups:
+            security_group_lookup[security_group['id']] = security_group
+
+        for port in ports:
+            for port_security_group in port.get('security_groups', []):
+                try:
+                    sg = security_group_lookup[port_security_group]
+                    # name is optional in quantum so if not specified return id
+                    if sg.get('name'):
+                        sg_entry = {'name': sg['name']}
+                    else:
+                        sg_entry = {'name': sg['id']}
+                    instances_security_group_bindings.setdefault(
+                        port['device_id'], []).append(sg_entry)
+                except KeyError:
+                    # This should only happen due to a race condition
+                    # if the security group on a port was deleted after the
+                    # ports were returned. We pass since this security
+                    # group is no longer on the port.
+                    pass
+        return instances_security_group_bindings
+
     def get_instance_security_groups(self, context, instance_id,
                                      instance_uuid=None, detailed=False):
         """Returns the security groups that are associated with an instance.
@@ -279,7 +309,7 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
                             name = security_group['id']
                         ret.append({'name': name})
                 except KeyError:
-                    # If this should only happen due to a race condition
+                    # This should only happen due to a race condition
                     # if the security group on a port was deleted after the
                     # ports were returned. We pass since this security
                     # group is no longer on the port.
diff --git a/nova/tests/api/openstack/compute/contrib/test_quantum_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_quantum_security_groups.py
index fb0df975a..94bc36b1f 100644
--- a/nova/tests/api/openstack/compute/contrib/test_quantum_security_groups.py
+++ b/nova/tests/api/openstack/compute/contrib/test_quantum_security_groups.py
@@ -130,7 +130,7 @@ class TestQuantumSecurityGroups(
         net = self._create_network()
         self._create_port(
             network_id=net['network']['id'], security_groups=[sg['id']],
-            device_id=test_security_groups.FAKE_UUID)
+            device_id=test_security_groups.FAKE_UUID1)
         expected = [{'rules': [], 'tenant_id': 'fake_tenant', 'id': sg['id'],
                     'name': 'test', 'description': 'test-description'}]
         self.stubs.Set(nova.db, 'instance_get',
@@ -138,9 +138,9 @@ class TestQuantumSecurityGroups(
         self.stubs.Set(nova.db, 'instance_get_by_uuid',
                        test_security_groups.return_server_by_uuid)
         req = fakes.HTTPRequest.blank('/v2/fake/servers/%s/os-security-groups'
-                                      % test_security_groups.FAKE_UUID)
+                                      % test_security_groups.FAKE_UUID1)
         res_dict = self.server_controller.index(
-            req, test_security_groups.FAKE_UUID)['security_groups']
+            req, test_security_groups.FAKE_UUID1)['security_groups']
         self.assertEquals(expected, res_dict)
 
     def test_get_security_group_by_id(self):
@@ -192,7 +192,7 @@ class TestQuantumSecurityGroups(
         net = self._create_network()
         self._create_port(
             network_id=net['network']['id'], security_groups=[sg['id']],
-            device_id=test_security_groups.FAKE_UUID)
+            device_id=test_security_groups.FAKE_UUID1)
 
         self.stubs.Set(nova.db, 'instance_get',
                        test_security_groups.return_server)
@@ -229,7 +229,7 @@ class TestQuantumSecurityGroups(
         net = self._create_network()
         self._create_port(
             network_id=net['network']['id'], security_groups=[sg['id']],
-            device_id=test_security_groups.FAKE_UUID)
+            device_id=test_security_groups.FAKE_UUID1)
 
         self.stubs.Set(nova.db, 'instance_get',
                        test_security_groups.return_server)
@@ -240,6 +240,34 @@ class TestQuantumSecurityGroups(
         req = fakes.HTTPRequest.blank('/v2/fake/servers/1/action')
         self.manager._removeSecurityGroup(req, '1', body)
 
+    def test_get_instances_security_groups_bindings(self):
+        sg1 = self._create_sg_template(name='test1').get('security_group')
+        sg2 = self._create_sg_template(name='test2').get('security_group')
+        # test name='' is replaced with id
+        sg3 = self._create_sg_template(name='').get('security_group')
+        net = self._create_network()
+        self._create_port(
+            network_id=net['network']['id'], security_groups=[sg1['id'],
+                                                              sg2['id']],
+            device_id=test_security_groups.FAKE_UUID1)
+        self._create_port(
+            network_id=net['network']['id'], security_groups=[sg2['id'],
+                                                              sg3['id']],
+            device_id=test_security_groups.FAKE_UUID2)
+        expected = {test_security_groups.FAKE_UUID1: [{'name': sg1['name']},
+                                                      {'name': sg2['name']}],
+                    test_security_groups.FAKE_UUID2: [{'name': sg2['name']},
+                                                      {'name': sg3['id']}]}
+        self.stubs.Set(nova.db, 'instance_get',
+                       test_security_groups.return_server)
+        self.stubs.Set(nova.db, 'instance_get_by_uuid',
+                       test_security_groups.return_server_by_uuid)
+        security_group_api = self.controller.security_group_api
+        bindings = (
+            security_group_api.get_instances_security_groups_bindings(
+                context.get_admin_context()))
+        self.assertEquals(bindings, expected)
+
 
 class TestQuantumSecurityGroupRulesTestCase(TestQuantumSecurityGroupsTestCase):
     def setUp(self):
@@ -329,8 +357,9 @@ class TestQuantumSecurityGroupsOutputTest(TestQuantumSecurityGroupsTestCase):
         self.stubs.Set(compute.api.API, 'create',
                        test_security_groups.fake_compute_create)
         self.stubs.Set(quantum_driver.SecurityGroupAPI,
-                       'get_instance_security_groups',
-                       test_security_groups.fake_get_instance_security_groups)
+                       'get_instances_security_groups_bindings',
+                       (test_security_groups.
+                       fake_get_instances_security_groups_bindings))
         self.flags(
             osapi_compute_extension=[
                 'nova.api.openstack.compute.contrib.select_extensions'],
diff --git a/nova/tests/api/openstack/compute/contrib/test_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
index aa33f1729..480156b97 100644
--- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py
+++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
@@ -34,7 +34,8 @@ from nova.tests.api.openstack import fakes
 from nova.tests import utils
 
 CONF = cfg.CONF
-FAKE_UUID = 'a47ae74e-ab08-447f-8eee-ffd43fc46c16'
+FAKE_UUID1 = 'a47ae74e-ab08-447f-8eee-ffd43fc46c16'
+FAKE_UUID2 = 'c6e6430a-6563-4efa-9542-5e93c9e97d18'
 
 
 class AttrDict(dict):
@@ -81,7 +82,7 @@ def return_server(context, server_id):
     return {'id': int(server_id),
             'power_state': 0x01,
             'host': "localhost",
-            'uuid': FAKE_UUID,
+            'uuid': FAKE_UUID1,
             'name': 'asdf'}
 
 
@@ -95,12 +96,12 @@ def return_server_by_uuid(context, server_uuid):
 
 def return_non_running_server(context, server_id):
     return {'id': server_id, 'power_state': power_state.SHUTDOWN,
-            'uuid': FAKE_UUID, 'host': "localhost", 'name': 'asdf'}
+            'uuid': FAKE_UUID1, 'host': "localhost", 'name': 'asdf'}
 
 
 def return_security_group_by_name(context, project_id, group_name):
     return {'id': 1, 'name': group_name,
-            "instances": [{'id': 1, 'uuid': FAKE_UUID}]}
+            "instances": [{'id': 1, 'uuid': FAKE_UUID1}]}
 
 
 def return_security_group_without_instances(context, project_id, group_name):
@@ -347,7 +348,7 @@ class TestSecurityGroups(test.TestCase):
         expected = {'security_groups': groups}
 
         def return_instance(context, server_id):
-            self.assertEquals(server_id, FAKE_UUID)
+            self.assertEquals(server_id, FAKE_UUID1)
             return return_server_by_uuid(context, server_id)
 
         self.stubs.Set(nova.db, 'instance_get_by_uuid',
@@ -361,8 +362,8 @@ class TestSecurityGroups(test.TestCase):
                        return_security_groups)
 
         req = fakes.HTTPRequest.blank('/v2/%s/servers/%s/os-security-groups' %
-                                      ('fake', FAKE_UUID))
-        res_dict = self.server_controller.index(req, FAKE_UUID)
+                                      ('fake', FAKE_UUID1))
+        res_dict = self.server_controller.index(req, FAKE_UUID1)
 
         self.assertEquals(res_dict, expected)
 
@@ -1388,12 +1389,9 @@ def fake_compute_create(*args, **kwargs):
     return ([fake_compute_get()], '')
 
 
-def fake_get_instance_security_groups(inst, context, instance_id):
-    if instance_id == UUID1:
-        return [{'name': 'fake-0-0'}, {'name': 'fake-0-1'}]
-
-    elif instance_id == UUID2:
-        return [{'name': 'fake-1-0'}, {'name': 'fake-1-1'}]
+def fake_get_instances_security_groups_bindings(inst, context):
+    return {UUID1: [{'name': 'fake-0-0'}, {'name': 'fake-0-1'}],
+            UUID2: [{'name': 'fake-1-0'}, {'name': 'fake-1-1'}]}
 
 
 class SecurityGroupsOutputTest(test.TestCase):