Call to instance_info_cache_delete to use uuid

Fixes bug 903497
Also updated incorrect calls to instance_destroy that were using uuids.

Change-Id: I25eead020ceb7ebf7234c268543ad77d8ecf1185

5 files changed, 73 insertions, 72 deletions

diff --git a/nova/db/api.py b/nova/db/api.py
index 5f9f9f43e..81245a285 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -654,32 +654,32 @@ def instance_info_cache_create(context, values):
     return IMPL.instance_info_cache_create(context, values)
 
 
-def instance_info_cache_get(context, instance_id, session=None):
+def instance_info_cache_get(context, instance_uuid, session=None):
     """Gets an instance info cache from the table.
 
-    :param instance_id: = id of the info cache's instance
+    :param instance_uuid: = uuid of the info cache's instance
     :param session: = optional session object
     """
-    return IMPL.instance_info_cache_get(context, instance_id, session=None)
+    return IMPL.instance_info_cache_get(context, instance_uuid, session=None)
 
 
-def instance_info_cache_update(context, instance_id, values,
+def instance_info_cache_update(context, instance_uuid, values,
                                session=None):
     """Update an instance info cache record in the table.
 
-    :param instance_id: = id of info cache's instance
+    :param instance_uuid: = uuid of info cache's instance
     :param values: = dict containing column values to update
     """
-    return IMPL.instance_info_cache_update(context, instance_id, values,
+    return IMPL.instance_info_cache_update(context, instance_uuid, values,
                                            session)
 
 
-def instance_info_cache_delete(context, instance_id, session=None):
+def instance_info_cache_delete(context, instance_uuid, session=None):
     """Deletes an existing instance_info_cache record
 
-    :param instance_id: = id of the instance tied to the cache record
+    :param instance_uuid: = uuid of the instance tied to the cache record
     """
-    return IMPL.instance_info_cache_delete(context, instance_id, session)
+    return IMPL.instance_info_cache_delete(context, instance_uuid, session)
 
 
 ###################
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 83a022292..7ade110fd 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -1128,6 +1128,7 @@ def instance_data_get_for_project(context, project_id):
 def instance_destroy(context, instance_id):
     session = get_session()
     with session.begin():
+        instance_ref = instance_get(context, instance_id, session=session)
         session.query(models.Instance).\
                 filter_by(id=instance_id).\
                 update({'deleted': True,
@@ -1148,7 +1149,9 @@ def instance_destroy(context, instance_id):
                 update({'deleted': True,
                         'deleted_at': utils.utcnow(),
                         'updated_at': literal_column('updated_at')})
-        instance_info_cache_delete(context, instance_id, session=session)
+
+        instance_info_cache_delete(context, instance_ref['uuid'],
+                                   session=session)
 
 
 @require_context
@@ -1426,7 +1429,6 @@ def instance_get_project_vpn(context, project_id):
 def instance_get_fixed_addresses(context, instance_id):
     session = get_session()
     with session.begin():
-        instance_ref = instance_get(context, instance_id, session=session)
         try:
             fixed_ips = fixed_ip_get_by_instance(context, instance_id)
         except exception.NotFound:
@@ -1590,31 +1592,31 @@ def instance_info_cache_create(context, values):
 
 
 @require_context
-def instance_info_cache_get(context, instance_id, session=None):
+def instance_info_cache_get(context, instance_uuid, session=None):
     """Gets an instance info cache from the table.
 
-    :param instance_id: = uuid of the info cache's instance
+    :param instance_uuid: = uuid of the info cache's instance
     :param session: = optional session object
     """
     session = session or get_session()
 
     info_cache = session.query(models.InstanceInfoCache).\
-                         filter_by(instance_id=instance_id).\
+                         filter_by(instance_id=instance_uuid).\
                          first()
     return info_cache
 
 
 @require_context
-def instance_info_cache_update(context, instance_id, values,
+def instance_info_cache_update(context, instance_uuid, values,
                                session=None):
     """Update an instance info cache record in the table.
 
-    :param instance_id: = uuid of info cache's instance
+    :param instance_uuid: = uuid of info cache's instance
     :param values: = dict containing column values to update
     :param session: = optional session object
     """
     session = session or get_session()
-    info_cache = instance_info_cache_get(context, instance_id,
+    info_cache = instance_info_cache_get(context, instance_uuid,
                                          session=session)
 
     values['updated_at'] = literal_column('updated_at')
@@ -1626,15 +1628,15 @@ def instance_info_cache_update(context, instance_id, values,
 
 
 @require_context
-def instance_info_cache_delete(context, instance_id, session=None):
+def instance_info_cache_delete(context, instance_uuid, session=None):
     """Deletes an existing instance_info_cache record
 
-    :param instance_id: = uuid of the instance tied to the cache record
+    :param instance_uuid: = uuid of the instance tied to the cache record
     :param session: = optional session object
     """
     values = {'deleted': True,
               'deleted_at': utils.utcnow()}
-    instance_info_cache_update(context, instance_id, values, session)
+    instance_info_cache_update(context, instance_uuid, values, session)
 
 
 ###################
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index ad4aa3661..c0e5e6d34 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -268,7 +268,6 @@ class InstanceInfoCache(BASE, NovaBase):
     # text column used for storing a json object of network data for api
     network_info = Column(Text)
 
-    # this is all uuid based, we have them might as well start using them
     instance_id = Column(String(36), ForeignKey('instances.uuid'),
                                      nullable=False, unique=True)
     instance = relationship(Instance,
diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py
index f2098de56..4f5004736 100644
--- a/nova/tests/test_compute.py
+++ b/nova/tests/test_compute.py
@@ -1141,7 +1141,7 @@ class ComputeTestCase(BaseTestCase):
                           self.compute.pre_live_migration,
                           c, inst_ref['id'], time=FakeTime())
         # cleanup
-        db.instance_destroy(c, inst_ref['uuid'])
+        db.instance_destroy(c, inst_ref['id'])
 
     def test_pre_live_migration_works_correctly(self):
         """Confirm setup_compute_volume is called when volume is mounted."""
@@ -1170,7 +1170,7 @@ class ComputeTestCase(BaseTestCase):
         self.assertEqual(ret, None)
 
         # cleanup
-        db.instance_destroy(c, inst_ref['uuid'])
+        db.instance_destroy(c, inst_ref['id'])
 
     def test_live_migration_dest_raises_exception(self):
         """Confirm exception when pre_live_migration fails."""
@@ -1213,7 +1213,7 @@ class ComputeTestCase(BaseTestCase):
                                                                 instance_id):
             db.block_device_mapping_destroy(c, bdms['id'])
         db.volume_destroy(c, volume_id)
-        db.instance_destroy(c, inst_ref['uuid'])
+        db.instance_destroy(c, inst_ref['id'])
 
     def test_live_migration_works_correctly(self):
         """Confirm live_migration() works as expected correctly."""
@@ -1537,7 +1537,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.STARTING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_stop(self):
         instance = self._create_fake_instance()
@@ -1552,7 +1552,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.STOPPING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_delete(self):
         instance = self._create_fake_instance()
@@ -1567,7 +1567,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.DELETING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_delete_soft(self):
         instance = self._create_fake_instance()
@@ -1582,7 +1582,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.POWERING_OFF)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_force_delete(self):
         """Ensure instance can be deleted after a soft delete"""
@@ -1614,7 +1614,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.SUSPENDING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_resume(self):
         """Ensure instance can be resumed (if suspended)"""
@@ -1633,7 +1633,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.RESUMING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_pause(self):
         """Ensure instance can be paused"""
@@ -1648,7 +1648,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.PAUSING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_unpause(self):
         """Ensure instance can be unpaused"""
@@ -1669,7 +1669,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.UNPAUSING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_restore(self):
         """Ensure instance can be restored from a soft delete"""
@@ -1688,7 +1688,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['task_state'], task_states.POWERING_ON)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_rebuild(self):
         inst_ref = self._create_fake_instance()
@@ -1705,7 +1705,7 @@ class ComputeAPITestCase(BaseTestCase):
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(instance['vm_state'], vm_states.REBUILDING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_reboot_soft(self):
         """Ensure instance can be soft rebooted"""
@@ -1722,7 +1722,7 @@ class ComputeAPITestCase(BaseTestCase):
         inst_ref = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(inst_ref['task_state'], task_states.REBOOTING)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, inst_ref['id'])
 
     def test_reboot_hard(self):
         """Ensure instance can be hard rebooted"""
@@ -1739,7 +1739,7 @@ class ComputeAPITestCase(BaseTestCase):
         inst_ref = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(inst_ref['task_state'], task_states.REBOOTING_HARD)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, inst_ref['id'])
 
     def test_hostname_create(self):
         """Ensure instance hostname is set during creation."""
@@ -1966,7 +1966,7 @@ class ComputeAPITestCase(BaseTestCase):
                           None,
                           None)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_snapshot_conflict(self):
         """Can't snapshot an instance which is already being snapshotted."""
@@ -1982,7 +1982,7 @@ class ComputeAPITestCase(BaseTestCase):
                           instance,
                           None)
 
-        db.instance_destroy(self.context, instance_uuid)
+        db.instance_destroy(self.context, instance['id'])
 
     def test_resize_confirm_through_api(self):
         instance = self._create_fake_instance()
@@ -2221,9 +2221,9 @@ class ComputeAPITestCase(BaseTestCase):
                 search_opts={'name': 'noth.*'})
         self.assertEqual(len(instances), 0)
 
