From 129a6a21a6d4201a1af9756a97831d4b30c76162 Mon Sep 17 00:00:00 2001 From: Aaron Lee Date: Thu, 23 Feb 2012 17:45:29 -0600 Subject: [PATCH] bigger-than-unit test for cleanup_running_deleted_instances My team is going to lambaste me for this, but the cleanup_running_deleted_instances was missing a unit test. This lead to a bug in the refactor. This patch fixes both issues; however utils.temporary_mutation makes it hard to write a proper unit test, this one hits the DB. Change-Id: I93a595ffce4f17261f18a52d4c2a11434653a630 --- nova/compute/manager.py | 9 +++++++-- nova/tests/test_compute.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 7eb8d45d6f69..b4b9e3aef4ab 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2347,7 +2347,7 @@ class ComputeManager(manager.SchedulerDependentManager): # NOTE(sirp): admin contexts don't ordinarily return deleted records with utils.temporary_mutation(context, read_deleted="yes"): - for instance in self._errored_instances(context): + for instance in self._running_deleted_instances(context): if action == "log": LOG.warning(_("Detected instance with name label " "'%(name_label)s' which is marked as " @@ -2355,8 +2355,9 @@ class ComputeManager(manager.SchedulerDependentManager): locals(), instance=instance) elif action == 'reap': + name = instance['name'] LOG.info(_("Destroying instance with name label " - "'%(name_label)s' which is marked as " + "'%(name)s' which is marked as " "DELETED but still present on host."), locals(), instance=instance) self._shutdown_instance(context, instance, 'Terminating') @@ -2368,6 +2369,10 @@ class ComputeManager(manager.SchedulerDependentManager): instance=instance) def _running_deleted_instances(self, context): + """Returns a list of instances nova thinks is deleted, + but the hypervisor thinks is still running. This method + should be pushed down to the virt layer for efficiency. + """ def deleted_instance(instance): present = instance.name in present_name_labels erroneously_running = instance.deleted and present diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 7fc3d8bed11e..522f57eb137f 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -1576,6 +1576,35 @@ class ComputeTestCase(BaseTestCase): self.compute.add_instance_fault_from_exc(ctxt, instance_uuid, NotImplementedError('test')) + def test_cleanup_running_deleted_instances(self): + admin_context = context.get_admin_context() + deleted_at = utils.utcnow() - datetime.timedelta(hours=1, minutes=5) + instance = self._create_fake_instance({"deleted_at": deleted_at, + "deleted": True}) + + self.compute.host = instance['host'] + + self.mox.StubOutWithMock(self.compute.driver, 'list_instances') + self.compute.driver.list_instances().AndReturn([instance['name']]) + FLAGS.running_deleted_instance_timeout = 3600 + FLAGS.running_deleted_instance_action = 'reap' + + self.mox.StubOutWithMock(self.compute.db, "instance_get_all_by_host") + self.compute.db.instance_get_all_by_host(admin_context, + self.compute.host + ).AndReturn([instance]) + + self.mox.StubOutWithMock(self.compute, "_shutdown_instance") + self.compute._shutdown_instance(admin_context, instance, + 'Terminating').AndReturn(None) + + self.mox.StubOutWithMock(self.compute, "_cleanup_volumes") + self.compute._cleanup_volumes(admin_context, + instance['id']).AndReturn(None) + + self.mox.ReplayAll() + self.compute._cleanup_running_deleted_instances(admin_context) + def test_running_deleted_instances(self): self.mox.StubOutWithMock(self.compute.driver, 'list_instances') self.compute.driver.list_instances().AndReturn(['herp', 'derp'])