From 57154884c47143bfd6101d4b758d5e8d45966622 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Wed, 30 Jan 2013 18:22:45 -0500 Subject: [PATCH] Make scheduler modules pass conductor to add_instance_fault The add_instance_fault_from_exc() method was recently changed to take a conductor to avoid direct database access. The scheduler was not updated for this, and thus was not passing it in a couple of cases. This makes those calls pass a conductor LocalAPI, resulting in direct database access (which is desired from the scheduler). The tests that one might have thought would catch this didn't because they mock out the method itself. This fixes those and adds two tests that exercise the add_instance_fault path all the way down to the DB API, which would have caught it in the first place. Fixes bug 1110808 Change-Id: If1c2988487d408a39fdf4080541f58f6bdac216c --- nova/scheduler/driver.py | 2 + nova/scheduler/manager.py | 2 + nova/tests/scheduler/test_chance_scheduler.py | 4 +- nova/tests/scheduler/test_filter_scheduler.py | 7 ++- nova/tests/scheduler/test_scheduler.py | 48 +++++++++++++++++-- 5 files changed, 57 insertions(+), 6 deletions(-) diff --git a/nova/scheduler/driver.py b/nova/scheduler/driver.py index 16714a5ffd6..01bef4185dc 100644 --- a/nova/scheduler/driver.py +++ b/nova/scheduler/driver.py @@ -27,6 +27,7 @@ from nova.compute import rpcapi as compute_rpcapi from nova.compute import utils as compute_utils from nova.compute import vm_states +from nova.conductor import api as conductor_api from nova import db from nova import exception from nova import notifications @@ -66,6 +67,7 @@ def handle_schedule_error(context, ex, instance_uuid, request_spec): notifications.send_update(context, old_ref, new_ref, service="scheduler") compute_utils.add_instance_fault_from_exc(context, + conductor_api.LocalAPI(), new_ref, ex, sys.exc_info()) properties = request_spec.get('instance_properties', {}) diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py index 23e64cd7cb4..e6bf1a29381 100644 --- a/nova/scheduler/manager.py +++ b/nova/scheduler/manager.py @@ -26,6 +26,7 @@ from nova.compute import rpcapi as compute_rpcapi from nova.compute import utils as compute_utils from nova.compute import vm_states +from nova.conductor import api as conductor_api import nova.context from nova import db from nova import exception @@ -190,6 +191,7 @@ def _set_vm_state_and_notify(self, method, updates, context, ex, notifications.send_update(context, old_ref, new_ref, service="scheduler") compute_utils.add_instance_fault_from_exc(context, + conductor_api.LocalAPI(), new_ref, ex, sys.exc_info()) payload = dict(request_spec=request_spec, diff --git a/nova/tests/scheduler/test_chance_scheduler.py b/nova/tests/scheduler/test_chance_scheduler.py index 76fba900d17..dcbe86f75b4 100644 --- a/nova/tests/scheduler/test_chance_scheduler.py +++ b/nova/tests/scheduler/test_chance_scheduler.py @@ -25,6 +25,7 @@ from nova.compute import rpcapi as compute_rpcapi from nova.compute import utils as compute_utils from nova.compute import vm_states +from nova.conductor import api as conductor_api from nova import context from nova import db from nova import exception @@ -134,7 +135,8 @@ def test_basic_schedule_run_instance_no_hosts(self): {'vm_state': vm_states.ERROR, 'task_state': None}).AndReturn(({}, {})) compute_utils.add_instance_fault_from_exc(ctxt, - new_ref, mox.IsA(exception.NoValidHost), mox.IgnoreArg()) + mox.IsA(conductor_api.LocalAPI), new_ref, + mox.IsA(exception.NoValidHost), mox.IgnoreArg()) self.mox.ReplayAll() self.driver.schedule_run_instance( diff --git a/nova/tests/scheduler/test_filter_scheduler.py b/nova/tests/scheduler/test_filter_scheduler.py index 2bd2cb85b46..ff3a00f22fb 100644 --- a/nova/tests/scheduler/test_filter_scheduler.py +++ b/nova/tests/scheduler/test_filter_scheduler.py @@ -21,6 +21,7 @@ from nova.compute import instance_types from nova.compute import utils as compute_utils from nova.compute import vm_states +from nova.conductor import api as conductor_api from nova import context from nova import db from nova import exception @@ -62,7 +63,8 @@ def _fake_empty_call_zone_method(*args, **kwargs): uuid, {'vm_state': vm_states.ERROR, 'task_state': None}).AndReturn(({}, {})) compute_utils.add_instance_fault_from_exc(fake_context, - new_ref, mox.IsA(exception.NoValidHost), mox.IgnoreArg()) + mox.IsA(conductor_api.LocalAPI), new_ref, + mox.IsA(exception.NoValidHost), mox.IgnoreArg()) self.mox.ReplayAll() sched.schedule_run_instance( fake_context, request_spec, None, None, None, None, {}) @@ -92,7 +94,8 @@ def fake_get(context, *args, **kwargs): uuid, {'vm_state': vm_states.ERROR, 'task_state': None}).AndReturn(({}, {})) compute_utils.add_instance_fault_from_exc(fake_context, - new_ref, mox.IsA(exception.NoValidHost), mox.IgnoreArg()) + mox.IsA(conductor_api.LocalAPI), new_ref, + mox.IsA(exception.NoValidHost), mox.IgnoreArg()) self.mox.ReplayAll() sched.schedule_run_instance( fake_context, request_spec, None, None, None, None, {}) diff --git a/nova/tests/scheduler/test_scheduler.py b/nova/tests/scheduler/test_scheduler.py index eb4c3864f27..142d8ea0e81 100644 --- a/nova/tests/scheduler/test_scheduler.py +++ b/nova/tests/scheduler/test_scheduler.py @@ -26,10 +26,12 @@ from nova.compute import rpcapi as compute_rpcapi from nova.compute import utils as compute_utils from nova.compute import vm_states +from nova.conductor import api as conductor_api from nova import context from nova import db from nova import exception from nova.openstack.common import jsonutils +from nova.openstack.common.notifier import api as notifier from nova.openstack.common import rpc from nova.scheduler import driver from nova.scheduler import manager @@ -187,7 +189,8 @@ def test_run_instance_exception_puts_instance_in_error_state(self): fake_instance_uuid, {"vm_state": vm_states.ERROR, "task_state": None}).AndReturn((inst, inst)) - compute_utils.add_instance_fault_from_exc(self.context, new_ref, + compute_utils.add_instance_fault_from_exc(self.context, + mox.IsA(conductor_api.LocalAPI), new_ref, mox.IsA(exception.NoValidHost), mox.IgnoreArg()) self.mox.ReplayAll() @@ -221,7 +224,8 @@ def test_prep_resize_no_valid_host_back_in_active_state(self): fake_instance_uuid, {"vm_state": vm_states.ACTIVE, "task_state": None}).AndReturn( (inst, inst)) - compute_utils.add_instance_fault_from_exc(self.context, new_ref, + compute_utils.add_instance_fault_from_exc(self.context, + mox.IsA(conductor_api.LocalAPI), new_ref, mox.IsA(exception.NoValidHost), mox.IgnoreArg()) self.mox.ReplayAll() @@ -258,7 +262,8 @@ def test_prep_resize_exception_host_in_error_state_and_raise(self): fake_instance_uuid, {"vm_state": vm_states.ERROR, "task_state": None}).AndReturn((inst, inst)) - compute_utils.add_instance_fault_from_exc(self.context, new_ref, + compute_utils.add_instance_fault_from_exc(self.context, + mox.IsA(conductor_api.LocalAPI), new_ref, mox.IsA(test.TestingException), mox.IgnoreArg()) self.mox.ReplayAll() @@ -266,6 +271,25 @@ def test_prep_resize_exception_host_in_error_state_and_raise(self): self.assertRaises(test.TestingException, self.manager.prep_resize, **kwargs) + def test_set_vm_state_and_notify_adds_instance_fault(self): + request = {'instance_properties': {'uuid': 'fake-uuid'}} + updates = {'vm_state': 'foo'} + fake_inst = {'uuid': 'fake-uuid'} + + self.mox.StubOutWithMock(db, 'instance_update_and_get_original') + self.mox.StubOutWithMock(db, 'instance_fault_create') + self.mox.StubOutWithMock(notifier, 'notify') + db.instance_update_and_get_original(self.context, 'fake-uuid', + updates).AndReturn((None, + fake_inst)) + db.instance_fault_create(self.context, mox.IgnoreArg()) + notifier.notify(self.context, mox.IgnoreArg(), 'scheduler.foo', + notifier.ERROR, mox.IgnoreArg()) + self.mox.ReplayAll() + + self.manager._set_vm_state_and_notify('foo', {'vm_state': 'foo'}, + self.context, None, request) + class SchedulerTestCase(test.TestCase): """Test case for base scheduler driver class.""" @@ -620,6 +644,24 @@ def test_live_migration_dest_hypervisor_version_older_raises(self): block_migration=block_migration, disk_over_commit=disk_over_commit) + def test_handle_schedule_error_adds_instance_fault(self): + instance = {'uuid': 'fake-uuid'} + self.mox.StubOutWithMock(db, 'instance_update_and_get_original') + self.mox.StubOutWithMock(db, 'instance_fault_create') + self.mox.StubOutWithMock(notifier, 'notify') + db.instance_update_and_get_original(self.context, instance['uuid'], + mox.IgnoreArg()).AndReturn( + (None, instance)) + db.instance_fault_create(self.context, mox.IgnoreArg()) + notifier.notify(self.context, mox.IgnoreArg(), + 'scheduler.run_instance', + notifier.ERROR, mox.IgnoreArg()) + self.mox.ReplayAll() + + driver.handle_schedule_error(self.context, + exception.NoValidHost('test'), + instance['uuid'], {}) + class SchedulerDriverBaseTestCase(SchedulerTestCase): """Test cases for base scheduler driver class methods