Refactors get_instance_security_groups to only use instance_uuid

Fixes Bug 1160078

Change-Id: Ic4d7ee0202ca15837f9dcab97e5502cbf82aa16f

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 041eb0dc5..bd0293038 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -412,7 +412,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 65142aef0..b0e8397fb 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -3238,11 +3238,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 8a7c6dc48..d54659d0c 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1077,9 +1077,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 adacc6ead..17cf51228 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 1406ed97a..96fddf495 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 4dea52dfb..fbbc1f9bc 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 e5abd1182..69de66fcf 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 60811e65b..0319f8feb 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):