diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index befb38c2ba1c..d1d3155341e0 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -1809,7 +1809,8 @@ class IronicDriverTestCase(test.NoDBTestCase): mock_node.get_by_instance_uuid.assert_called_with( instance.uuid, fields=ironic_driver._NODE_FIELDS) - mock_cleanup_deploy.assert_called_with(node, instance, network_info) + mock_cleanup_deploy.assert_called_with(node, instance, network_info, + remove_instance_info=False) # For states that makes sense check if set_provision_state has # been called @@ -1842,7 +1843,8 @@ 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_clean.assert_called_once_with(node, instance, None, + remove_instance_info=False) @mock.patch.object(FAKE_CLIENT.node, 'update') @mock.patch.object(ironic_driver.IronicDriver, @@ -1861,7 +1863,8 @@ class IronicDriverTestCase(test.NoDBTestCase): 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_clean.assert_called_once_with(node, instance, None, + remove_instance_info=False) @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state') @mock.patch.object(ironic_driver.IronicDriver, @@ -2692,6 +2695,24 @@ class IronicDriverTestCase(test.NoDBTestCase): mock_call.has_calls( [mock.call('node.update', node.uuid, expected_patch)]) + @mock.patch.object(ironic_driver.IronicDriver, '_stop_firewall') + @mock.patch.object(ironic_driver.IronicDriver, '_unplug_vifs') + @mock.patch.object(ironic_driver.IronicDriver, + '_cleanup_volume_target_info') + @mock.patch.object(cw.IronicClientWrapper, 'call') + def test__cleanup_deploy_no_remove_ii(self, mock_call, mock_vol, + mock_unvif, mock_stop_fw): + # TODO(TheJulia): This REALLY should be updated to cover all of the + # calls that take place. + node = ironic_utils.get_test_node(driver='fake') + instance = fake_instance.fake_instance_obj(self.ctx, + node=node.uuid) + self.driver._cleanup_deploy(node, instance, remove_instance_info=False) + mock_vol.assert_called_once_with(instance) + mock_unvif.assert_called_once_with(node, instance, None) + mock_stop_fw.assert_called_once_with(instance, None) + self.assertFalse(mock_call.called) + class IronicDriverSyncTestCase(IronicDriverTestCase): diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index fa2ae719fa56..0a449371ffb9 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -471,11 +471,13 @@ class IronicDriver(virt_driver.ComputeDriver): 'reason': e}, instance=instance) - def _cleanup_deploy(self, node, instance, network_info=None): + def _cleanup_deploy(self, node, instance, network_info=None, + remove_instance_info=True): self._cleanup_volume_target_info(instance) self._unplug_vifs(node, instance, network_info) self._stop_firewall(instance, network_info) - self._remove_instance_info_from_node(node, instance) + if remove_instance_info: + self._remove_instance_info_from_node(node, instance) def _wait_for_active(self, instance): """Wait for the node to be marked as ACTIVE in Ironic.""" @@ -1264,7 +1266,12 @@ class IronicDriver(virt_driver.ComputeDriver): # removed from ironic node. self._remove_instance_info_from_node(node, instance) finally: - self._cleanup_deploy(node, instance, network_info) + # NOTE(mgoddard): We don't need to remove instance info at this + # point since we will have already done it. The destroy will only + # succeed if this method returns without error, so we will end up + # removing the instance info eventually. + self._cleanup_deploy(node, instance, network_info, + remove_instance_info=False) LOG.info('Successfully unprovisioned Ironic node %s', node.uuid, instance=instance)