ec2 _format_security_group() accesses db when using quantum_driver

This patch fixes _format_security_group() so that it does not
access the db when using quantum. In addition, this patch implements
the querying params that were missing in get() and list() in the quantum
security group driver.

Fixes bug 1131956

Change-Id: I5ec65829301bca7e5b215d5052a18528dff76517

4 files changed, 101 insertions, 8 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index dcbde3428..60b06c233 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -503,9 +503,17 @@ class CloudController(object):
             r['groups'] = []
             r['ipRanges'] = []
             if rule['group_id']:
-                source_group = rule['grantee_group']
-                r['groups'] += [{'groupName': source_group['name'],
-                                 'userId': source_group['project_id']}]
+                if rule.get('grantee_group'):
+                    source_group = rule['grantee_group']
+                    r['groups'] += [{'groupName': source_group['name'],
+                                     'userId': source_group['project_id']}]
+                else:
+                    # rule is not always joined with grantee_group
+                    # for example when using quantum driver.
+                    source_group = self.security_group_api.get(
+                        context, id=rule['group_id'])
+                    r['groups'] += [{'groupName': source_group.get('name'),
+                                     'userId': source_group.get('project_id')}]
                 if rule['protocol']:
                     r['ipProtocol'] = rule['protocol'].lower()
                     r['fromPort'] = rule['from_port']
diff --git a/nova/network/security_group/quantum_driver.py b/nova/network/security_group/quantum_driver.py
index 918c839e9..623f2f4ed 100644
--- a/nova/network/security_group/quantum_driver.py
+++ b/nova/network/security_group/quantum_driver.py
@@ -97,6 +97,9 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
     def get(self, context, name=None, id=None, map_exception=False):
         quantum = quantumv2.get_client(context)
         try:
+            if not id and name:
+                id = quantumv20.find_resourceid_by_name_or_id(
+                    quantum, 'security_group', name)
             group = quantum.show_security_group(id).get('security_group')
         except q_exc.QuantumClientException as e:
             if e.status_code == 404:
@@ -113,8 +116,13 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
              search_opts=None):
         """Returns list of security group rules owned by tenant."""
         quantum = quantumv2.get_client(context)
+        search_opts = {}
+        if names:
+            search_opts['name'] = names
+        if ids:
+            search_opts['id'] = ids
         try:
-            security_groups = quantum.list_security_groups().get(
+            security_groups = quantum.list_security_groups(**search_opts).get(
                 'security_groups')
         except q_exc.QuantumClientException as e:
             LOG.exception(_("Quantum Error getting security groups"))
diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index 07780eb02..b8a4712c4 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -41,9 +41,12 @@ from nova import db
 from nova import exception
 from nova.image import s3
 from nova.network import api as network_api
+from nova.network import quantumv2
 from nova.openstack.common import log as logging
 from nova.openstack.common import rpc
 from nova import test
+from nova.tests.api.openstack.compute.contrib import (
+    test_quantum_security_groups as test_quantum)
 from nova.tests import fake_network
 from nova.tests.image import fake
 from nova.tests import matchers
@@ -2226,3 +2229,66 @@ class CloudTestCase(test.TestCase):
         test_dia_iisb('stop', image_id='ami-4')
         test_dia_iisb('stop', image_id='ami-5')
         test_dia_iisb('stop', image_id='ami-6')
+
+
+class CloudTestCaseQuantumProxy(test.TestCase):
+    def setUp(self):
+        cfg.CONF.set_override('security_group_api', 'quantum')
+        self.cloud = cloud.CloudController()
+        self.original_client = quantumv2.get_client
+        quantumv2.get_client = test_quantum.get_client
+        self.user_id = 'fake'
+        self.project_id = 'fake'
+        self.context = context.RequestContext(self.user_id,
+                                              self.project_id,
+                                              is_admin=True)
+        super(CloudTestCaseQuantumProxy, self).setUp()
+
+    def tearDown(self):
+        quantumv2.get_client = self.original_client
+        test_quantum.get_client()._reset()
+        super(CloudTestCaseQuantumProxy, self).tearDown()
+
+    def test_describe_security_groups(self):
+        # Makes sure describe_security_groups works and filters results.
+        group_name = 'test'
+        description = 'test'
+        self.cloud.create_security_group(self.context, group_name,
+                                         description)
+        result = self.cloud.describe_security_groups(self.context)
+        # NOTE(vish): should have the default group as well
+        self.assertEqual(len(result['securityGroupInfo']), 2)
+        result = self.cloud.describe_security_groups(self.context,
+                      group_name=[group_name])
+        self.assertEqual(len(result['securityGroupInfo']), 1)
+        self.assertEqual(result['securityGroupInfo'][0]['groupName'],
+                         group_name)
+        self.cloud.delete_security_group(self.context, group_name)
+
+    def test_describe_security_groups_by_id(self):
+        group_name = 'test'
+        description = 'test'
+        self.cloud.create_security_group(self.context, group_name,
+                                         description)
+        quantum = test_quantum.get_client()
+        # Get id from quantum since cloud.create_security_group
+        # does not expose it.
+        search_opts = {'name': group_name}
+        groups = quantum.list_security_groups(
+            **search_opts)['security_groups']
+        result = self.cloud.describe_security_groups(self.context,
+                      group_id=[groups[0]['id']])
+        self.assertEqual(len(result['securityGroupInfo']), 1)
+        self.assertEqual(
+                result['securityGroupInfo'][0]['groupName'],
+                group_name)
+        self.cloud.delete_security_group(self.context, group_name)
+
+    def test_create_delete_security_group(self):
+        descript = 'test description'
+        create = self.cloud.create_security_group
+        result = create(self.context, 'testgrp', descript)
+        group_descript = result['securityGroupSet'][0]['groupDescription']
+        self.assertEqual(descript, group_descript)
+        delete = self.cloud.delete_security_group
+        self.assertTrue(delete(self.context, 'testgrp'))
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 e32fadbb8..5f9c5cefa 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
@@ -569,10 +569,21 @@ class MockClient(object):
     def list_security_groups(self, **_params):
         ret = []
         for security_group in self._fake_security_groups.values():
-            if _params.get('name'):
-                if security_group.get('name') == _params['name']:
-                    ret.append(security_group)
-            else:
+            names = _params.get('name')
+            if names:
+                if not isinstance(names, list):
+                    names = [names]
+                for name in names:
+                    if security_group.get('name') == name:
+                        ret.append(security_group)
+            ids = _params.get('id')
+            if ids:
+                if not isinstance(ids, list):
+                    ids = [ids]
+                for id in ids:
+                    if security_group.get('id') == id:
+                        ret.append(security_group)
+            elif not (names or ids):
                 ret.append(security_group)
         return {'security_groups': ret}