Merge "Call virt.driver.destroy before deallocating network."

nova/compute/manager.py

@@ -1297,6 +1297,16 @@ class ComputeManager(manager.SchedulerDependentManager):
                     admin_password, is_first_time, node, instance)
         do_run_instance()
 
+    def _try_deallocate_network(self, context, instance):
+        try:
+            # tear down allocated network structure
+            self._deallocate_network(context, instance)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                LOG.error(_('Failed to deallocate network for instance.'),
+                          instance=instance)
+                self._set_instance_error_state(context, instance['uuid'])
+
     def _shutdown_instance(self, context, instance, bdms):
         """Shutdown an instance on this host."""
         context = context.elevated()
@@ -1311,21 +1321,28 @@ class ComputeManager(manager.SchedulerDependentManager):
         except exception.NetworkNotFound:
             network_info = network_model.NetworkInfo()
 
-        try:
-            # tear down allocated network structure
-            self._deallocate_network(context, instance)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                LOG.error(_('Failed to deallocate network for instance.'),
-                          instance=instance)
-                self._set_instance_error_state(context, instance['uuid'])
-
         # NOTE(vish) get bdms before destroying the instance
         vol_bdms = self._get_volume_bdms(bdms)
         block_device_info = self._get_instance_volume_block_device_info(
             context, instance, bdms=bdms)
-        self.driver.destroy(instance, self._legacy_nw_info(network_info),
-                            block_device_info)
+
+        # NOTE(melwitt): attempt driver destroy before releasing ip, may
+        #                want to keep ip allocated for certain failures
+        try:
+            self.driver.destroy(instance, self._legacy_nw_info(network_info),
+                                block_device_info)
+        except exception.InstancePowerOffFailure:
+            # if the instance can't power off, don't release the ip
+            with excutils.save_and_reraise_exception():
+                pass
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                # deallocate ip and fail without proceeding to
+                # volume api calls, preserving current behavior
+                self._try_deallocate_network(context, instance)
+
+        self._try_deallocate_network(context, instance)
+
         for bdm in vol_bdms:
             try:
                 # NOTE(vish): actual driver detach done in driver.destroy, so

nova/tests/compute/test_compute.py

@@ -2337,6 +2337,36 @@ class ComputeTestCase(BaseTestCase):
                 instance=jsonutils.to_primitive(instance),
                 bdms={})
 
+    def test_delete_instance_keeps_net_on_power_off_fail(self):
+        self.mox.StubOutWithMock(self.compute.driver, 'destroy')
+        self.mox.StubOutWithMock(self.compute, '_deallocate_network')
+        exp = exception.InstancePowerOffFailure(reason='')
+        self.compute.driver.destroy(mox.IgnoreArg(), mox.IgnoreArg(),
+                                    mox.IgnoreArg()).AndRaise(exp)
+        # mox will detect if _deallocate_network gets called unexpectedly
+        self.mox.ReplayAll()
+        instance = self._create_fake_instance()
+        self.assertRaises(exception.InstancePowerOffFailure,
+                          self.compute._delete_instance,
+                          self.context,
+                          instance=jsonutils.to_primitive(instance),
+                          bdms={})
+
+    def test_delete_instance_loses_net_on_other_fail(self):
+        self.mox.StubOutWithMock(self.compute.driver, 'destroy')
+        self.mox.StubOutWithMock(self.compute, '_deallocate_network')
+        exp = test.TestingException()
+        self.compute.driver.destroy(mox.IgnoreArg(), mox.IgnoreArg(),
+                                    mox.IgnoreArg()).AndRaise(exp)
+        self.compute._deallocate_network(mox.IgnoreArg(), mox.IgnoreArg())
+        self.mox.ReplayAll()
+        instance = self._create_fake_instance()
+        self.assertRaises(test.TestingException,
+                          self.compute._delete_instance,
+                          self.context,
+                          instance=jsonutils.to_primitive(instance),
+                          bdms={})
+
     def test_delete_instance_deletes_console_auth_tokens(self):
         instance = self._create_fake_instance()
         self.flags(vnc_enabled=True)