-        db.instance_destroy(c, instance1['uuid'])
-        db.instance_destroy(c, instance2['uuid'])
-        db.instance_destroy(c, instance3['uuid'])
+        db.instance_destroy(c, instance1['id'])
+        db.instance_destroy(c, instance2['id'])
+        db.instance_destroy(c, instance3['id'])
 
     def test_get_all_by_instance_name_regexp(self):
         """Test searching instances by name"""
@@ -2250,9 +2250,9 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertEqual(len(instances), 1)
         self.assertEqual(instances[0]['uuid'], instance2['uuid'])
 
-        db.instance_destroy(c, instance1['uuid'])
-        db.instance_destroy(c, instance2['uuid'])
-        db.instance_destroy(c, instance3['uuid'])
+        db.instance_destroy(c, instance1['id'])
+        db.instance_destroy(c, instance2['id'])
+        db.instance_destroy(c, instance3['id'])
 
     def test_get_all_by_multiple_options_at_once(self):
         """Test searching by multiple options at once"""
@@ -2303,9 +2303,9 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertEqual(len(instances), 1)
         self.assertEqual(instances[0]['uuid'], instance3['uuid'])
 
-        db.instance_destroy(c, instance1['uuid'])
-        db.instance_destroy(c, instance2['uuid'])
-        db.instance_destroy(c, instance3['uuid'])
+        db.instance_destroy(c, instance1['id'])
+        db.instance_destroy(c, instance2['id'])
+        db.instance_destroy(c, instance3['id'])
 
     def test_get_all_by_image(self):
         """Test searching instances by image"""
