openstack
/
nova
Code Issues Proposed changes

Ironic: Better handle InstanceNotFound on destroy() 

After telling Ironic to delete the instance the destroy() code in the
Ironic nova driver keeps waiting for the provision state of that node
to change. If the instance have been already cleaned up in Ironic the
nova code should just ignore it and continue. This also skip trying to
update the node to remove the instance association if the instance was
already removed in Ironic.

Missing unittests to the _unprovision() method were added as part of
this patch since it touches it.

Closes-Bug: #1436568
Change-Id: I0b1b710056d48c8b7bb2b46fdaba192922926420
This commit is contained in:
Lucas Alvares Gomes 2015-07-23 10:54:12 +01:00
parent fbfbed675c
commit 5eddd50537
2 changed files with 76 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -869,6 +869,23 @@ class IronicDriverTestCase(test.NoDBTestCase):
expected_patch = [{'path': '/instance_uuid', 'op': 'remove'}]
mock_update.assert_called_once_with(node.uuid, expected_patch)
@mock.patch.object(ironic_driver, '_validate_instance_and_node')
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_instance_already_removed(self, mock_update,
mock_validate):
mock_validate.side_effect = exception.InstanceNotFound(
instance_id='fake-instance')
node = ironic_utils.get_test_node(driver='fake',
instance_uuid=self.instance_uuid)
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
flavor = ironic_utils.get_test_flavor(extra_specs={})
self.driver._cleanup_deploy(self.ctx, node, instance, None,
flavor=flavor)
# assert node.update is not called
self.assertFalse(mock_update.called)
mock_validate.assert_called_once_with(mock.ANY, instance)
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_without_flavor(self, mock_update):
node = ironic_utils.get_test_node(driver='fake',
@ -1132,18 +1149,52 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.assertRaises(exception.NovaException, self.driver.destroy,
self.ctx, instance, None, None)
@mock.patch.object(FAKE_CLIENT, 'node')
def test_destroy_unprovision_fail(self, mock_node):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid,
provision_state=ironic_states.ACTIVE)
instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
@mock.patch.object(ironic_driver, '_validate_instance_and_node')
def test__unprovision_instance(self, mock_validate_inst):
fake_ironic_client = mock.Mock()
node = ironic_utils.get_test_node(
driver='fake',
provision_state=ironic_states.CLEANING)
instance = fake_instance.fake_instance_obj(self.ctx, node=node.uuid)
mock_validate_inst.return_value = node
self.driver._unprovision(fake_ironic_client, instance, node)
mock_validate_inst.assert_called_once_with(fake_ironic_client,
instance)
fake_ironic_client.call.assert_called_once_with(
"node.set_provision_state", node.uuid, "deleted")
mock_node.get_by_instance_uuid.return_value = node
self.assertRaises(exception.NovaException, self.driver.destroy,
self.ctx, instance, None, None)
mock_node.set_provision_state.assert_called_once_with(node_uuid,
'deleted')
@mock.patch.object(ironic_driver, '_validate_instance_and_node')
def test__unprovision_fail_max_retries(self, mock_validate_inst):
CONF.set_default('api_max_retries', default=2, group='ironic')
fake_ironic_client = mock.Mock()
node = ironic_utils.get_test_node(
driver='fake',
provision_state=ironic_states.ACTIVE)
instance = fake_instance.fake_instance_obj(self.ctx, node=node.uuid)
mock_validate_inst.return_value = node
self.assertRaises(exception.NovaException, self.driver._unprovision,
fake_ironic_client, instance, node)
expected_calls = (mock.call(mock.ANY, instance),
mock.call(mock.ANY, instance))
mock_validate_inst.assert_has_calls(expected_calls)
fake_ironic_client.call.assert_called_once_with(
"node.set_provision_state", node.uuid, "deleted")
@mock.patch.object(ironic_driver, '_validate_instance_and_node')
def test__unprovision_instance_not_found(self, mock_validate_inst):
fake_ironic_client = mock.Mock()
node = ironic_utils.get_test_node(
driver='fake', provision_state=ironic_states.DELETING)
instance = fake_instance.fake_instance_obj(self.ctx, node=node.uuid)
mock_validate_inst.side_effect = exception.InstanceNotFound(
instance_id='fake')
self.driver._unprovision(fake_ironic_client, instance, node)
mock_validate_inst.assert_called_once_with(fake_ironic_client,
instance)
fake_ironic_client.call.assert_called_once_with(
"node.set_provision_state", node.uuid, "deleted")
@mock.patch.object(FAKE_CLIENT, 'node')
def test_destroy_unassociate_fail(self, mock_node):

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

@ -369,10 +369,17 @@ class IronicDriver(virt_driver.ComputeDriver):
patch = patcher.create(node).get_cleanup_patch(instance, network_info,
flavor)
# Unassociate the node
# TODO(lucasagomes): This code is here for backwards compatibility
# with old versions of Ironic that won't clean up the instance
# association as part of the node's tear down. Should be removed
# on the next cycle (M).
patch.append({'op': 'remove', 'path': '/instance_uuid'})
try:
_validate_instance_and_node(self.ironicclient, instance)
self.ironicclient.call('node.update', node.uuid, patch)
except exception.InstanceNotFound:
LOG.debug("Instance already removed from Ironic node %s. Skip "
"updating it", node.uuid, instance=instance)
except ironic.exc.BadRequest:
LOG.error(_LE("Failed to clean up the parameters on node %(node)s "
"when unprovisioning the instance %(instance)s"),
@ -806,7 +813,12 @@ class IronicDriver(virt_driver.ComputeDriver):
data = {'tries': 0}
def _wait_for_provision_state():
node = _validate_instance_and_node(ironicclient, instance)
try:
node = _validate_instance_and_node(ironicclient, instance)
except exception.InstanceNotFound:
LOG.debug("Instance already removed from Ironic",
instance=instance)
raise loopingcall.LoopingCallDone()
if node.provision_state in (ironic_states.NOSTATE,
ironic_states.CLEANING,
ironic_states.CLEANFAIL,