From 26b7b9457a5899ecca93fd67d3879efcad4e4968 Mon Sep 17 00:00:00 2001 From: Alex Meade Date: Wed, 11 Jan 2012 19:43:26 +0000 Subject: [PATCH] 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 --- nova/db/api.py | 18 +++---- nova/db/sqlalchemy/api.py | 24 +++++---- nova/db/sqlalchemy/models.py | 1 - nova/tests/test_compute.py | 100 +++++++++++++++++------------------ nova/tests/test_db_api.py | 2 +- 5 files changed, 73 insertions(+), 72 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index 5f9f9f43e53..81245a285d7 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 83a022292a2..7ade110fdef 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 ad4aa36616b..c0e5e6d34fb 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 f2098de56ea..4f500473664 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -1141,7 +1141,7 @@ def test_pre_live_migration_instance_has_no_fixed_ip(self): 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 @@ def test_pre_live_migration_works_correctly(self): 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 @@ def test_live_migration_dest_raises_exception(self): 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 @@ def test_start(self): 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 @@ def test_stop(self): 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 @@ def test_delete(self): 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 @@ def test_delete_soft(self): 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 @@ def test_suspend(self): 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 @@ def test_resume(self): 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 @@ def test_pause(self): 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 @@ def test_unpause(self): 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 @@ def test_restore(self): 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 @@ def test_rebuild(self): 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 @@ def test_reboot_soft(self): 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 @@ def test_reboot_hard(self): 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 @@ def test_backup_conflict(self): 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 @@ def test_snapshot_conflict(self): 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 @@ def test_get_all_by_name_regexp(self): 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 @@ def test_get_all_by_instance_name_regexp(self): 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 @@ def test_get_all_by_multiple_options_at_once(self): 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 @@ def test_get_all_by_image(self): 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 @@ def test_get_all_by_flavor(self): 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 @@ def test_get_all_by_state(self): 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 @@ def test_get_all_by_metadata(self): 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 @@ def test_instance_metadata(self): 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 @@ def return_fault(_ctxt, instance_uuids): 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 @@ def test_reservation_ids_two_instances(self): 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 @@ def test_create_with_specified_reservation_id(self): 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 @@ def test_create_with_specified_reservation_id(self): 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 @@ def test_instance_name_template(self): 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 @@ def test_inject_file(self): 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 314a9f276eb..524a2811f30 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -92,7 +92,7 @@ def test_instance_get_all_by_filters_deleted(self): 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])