@@ -2333,9 +2333,9 @@ class ComputeAPITestCase(BaseTestCase):
                                     search_opts={'image': ['1234', '4567']})
         self.assertEqual(len(instances), 3)
 
-        db.instance_destroy(c, instance1['uuid'])
-        db.instance_destroy(c, instance2['uuid'])
-        db.instance_destroy(c, instance3['uuid'])
+        db.instance_destroy(c, instance1['id'])
+        db.instance_destroy(c, instance2['id'])
+        db.instance_destroy(c, instance3['id'])
 
     def test_get_all_by_flavor(self):
         """Test searching instances by image"""
@@ -2372,9 +2372,9 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertTrue(instance2['uuid'] in instance_uuids)
         self.assertTrue(instance3['uuid'] in instance_uuids)
 
-        db.instance_destroy(c, instance1['uuid'])
-        db.instance_destroy(c, instance2['uuid'])
-        db.instance_destroy(c, instance3['uuid'])
+        db.instance_destroy(c, instance1['id'])
+        db.instance_destroy(c, instance2['id'])
+        db.instance_destroy(c, instance3['id'])
 
     def test_get_all_by_state(self):
         """Test searching instances by state"""
@@ -2412,9 +2412,9 @@ class ComputeAPITestCase(BaseTestCase):
                         power_state.RUNNING]})
         self.assertEqual(len(instances), 3)
 
-        db.instance_destroy(c, instance1['uuid'])
-        db.instance_destroy(c, instance2['uuid'])
-        db.instance_destroy(c, instance3['uuid'])
+        db.instance_destroy(c, instance1['id'])
+        db.instance_destroy(c, instance2['id'])
+        db.instance_destroy(c, instance3['id'])
 
     def test_get_all_by_metadata(self):
         """Test searching instances by metadata"""
@@ -2473,11 +2473,11 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertEqual(len(instances), 1)
         self.assertEqual(instances[0]['uuid'], instance4['uuid'])
 
-        db.instance_destroy(c, instance0['uuid'])
-        db.instance_destroy(c, instance1['uuid'])
-        db.instance_destroy(c, instance2['uuid'])
-        db.instance_destroy(c, instance3['uuid'])
-        db.instance_destroy(c, instance4['uuid'])
+        db.instance_destroy(c, instance0['id'])
+        db.instance_destroy(c, instance1['id'])
+        db.instance_destroy(c, instance2['id'])
+        db.instance_destroy(c, instance3['id'])
+        db.instance_destroy(c, instance4['id'])
 
     def test_instance_metadata(self):
         """Test searching instances by state"""
