ironic: Clean up resources after unprovision fails

When deleting a bare metal instance, even if ironic fails to
unprovision a node, network resources are deallocated. Then, VIFs are
removed though the VIFs are not detached from the ironic node.

This patch cleans up resources nova prepares for ironic even if
unprovision fails.

Change-Id: I01f45f157078a92ccc9f4ff5b4bfeae6cef77c1a
Closes-Bug: 1749629

nova/tests/unit/virt/ironic/test_driver.py

@@ -1589,7 +1589,10 @@ class IronicDriverTestCase(test.NoDBTestCase):
     @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
     @mock.patch.object(ironic_driver.IronicDriver,
                        '_validate_instance_and_node')
-    def test_destroy_trigger_undeploy_fail(self, fake_validate, mock_sps):
+    @mock.patch.object(ironic_driver.IronicDriver,
+                       '_cleanup_deploy')
+    def test_destroy_trigger_undeploy_fail(self, mock_clean, fake_validate,
+                                           mock_sps):
         node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
         node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid,
                                           provision_state=ironic_states.ACTIVE)
@@ -1599,6 +1602,26 @@ class IronicDriverTestCase(test.NoDBTestCase):
         mock_sps.side_effect = exception.NovaException()
         self.assertRaises(exception.NovaException, self.driver.destroy,
                           self.ctx, instance, None, None)
+        mock_clean.assert_called_once_with(node, instance, None)
+
+    @mock.patch.object(FAKE_CLIENT.node, 'update')
+    @mock.patch.object(ironic_driver.IronicDriver,
+                       '_validate_instance_and_node')
+    @mock.patch.object(ironic_driver.IronicDriver,
+                       '_cleanup_deploy')
+    def test_destroy_trigger_remove_info_fail(self, mock_clean, fake_validate,
+                                              mock_update):
+        node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
+        node = ironic_utils.get_test_node(
+            driver='fake', uuid=node_uuid,
+            provision_state=ironic_states.AVAILABLE)
+        fake_validate.return_value = node
+        instance = fake_instance.fake_instance_obj(self.ctx,
+                                                   node=node_uuid)
+        mock_update.side_effect = SystemError('unexpected error')
+        self.assertRaises(SystemError, self.driver.destroy,
+                          self.ctx, instance, None, None)
+        mock_clean.assert_called_once_with(node, instance, None)
 
     @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
     @mock.patch.object(ironic_driver.IronicDriver,

nova/virt/ironic/driver.py

@@ -1209,15 +1209,17 @@ class IronicDriver(virt_driver.ComputeDriver):
             #             without raising any exceptions.
             return
 
-        if node.provision_state in _UNPROVISION_STATES:
-            self._unprovision(instance, node)
-        else:
-            # NOTE(hshiina): if spawn() fails before ironic starts
-            #                provisioning, instance information should be
-            #                removed from ironic node.
-            self._remove_instance_info_from_node(node, instance)
+        try:
+            if node.provision_state in _UNPROVISION_STATES:
+                self._unprovision(instance, node)
+            else:
+                # NOTE(hshiina): if spawn() fails before ironic starts
+                #                provisioning, instance information should be
+                #                removed from ironic node.
+                self._remove_instance_info_from_node(node, instance)
+        finally:
+            self._cleanup_deploy(node, instance, network_info)
 
-        self._cleanup_deploy(node, instance, network_info)
         LOG.info('Successfully unprovisioned Ironic node %s',
                  node.uuid, instance=instance)