From 17bca438954638d74035d560e826a26a532b3ea3 Mon Sep 17 00:00:00 2001 From: Joe Gordon Date: Wed, 13 Mar 2013 16:58:13 -0700 Subject: [PATCH] Delete InstanceSystemMetadata on instance deletion Thanks to no-db-compute only pre-delete instance data is used in notifications. So now we can go back to deleting InstanceSystemMetadata when an instance is deleted. Fixes bug 1153827 Change-Id: Ic66b998cde2a15a24f302f08c34753a8b57da73d --- nova/db/sqlalchemy/api.py | 3 +++ nova/tests/compute/test_compute_utils.py | 28 +++++------------------- nova/tests/test_db_api.py | 7 ++++-- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 2b278119ab72..a7b73ff14de7 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1522,6 +1522,9 @@ def instance_destroy(context, instance_uuid, constraint=None): session.query(models.InstanceMetadata).\ filter_by(instance_uuid=instance_uuid).\ soft_delete() + session.query(models.InstanceSystemMetadata).\ + filter_by(instance_uuid=instance_uuid).\ + soft_delete() return instance_ref diff --git a/nova/tests/compute/test_compute_utils.py b/nova/tests/compute/test_compute_utils.py index 52888c11288c..05d91075cb14 100644 --- a/nova/tests/compute/test_compute_utils.py +++ b/nova/tests/compute/test_compute_utils.py @@ -309,8 +309,9 @@ class UsageInfoTestCase(test.TestCase): self.assertEquals(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance) - def test_notify_usage_exists_deleted_instance(self): - # Ensure 'exists' notification generates appropriate usage data. + def test_notify_usage_exists_fail_on_deleted_instance(self): + # notify_usage_exists should not work for a deleted VM. A + # notification should be done before the instance is deleted in the db. instance_id = self._create_instance() instance = db.instance_get(self.context, instance_id) # Set some system metadata @@ -322,27 +323,8 @@ class UsageInfoTestCase(test.TestCase): self.compute.terminate_instance(self.context, instance) instance = db.instance_get(self.context.elevated(read_deleted='yes'), instance_id) - compute_utils.notify_usage_exists(self.context, instance) - msg = test_notifier.NOTIFICATIONS[-1] - self.assertEquals(msg['priority'], 'INFO') - self.assertEquals(msg['event_type'], 'compute.instance.exists') - payload = msg['payload'] - self.assertEquals(payload['tenant_id'], self.project_id) - self.assertEquals(payload['user_id'], self.user_id) - self.assertEquals(payload['instance_id'], instance['uuid']) - self.assertEquals(payload['instance_type'], 'm1.tiny') - type_id = instance_types.get_instance_type_by_name('m1.tiny')['id'] - self.assertEquals(str(payload['instance_type_id']), str(type_id)) - for attr in ('display_name', 'created_at', 'launched_at', - 'state', 'state_description', - 'bandwidth', 'audit_period_beginning', - 'audit_period_ending', 'image_meta'): - self.assertTrue(attr in payload, - msg="Key %s not in payload" % attr) - self.assertEquals(payload['image_meta'], - {'md_key1': 'val1', 'md_key2': 'val2'}) - image_ref_url = "%s/images/1" % glance.generate_glance_url() - self.assertEquals(payload['image_ref_url'], image_ref_url) + self.assertRaises(KeyError, compute_utils.notify_usage_exists, + self.context, instance) def test_notify_usage_exists_instance_not_found(self): # Ensure 'exists' notification generates appropriate usage data. diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py index 6dc3287bfb09..cab93fc325fa 100644 --- a/nova/tests/test_db_api.py +++ b/nova/tests/test_db_api.py @@ -470,7 +470,7 @@ class DbApiTestCase(DbTestCase): system_meta = db.instance_system_metadata_get(ctxt, instance['uuid']) self.assertEqual('baz', system_meta['original_image_ref']) - def test_delete_instance_metadata_on_instance_destroy(self): + def test_delete_instance_and_system_metadata_on_instance_destroy(self): ctxt = context.get_admin_context() # Create an instance with some metadata @@ -482,8 +482,11 @@ class DbApiTestCase(DbTestCase): self.assertEqual('meow', instance_meta['key1']) db.instance_destroy(ctxt, instance['uuid']) instance_meta = db.instance_metadata_get(ctxt, instance['uuid']) - # Make sure instance metadata is deleted as well + instance_system_meta = db.instance_system_metadata_get(ctxt, + instance['uuid']) + # Make sure instance and system metadata is deleted as well self.assertEqual({}, instance_meta) + self.assertEqual({}, instance_system_meta) def test_instance_update_unique_name(self): otherprojectcontext = context.RequestContext(self.user_id,