Merge "Refactors get_instance_security_groups to only use instance_uuid"

13 files changed, 27 insertions, 28 deletions

diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index 40b085be1..b34a77cab 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -420,7 +420,7 @@ class ServerSecurityGroupController(SecurityGroupControllerBase):
             raise exc.HTTPNotFound(explanation=exp.format_message())
 
         groups = self.security_group_api.get_instance_security_groups(
-            context, instance['id'], instance['uuid'], True)
+            context, 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 145a0ee27..75a438fa6 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -3237,11 +3237,12 @@ 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, context, instance_id,
-                                     instance_uuid=None, detailed=False):
+    def get_instance_security_groups(self, context, instance_uuid,
+                                     detailed=False):
         if detailed:
-            return self.db.security_group_get_by_instance(context, instance_id)
-        instance = self.db.instance_get(context, instance_id)
+            return self.db.security_group_get_by_instance(context,
+                                                          instance_uuid)
+        instance = self.db.instance_get_by_uuid(context, instance_uuid)
         groups = instance.get('security_groups')
         if groups:
             return [{'name': group['name']} for group in groups]
diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index 6eccaf341..d6e718ffb 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -239,7 +239,7 @@ class ConductorManager(manager.Manager):
 
     def security_group_get_by_instance(self, context, instance):
         group = self.db.security_group_get_by_instance(context,
-                                                       instance['id'])
+                                                       instance['uuid'])
         return jsonutils.to_primitive(group)
 
     def security_group_rule_get_by_security_group(self, context, secgroup):
diff --git a/nova/db/api.py b/nova/db/api.py
index 7ecf6dfc0..a0451dcc7 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1167,9 +1167,9 @@ def security_group_get_by_project(context, project_id):
     return IMPL.security_group_get_by_project(context, project_id)
 
 
-def security_group_get_by_instance(context, instance_id):
+def security_group_get_by_instance(context, instance_uuid):
     """Get security groups to which the instance is assigned."""
-    return IMPL.security_group_get_by_instance(context, instance_id)
+    return IMPL.security_group_get_by_instance(context, instance_uuid)
 
 
 def security_group_exists(context, project_id, group_name):
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index d27f4e695..e0807b64a 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -3304,10 +3304,10 @@ def security_group_get_by_project(context, project_id):
 
 
 @require_context
-def security_group_get_by_instance(context, instance_id):
+def security_group_get_by_instance(context, instance_uuid):
     return _security_group_get_query(context, read_deleted="no").\
                    join(models.SecurityGroup.instances).\
-                   filter_by(id=instance_id).\
+                   filter_by(uuid=instance_uuid).\
                    all()
 
 
diff --git a/nova/network/security_group/quantum_driver.py b/nova/network/security_group/quantum_driver.py
index cb5f6551f..fee897c42 100644
--- a/nova/network/security_group/quantum_driver.py
+++ b/nova/network/security_group/quantum_driver.py
@@ -293,17 +293,14 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
                     pass
         return instances_security_group_bindings
 
