summaryrefslogtreecommitdiffstats
path: root/nova
diff options
context:
space:
mode:
authorJohannes Erdfelt <johannes.erdfelt@rackspace.com>2012-09-19 21:07:53 +0000
committerJohannes Erdfelt <johannes.erdfelt@rackspace.com>2012-09-19 21:55:50 +0000
commit66c34275f2defe94e68669fe4df1c4ea1830f720 (patch)
tree1b4afd0fb0df821779cec7c2b45eed34acf3ec5d /nova
parent51cc814e435973ab2c150fddf8fda6d08ad03547 (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.py2
-rw-r--r--nova/tests/compute/test_compute.py59
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"""