Optimize SecurityGroupsOutputController by len(servers)

The following patch adds an optimization for _extend_servers() so that
it calls get_instance_security_groups() when len(servers) == 1, and
get_instances_security_groups_bindings() when > 1. In addition, this patch
adds a return statement if not len(servers) as there is no reason to
perform a query if there are no servers.

Fixes bug 1182730

Change-Id: I149bf32176d60e72d76576168d15995f7215b657

2 files changed, 33 insertions, 7 deletions

diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index de5a9bd8e..87ef6d882 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -466,6 +466,8 @@ class SecurityGroupsOutputController(wsgi.Controller):
     def _extend_servers(self, req, servers):
         # TODO(arosen) this function should be refactored to reduce duplicate
         # code and use get_instance_security_groups instead of get_db_instance.
+        if not len(servers):
+            return
         key = "security_groups"
         context = _authorize_context(req)
         if not openstack_driver.is_quantum_security_groups():
@@ -480,13 +482,20 @@ 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 = sg_instance_bindings.get(server['id'])
-                    if groups:
-                        server[key] = groups
+                if len(servers) == 1:
+                    group = (self.security_group_api
+                             .get_instance_security_groups(context,
+                                                           servers[0]['id']))
+                    if group:
+                        servers[0][key] = group
+                else:
+                    sg_instance_bindings = (
+                        self.security_group_api
+                        .get_instances_security_groups_bindings(context))
+                    for server in servers:
+                        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
             # one server in an API request.
             else:
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 8eab9224d..ee65f3707 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
@@ -426,6 +426,13 @@ class TestQuantumSecurityGroupsOutputTest(TestQuantumSecurityGroupsTestCase):
         self.assertEquals(group.get('name'), 'default')
 
     def test_show(self):
+        def fake_get_instance_security_groups(inst, context, id):
+            return [{'name': 'fake-2-0'}, {'name': 'fake-2-1'}]
+
+        self.stubs.Set(quantum_driver.SecurityGroupAPI,
+                       'get_instance_security_groups',
+                       fake_get_instance_security_groups)
+
         url = '/v2/fake/servers'
         image_uuid = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
         req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups')
@@ -444,6 +451,16 @@ class TestQuantumSecurityGroupsOutputTest(TestQuantumSecurityGroupsTestCase):
             name = 'fake-2-%s' % i
             self.assertEqual(group.get('name'), name)
 
+        # Test that show (GET) returns the same information as create (POST)
+        url = '/v2/fake/servers/' + test_security_groups.UUID3
+        res = self._make_request(url)
+        self.assertEqual(res.status_int, 200)
+        server = self._get_server(res.body)
+
+        for i, group in enumerate(self._get_groups(server)):
+            name = 'fake-2-%s' % i
+            self.assertEqual(group.get('name'), name)
+
     def test_detail(self):
         url = '/v2/fake/servers/detail'
         res = self._make_request(url)