Fix /servers/<instance-id>os-security-groups using quantum

When using quantum security groups:
	GET /servers/<instance-id>/os-security-groups
aways returns an empty list. This patch adds a call to
get_instance_security_groups in the api implemenation of security groups.
This method passes both the instance_id and instance_uuid so the api layer
does not need to be aware of which underlying implementation is being used.
The instance_id is needed when using nova and when using quantum the
instance_uuid is needed so both need to be passed.

Fixes bug 1159793

Change-Id: I372adcf93c55d4e6469ef08429ec260d3387f9fe

4 files changed, 63 insertions, 30 deletions

diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index ce6f2687f..615bf9764 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -27,7 +27,6 @@ from nova.api.openstack import wsgi
 from nova.api.openstack import xmlutil
 from nova import compute
 from nova.compute import api as compute_api
-from nova import db
 from nova import exception
 from nova.network.security_group import openstack_driver
 from nova.network.security_group import quantum_driver
@@ -390,8 +389,8 @@ class ServerSecurityGroupController(SecurityGroupControllerBase):
         except exception.InstanceNotFound as exp:
             raise exc.HTTPNotFound(explanation=unicode(exp))
 
-        groups = db.security_group_get_by_instance(context, instance['id'])
-
+        groups = self.security_group_api.get_instance_security_groups(
+            req, instance['id'], instance['uuid'], True)
         result = [self._format_security_group(context, group)
                     for group in groups]
 
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 30b4e73e0..b115772db 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -40,6 +40,7 @@ from nova.compute import task_states
 from nova.compute import utils as compute_utils
 from nova.compute import vm_states
 from nova.consoleauth import rpcapi as consoleauth_rpcapi
+from nova import context
 from nova import crypto
 from nova.db import base
 from nova import exception
@@ -3163,7 +3164,11 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
                 self.security_group_rpcapi.refresh_instance_security_rules(
                         context, instance['host'], instance)
 
-    def get_instance_security_groups(self, req, instance_id):
+    def get_instance_security_groups(self, req, instance_id,
+                                     instance_uuid=None, detailed=False):
+        if detailed:
+            return self.db.security_group_get_by_instance(
+                context.get_admin_context(), instance_id)
         instance = req.get_db_instance(instance_id)
         groups = instance.get('security_groups')
         if groups:
diff --git a/nova/network/security_group/quantum_driver.py b/nova/network/security_group/quantum_driver.py
index 1942d06ef..5a24a74ab 100644
--- a/nova/network/security_group/quantum_driver.py
+++ b/nova/network/security_group/quantum_driver.py
@@ -30,8 +30,6 @@ from nova.network.security_group import security_group_base
 from nova.openstack.common import excutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import uuidutils
-
-
 from nova import utils
 
 
@@ -248,37 +246,47 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
                 raise e
         return self._convert_to_nova_security_group_rule_format(rule)
 
-    def get_instance_security_groups(self, req, instance_id):
-        dict_security_groups = {}
-        security_group_name_map = {}
+    def get_instance_security_groups(self, req, instance_id,
+                                     instance_uuid=None, detailed=False):
+        """Returns the security groups that are associated with an instance.
+        If detailed is True then it also returns the full details of the
+        security groups associated with an instance.
+        """
         admin_context = context.get_admin_context()
-
         quantum = quantumv2.get_client(admin_context)
-        params = {'device_id': instance_id}
+        if instance_uuid:
+            params = {'device_id': instance_uuid}
+        else:
+            params = {'device_id': instance_id}
         ports = quantum.list_ports(**params)
         security_groups = quantum.list_security_groups().get('security_groups')
 
+        security_group_lookup = {}
         for security_group in security_groups:
-            name = security_group.get('name')
-            # Since the name is optional for quantum security groups
-            if not name:
-                name = security_group['id']
-            security_group_name_map[security_group['id']] = name
+            security_group_lookup[security_group['id']] = security_group
 
+        ret = []
         for port in ports['ports']:
             for security_group in port.get('security_groups', []):
                 try:
-                    dict_security_groups[security_group] = (
-                        security_group_name_map[security_group])
+                    if detailed:
+                        ret.append(self._convert_to_nova_security_group_format(
+                            security_group_lookup[security_group]))
+                    else:
+                        name = security_group_lookup[security_group].get(
+                            'name')
+                        # Since the name is optional for
+                        # quantum security groups
+                        if not name:
+                            name = security_group['id']
+                        ret.append({'name': name})
                 except KeyError:
                     # If 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.
+                    # ports were returned. We pass since this security
+                    # group is no longer on the port.
                     pass
-        ret = []
-        for security_group in dict_security_groups.values():
-            ret.append({'name': security_group})
+
         return ret
 
     def _has_security_group_requirements(self, 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 a8eadd2a6..98526bdad 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
@@ -125,10 +125,22 @@ class TestQuantumSecurityGroups(
         pass
 
     def test_get_security_group_by_instance(self):
-        pass
-
-    def test_get_security_group_by_instance_non_existing(self):
-        pass
+        sg = self._create_sg_template().get('security_group')
+        net = self._create_network()
+        self._create_port(
+            network_id=net['network']['id'], security_groups=[sg['id']],
+            device_id=test_security_groups.FAKE_UUID)
+        expected = [{'rules': [], 'tenant_id': 'fake_tenant', 'id': sg['id'],
+                    'name': 'test', 'description': 'test-description'}]
+        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)
+        req = fakes.HTTPRequest.blank('/v2/fake/servers/%s/os-security-groups'
+                                      % test_security_groups.FAKE_UUID)
+        res_dict = self.server_controller.index(
+            req, test_security_groups.FAKE_UUID)['security_groups']
+        self.assertEquals(expected, res_dict)
 
     def test_get_security_group_by_id(self):
         sg = self._create_sg_template().get('security_group')
@@ -508,7 +520,8 @@ class MockClient(object):
         ret = {'status': 'ACTIVE', 'id': str(uuid.uuid4()),
                'mac_address': p.get('mac_address', 'fa:16:3e:b8:f5:fb'),
                'port_security_enabled': p.get('port_security_enabled'),
-               'device_owner': str(uuid.uuid4())}
+               'device_id': p.get('device_id', str(uuid.uuid4())),
+               'security_groups': p.get('security_groups', [])}
 
         fields = ['network_id', 'security_groups', 'admin_state_up']
         for field in fields:
@@ -611,8 +624,16 @@ class MockClient(object):
                 [network for network in self._fake_networks.values()]}
 
     def list_ports(self, **_params):
-        return {'ports':
-                [port for port in self._fake_ports.values()]}
+        ret = []
+        device_id = _params.get('device_id')
+        for port in self._fake_ports.values():
+            if device_id:
+                if device_id == port['device_id']:
+                    print port
+                    ret.append(port)
+            else:
+                ret.append(port)
+        return {'ports': ret}
 
     def list_subnets(self, **_params):
         return {'subnets':