-    def get_instance_security_groups(self, context, instance_id,
-                                     instance_uuid=None, detailed=False):
+    def get_instance_security_groups(self, context, instance_uuid,
+                                     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.
         """
         quantum = quantumv2.get_client(context)
-        if instance_uuid:
-            params = {'device_id': instance_uuid}
-        else:
-            params = {'device_id': instance_id}
+        params = {'device_id': instance_uuid}
         ports = quantum.list_ports(**params)
         security_groups = quantum.list_security_groups().get('security_groups')
 
diff --git a/nova/network/security_group/security_group_base.py b/nova/network/security_group/security_group_base.py
index 484927c84..f802b955e 100644
--- a/nova/network/security_group/security_group_base.py
+++ b/nova/network/security_group/security_group_base.py
@@ -208,8 +208,8 @@ class SecurityGroupBase(object):
     def get_rule(self, context, id):
         raise NotImplementedError()
 
-    def get_instance_security_groups(self, context, instance_id,
-                                     instance_uuid=None, detailed=False):
+    def get_instance_security_groups(self, context, instance_uuid,
+                                     detailed=False):
         raise NotImplementedError()
 
     def add_to_instance(self, context, instance, security_group_name):
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 6bd220198..f1433bd0a 100644
--- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py
+++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
@@ -354,8 +354,8 @@ class TestSecurityGroups(test.TestCase):
         self.stubs.Set(nova.db, 'instance_get_by_uuid',
                        return_instance)
 
-        def return_security_groups(context, instance_id):
-            self.assertEquals(instance_id, 1)
+        def return_security_groups(context, instance_uuid):
+            self.assertEquals(instance_uuid, FAKE_UUID1)
             return [security_group_db(sg) for sg in groups]
 
         self.stubs.Set(nova.db, 'security_group_get_by_instance',
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index b1f046c52..3ff317c32 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -5266,7 +5266,7 @@ class ComputeAPITestCase(BaseTestCase):
                 security_group=['testgroup'])
         try:
             self.assertEqual(len(db.security_group_get_by_instance(
-                             self.context, ref[0]['id'])), 1)
+                             self.context, ref[0]['uuid'])), 1)
             group = db.security_group_get(self.context, group['id'])
             self.assert_(len(group['instances']) == 1)
         finally:
diff --git a/nova/tests/compute/test_virtapi.py b/nova/tests/compute/test_virtapi.py
index e3887d033..7cf05d87d 100644
--- a/nova/tests/compute/test_virtapi.py
+++ b/nova/tests/compute/test_virtapi.py
@@ -64,7 +64,7 @@ class VirtAPIBaseTest(test.TestCase, test.APICoverage):
 
     def test_security_group_get_by_instance(self):
         self.assertExpected('security_group_get_by_instance',
-                            {'id': 'fake-id'})
+                            {'uuid': 'fake-id'})
 
     def test_security_group_rule_get_by_security_group(self):
         self.assertExpected('security_group_rule_get_by_security_group',
@@ -99,11 +99,12 @@ class FakeVirtAPITest(VirtAPIBaseTest):
         self.mox.StubOutWithMock(db, db_method)
 
         if method in ('aggregate_metadata_add', 'aggregate_metadata_delete',
-                      'security_group_rule_get_by_security_group',
-                      'security_group_get_by_instance'):
+                      'security_group_rule_get_by_security_group'):
             # NOTE(danms): FakeVirtAPI will convert the first argument to
             # argument['id'], so expect that in the actual db call
             e_args = tuple([args[0]['id']] + list(args[1:]))
+        elif method in ('test_security_group_get_by_instance'):
+            e_args = tuple([args[0]['uuid']] + list(args[1:]))
         else:
             e_args = args
 
diff --git a/nova/tests/conductor/test_conductor.py b/nova/tests/conductor/test_conductor.py
index 03896ee3a..beb0d3b61 100644
--- a/nova/tests/conductor/test_conductor.py
+++ b/nova/tests/conductor/test_conductor.py
@@ -274,10 +274,10 @@ class _BaseTestCase(object):
         self.assertEqual(result, 'foo')
 
     def test_security_group_get_by_instance(self):
-        fake_instance = {'id': 'fake-instance'}
+        fake_instance = {'uuid': 'fake-instance'}
         self.mox.StubOutWithMock(db, 'security_group_get_by_instance')
         db.security_group_get_by_instance(
-            self.context, fake_instance['id']).AndReturn('it worked')
+            self.context, fake_instance['uuid']).AndReturn('it worked')
         self.mox.ReplayAll()
         result = self.conductor.security_group_get_by_instance(self.context,
                                                                fake_instance)
diff --git a/nova/tests/db/test_db_api.py b/nova/tests/db/test_db_api.py
index 711f81e52..94a151c21 100644
--- a/nova/tests/db/test_db_api.py
+++ b/nova/tests/db/test_db_api.py
@@ -2159,7 +2159,7 @@ class SecurityGroupTestCase(test.TestCase, ModelsObjectComparatorMixin):
                            for vals in values]
 
         real = db.security_group_get_by_instance(self.ctxt,
-                                                 instance['id'])
+                                                 instance['uuid'])
         expected = security_groups[:2]
         self._assertEqualListsOfObjects(expected, real,
                                         ignored_keys=['instances'])
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index 1b65571a5..7973c1b1d 100755
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -462,7 +462,7 @@ class FakeVirtAPI(virtapi.VirtAPI):
         return db.aggregate_metadata_delete(context, aggregate['id'], key)
 
     def security_group_get_by_instance(self, context, instance):
-        return db.security_group_get_by_instance(context, instance['id'])
+        return db.security_group_get_by_instance(context, instance['uuid'])
 
     def security_group_rule_get_by_security_group(self, context,
                                                   security_group):