diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index 6a422a71c0dd..c3f4692c7250 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -1666,7 +1666,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 @@ -1699,7 +1700,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, @@ -1718,7 +1720,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, @@ -2549,6 +2552,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 a7051a68a991..77a4c3d1fc63 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -486,11 +486,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.""" @@ -1336,7 +1338,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)