@@ -2502,7 +2502,7 @@ class ComputeAPITestCase(BaseTestCase):
         metadata = self.compute_api.get_instance_metadata(_context, instance)
         self.assertEqual(metadata, {'key3': 'value3'})
 
-        db.instance_destroy(_context, instance['uuid'])
+        db.instance_destroy(_context, instance['id'])
 
     def test_get_instance_faults(self):
         """Get an instances latest fault"""
@@ -2528,7 +2528,7 @@ class ComputeAPITestCase(BaseTestCase):
         expected = {instance['uuid']: [fault_fixture]}
         self.assertEqual(output, expected)
 
-        db.instance_destroy(_context, instance['uuid'])
+        db.instance_destroy(_context, instance['id'])
 
     @staticmethod
     def _parse_db_block_device_mapping(bdm_ref):
@@ -2687,7 +2687,8 @@ class ComputeAPITestCase(BaseTestCase):
         finally:
             for instance in refs:
                 self.assertEqual(instance['reservation_id'], resv_id)
-                db.instance_destroy(self.context, instance['id'])
+
+        db.instance_destroy(self.context, refs[0]['id'])
 
     def test_create_with_specified_reservation_id(self):
         """Verify building instances with a specified
@@ -2707,7 +2708,6 @@ class ComputeAPITestCase(BaseTestCase):
             self.assertEqual(resv_id, 'meow')
         finally:
             self.assertEqual(refs[0]['reservation_id'], resv_id)
-            db.instance_destroy(self.context, refs[0]['id'])
 
         # 2 instances
         (refs, resv_id) = self.compute_api.create(context,
@@ -2719,7 +2719,7 @@ class ComputeAPITestCase(BaseTestCase):
         finally:
             for instance in refs:
                 self.assertEqual(instance['reservation_id'], resv_id)
-                db.instance_destroy(self.context, instance['id'])
+            db.instance_destroy(self.context, refs[0]['id'])
 
     def test_instance_name_template(self):
         """Test the instance_name template"""
@@ -2727,24 +2727,24 @@ class ComputeAPITestCase(BaseTestCase):
         i_ref = self._create_fake_instance()
         instance_id = i_ref['id']
         self.assertEqual(i_ref['name'], 'instance-%d' % i_ref['id'])
-        db.instance_destroy(self.context, i_ref['uuid'])
+        db.instance_destroy(self.context, i_ref['id'])
 
         self.flags(instance_name_template='instance-%(uuid)s')
         i_ref = self._create_fake_instance()
         self.assertEqual(i_ref['name'], 'instance-%s' % i_ref['uuid'])
-        db.instance_destroy(self.context, i_ref['uuid'])
+        db.instance_destroy(self.context, i_ref['id'])
 
         self.flags(instance_name_template='%(id)d-%(uuid)s')
         i_ref = self._create_fake_instance()
         self.assertEqual(i_ref['name'], '%d-%s' %
                 (i_ref['id'], i_ref['uuid']))
-        db.instance_destroy(self.context, i_ref['uuid'])
+        db.instance_destroy(self.context, i_ref['id'])
 
         # not allowed.. default is uuid
         self.flags(instance_name_template='%(name)s')
         i_ref = self._create_fake_instance()
         self.assertEqual(i_ref['name'], i_ref['uuid'])
-        db.instance_destroy(self.context, i_ref['uuid'])
+        db.instance_destroy(self.context, i_ref['id'])
 
     def test_add_remove_fixed_ip(self):
         instance = self._create_fake_instance()
@@ -2905,4 +2905,4 @@ class ComputeAPITestCase(BaseTestCase):
         instance = self._create_fake_instance()
         self.compute_api.inject_file(self.context, instance,
                                      "/tmp/test", "File Contents")
-        db.instance_destroy(self.context, instance['uuid'])
+        db.instance_destroy(self.context, instance['id'])
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 314a9f276..524a2811f 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -92,7 +92,7 @@ class DbApiTestCase(test.TestCase):
         inst1 = db.instance_create(self.context, args1)
         args2 = {'reservation_id': 'b', 'image_ref': 1, 'host': 'host1'}
         inst2 = db.instance_create(self.context, args2)
-        db.instance_destroy(self.context, inst1.id)
+        db.instance_destroy(self.context.elevated(), inst1['id'])
         result = db.instance_get_all_by_filters(self.context.elevated(), {})
         self.assertEqual(2, len(result))
         self.assertIn(inst1.id, [result[0].id, result[1].id])