Generalize exception handling in Nova driver

We need to ensure nodes get unprovisioned and disassociated from instances upon
all failure cases, not only the specific exceptions being caught now.  This
removes the catching of those specific exceptions during instance spawn and
performs required cleanup and re-raising.

Change-Id: I9461139400d7aa0a339550cf89f3d49e3a8cb2c0

2 files changed, 41 insertions, 11 deletions

diff --git a/ironic/nova/tests/virt/ironic/test_driver.py b/ironic/nova/tests/virt/ironic/test_driver.py
index 1699fd525..2bdbaed41 100644
--- a/ironic/nova/tests/virt/ironic/test_driver.py
+++ b/ironic/nova/tests/virt/ironic/test_driver.py
@@ -631,7 +631,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
         mock_node.get.return_value = node
         mock_node.validate.return_value = ironic_utils.get_test_validation()
         mock_node.set_provision_state.side_effect = ironic_exception.BadRequest
-        self.assertRaises(exception.InstanceDeployFailure,
+        self.assertRaises(ironic_exception.BadRequest,
                           self.driver.spawn,
                           self.ctx, instance, None, [], None)
 
@@ -642,6 +642,37 @@ class IronicDriverTestCase(test.NoDBTestCase):
         mock_cleanup_deploy.assert_called_once_with(node, instance, None)
 
     @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
+    @mock.patch.object(FAKE_CLIENT, 'node')
+    @mock.patch.object(flavor_obj.Flavor, 'get_by_id')
+    @mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
+    @mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
+    @mock.patch.object(ironic_driver.IronicDriver, 'destroy')
+    def test_spawn_node_trigger_deploy_fail3(self, mock_destroy,
+                                            mock_pvifs, mock_sf,
+                                            mock_flavor, mock_node,
+                                            mock_looping):
+        node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
+        fake_net_info = utils.get_test_network_info()
+        node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
+        instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
+        fake_flavor = {'ephemeral_gb': 0}
+        mock_flavor.return_value = fake_flavor
+
+        mock_node.get.return_value = node
+        mock_node.validate.return_value = ironic_utils.get_test_validation()
+
+        fake_looping_call = FakeLoopingCall()
+        mock_looping.return_value = fake_looping_call
+
+        fake_looping_call.wait.side_effect = ironic_exception.BadRequest
+        fake_net_info = utils.get_test_network_info()
+        self.assertRaises(ironic_exception.BadRequest,
+                          self.driver.spawn,
+                          self.ctx, instance, None, [], None, fake_net_info)
+        mock_destroy.assert_called_once_with(self.ctx, instance,
+                                             fake_net_info)
+
+    @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
     @mock.patch.object(instance_obj.Instance, 'save')
     @mock.patch.object(FAKE_CLIENT, 'node')
     @mock.patch.object(flavor_obj.Flavor, 'get_by_id')
diff --git a/ironic/nova/virt/ironic/driver.py b/ironic/nova/virt/ironic/driver.py
index 1ac7d4eb0..8f71f0a81 100644
--- a/ironic/nova/virt/ironic/driver.py
+++ b/ironic/nova/virt/ironic/driver.py
@@ -549,21 +549,20 @@ class IronicDriver(virt_driver.ComputeDriver):
         try:
             icli.call("node.set_provision_state", node_uuid,
                       ironic_states.ACTIVE)
-        except (exception.NovaException,               # Retry failed
-                ironic_exception.InternalServerError,  # Validations
-                ironic_exception.BadRequest) as e:     # Maintenance
-            msg = (_("Failed to request Ironic to provision instance "
-                     "%(inst)s: %(reason)s") % {'inst': instance['uuid'],
-                                                'reason': str(e)})
-            LOG.error(msg)
-            self._cleanup_deploy(node, instance, network_info)
-            raise exception.InstanceDeployFailure(msg)
+        except Exception as e:
+            with excutils.save_and_reraise_exception():
+                msg = (_("Failed to request Ironic to provision instance "
+                         "%(inst)s: %(reason)s") % {'inst': instance['uuid'],
+                                                    'reason': str(e)})
+                LOG.error(msg)
+                self._cleanup_deploy(node, instance, network_info)
 
         timer = loopingcall.FixedIntervalLoopingCall(self._wait_for_active,
                                                      icli, instance)
+
         try:
             timer.start(interval=CONF.ironic.api_retry_interval).wait()
-        except exception.InstanceDeployFailure:
+        except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.error(_("Error deploying instance %(instance)s on "
                             "baremetal node %(node)s.") %