diff options
| author | Johannes Erdfelt <johannes.erdfelt@rackspace.com> | 2012-09-19 21:07:53 +0000 |
|---|---|---|
| committer | Johannes Erdfelt <johannes.erdfelt@rackspace.com> | 2012-09-19 21:55:50 +0000 |
| commit | 66c34275f2defe94e68669fe4df1c4ea1830f720 (patch) | |
| tree | 1b4afd0fb0df821779cec7c2b45eed34acf3ec5d /nova | |
| parent | 51cc814e435973ab2c150fddf8fda6d08ad03547 (diff) | |
Clean up test_state_revert
While the test is simply to ensure that the state is reverted, no
matter what the failure, it did so a bit bluntly. The use of Exception
on both the raising side and the catching side ended up masking a
variety of other unintended failures.
stop_instance and start_instance had bugs (fixed in parent patch)
which ended up raising a KeyError exception. This still ended up
resetting the state correctly because only the nested call to the
decorator did not use the keyword argument for instance.
rebuild_instance, revert_resize, prep_resize and resize_instance all
required multiple extra arguments that weren't passed. This raised
a TypeError exception.
revert_resize and resize_instance also required an elevated context
or nova.db.api.migration_get() would raise a exception.AdminRequired
exception.
Even with these unintended problems, the test still performed it's
intended purpose, to test that task state is reverted. However,
the use of Exception makes auditing the test harder.
Change-Id: I5150957375ab3e57e7971f5e15daba531bb58803
Diffstat (limited to 'nova')
| -rw-r--r-- | nova/compute/manager.py | 2 | ||||
| -rw-r--r-- | nova/tests/compute/test_compute.py | 59 |
2 files changed, 38 insertions, 23 deletions
diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 6088ba5e6..f8d07d92b 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1409,6 +1409,7 @@ class ComputeManager(manager.SchedulerDependentManager): source machine. """ + context = context.elevated() migration_ref = self.db.migration_get(context, migration_id) with self._error_out_instance_on_exception(context, instance['uuid'], reservations): @@ -1543,6 +1544,7 @@ class ComputeManager(manager.SchedulerDependentManager): def resize_instance(self, context, instance, migration_id, image, reservations=None): """Starts the migration of a running instance to another host.""" + context = context.elevated() migration_ref = self.db.migration_get(context, migration_id) with self._error_out_instance_on_exception(context, instance['uuid'], reservations): diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 14edaa7a0..d09ae3b32 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -1279,7 +1279,10 @@ class ComputeTestCase(BaseTestCase): instance=jsonutils.to_primitive(instance)) def _test_state_revert(self, operation, pre_task_state, - post_task_state): + post_task_state=None, kwargs=None): + if kwargs is None: + kwargs = {} + instance = self._create_fake_instance() self.compute.run_instance(self.context, instance=instance) @@ -1292,17 +1295,18 @@ class ComputeTestCase(BaseTestCase): orig_notify = self.compute._notify_about_instance_usage def _get_an_exception(*args, **kwargs): - raise Exception("This fails every single time!") + raise test.TestingException() self.stubs.Set(self.context, 'elevated', _get_an_exception) self.stubs.Set(self.compute, '_notify_about_instance_usage', _get_an_exception) + func = getattr(self.compute, operation) + raised = False try: - ret_val = getattr(self.compute, operation)(self.context, - instance=instance) - except Exception: + ret_val = func(self.context, instance=instance, **kwargs) + except test.TestingException: raised = True finally: # self.context.elevated() is called in tearDown() @@ -1320,28 +1324,37 @@ class ComputeTestCase(BaseTestCase): def test_state_revert(self): """ensure that task_state is reverted after a failed operation""" actions = [ - ("reboot_instance", task_states.REBOOTING, None), - ("stop_instance", task_states.STOPPING, None), - ("start_instance", task_states.STARTING, None), + ("reboot_instance", task_states.REBOOTING), + ("stop_instance", task_states.STOPPING), + ("start_instance", task_states.STARTING), ("terminate_instance", task_states.DELETING, task_states.DELETING), - ("power_off_instance", task_states.POWERING_OFF, None), - ("power_on_instance", task_states.POWERING_ON, None), - ("rebuild_instance", task_states.REBUILDING, None), - ("set_admin_password", task_states.UPDATING_PASSWORD, None), - ("rescue_instance", task_states.RESCUING, None), - ("unrescue_instance", task_states.UNRESCUING, None), - ("revert_resize", task_states.RESIZE_REVERTING, None), - ("prep_resize", task_states.RESIZE_PREP, None), - ("resize_instance", task_states.RESIZE_PREP, None), - ("pause_instance", task_states.PAUSING, None), - ("unpause_instance", task_states.UNPAUSING, None), - ("suspend_instance", task_states.SUSPENDING, None), - ("resume_instance", task_states.RESUMING, None), + ("power_off_instance", task_states.POWERING_OFF), + ("power_on_instance", task_states.POWERING_ON), + ("rebuild_instance", task_states.REBUILDING, None, + {'orig_image_ref': None, + 'image_ref': None, + 'injected_files': [], + 'new_pass': ''}), + ("set_admin_password", task_states.UPDATING_PASSWORD), + ("rescue_instance", task_states.RESCUING), + ("unrescue_instance", task_states.UNRESCUING), + ("revert_resize", task_states.RESIZE_REVERTING, None, + {'migration_id': None}), + ("prep_resize", task_states.RESIZE_PREP, None, + {'image': {}, + 'instance_type': {}}), + ("resize_instance", task_states.RESIZE_PREP, None, + {'migration_id': None, + 'image': {}}), + ("pause_instance", task_states.PAUSING), + ("unpause_instance", task_states.UNPAUSING), + ("suspend_instance", task_states.SUSPENDING), + ("resume_instance", task_states.RESUMING), ] - for operation, pre_state, post_state in actions: - self._test_state_revert(operation, pre_state, post_state) + for operation in actions: + self._test_state_revert(*operation) def _ensure_quota_reservations_committed(self): """Mock up commit of quota reservations""" |
