Don't return from a finally block

The code in the finally block always executes, even if an exception is
raised. If a return is done, that will cause exceptions to be silently
ignored.

Some unit tests have been failing because of API changes to the
nova.compute.manager:ComputeManager._delete_instance method but the
use of finally/return ended up silently ignoring that error and
caused the test to appear as if it was passing. Another unit test
fails because of a missing uuid attribute.

It could also potentially cause other exceptions to be ignored at
runtime (such as SystemExit) which is likely not the intended
behavior.

Change-Id: I083ee7215296e35a0027be5b3ddf8b764f44cc5b
Closes-bug: 1324277

nova/compute/manager.py

@@ -794,8 +794,7 @@ class ComputeManager(manager.Manager):
                 # we don't want that an exception blocks the init_host
                 msg = _('Failed to complete a deletion')
                 LOG.exception(msg, instance=instance)
-            finally:
-                return
+            return
 
         if (instance.vm_state == vm_states.BUILDING or
             instance.task_state in [task_states.SCHEDULING,
@@ -847,8 +846,7 @@ class ComputeManager(manager.Manager):
                 msg = _('Failed to complete a deletion')
                 LOG.exception(msg, instance=instance)
                 self._set_instance_error_state(context, instance['uuid'])
-            finally:
-                return
+            return
 
         try_reboot, reboot_type = self._retry_reboot(context, instance)
         current_power_state = self._get_power_state(context, instance)
@@ -888,8 +886,7 @@ class ComputeManager(manager.Manager):
                 # we don't want that an exception blocks the init_host
                 msg = _('Failed to stop instance')
                 LOG.exception(msg, instance=instance)
-            finally:
-                return
+            return
 
         if instance.task_state == task_states.POWERING_ON:
             try:
@@ -901,8 +898,7 @@ class ComputeManager(manager.Manager):
                 # we don't want that an exception blocks the init_host
                 msg = _('Failed to start instance')
                 LOG.exception(msg, instance=instance)
-            finally:
-                return
+            return
 
         net_info = compute_utils.get_nw_info_for_instance(instance)
         try:

nova/tests/compute/test_compute.py

@@ -6454,7 +6454,7 @@ class ComputeTestCase(BaseTestCase):
     def test_partial_deletion_raise_exception(self):
         admin_context = context.get_admin_context()
         instance = objects.Instance(admin_context)
-        instance.id = 1
+        instance.uuid = str(uuid.uuid4())
         instance.vm_state = vm_states.DELETED
         instance.deleted = False
 

nova/tests/compute/test_compute_mgr.py

@@ -325,7 +325,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
         instance.obj_load_attr('system_metadata')
         block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
                 self.context, instance.uuid).AndReturn(bdms)
-        self.compute._delete_instance(self.context, instance, bdms)
+        self.compute._delete_instance(self.context, instance, bdms,
+                                      mox.IgnoreArg())
 
         self.mox.ReplayAll()
         self.compute._init_instance(self.context, instance)
@@ -498,7 +499,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
         instance.obj_load_attr('system_metadata')
         block_device_obj.BlockDeviceMappingList.get_by_instance_uuid(
                 self.context, instance.uuid).AndReturn(bdms)
-        self.compute._delete_instance(self.context, instance, bdms)
+        self.compute._delete_instance(self.context, instance, bdms,
+                                      mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute._init_instance(self.context, instance)