openstack
/
nova
Code Issues Proposed changes

Merge "[ironic] Don't remove instance info twice in destroy"

This commit is contained in:
Zuul 2019-05-30 21:59:11 +00:00 committed by Gerrit Code Review
parent 809799e581 060e42b7fd
commit 743cac21c7
2 changed files with 34 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
nova/tests/unit/virt/ironic/test_driver.py
View File

@ -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):

13
nova/virt/ironic/driver.py
View File

@ -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)