openstack/ironic
Code Issues Proposed changes

Allow node.instance_uuid to be removed during cleaning

Browse Source 
As soon as Nova moves the node's provision state to DELETING, it
attempts to remove the node's instance UUID. Ironic throws an error
because it considers the node in a state transition. If cleaning
takes longer than the Nova timeout, the delete will error out.

The fix is to allow node.instance_uuid removal while the node is in
CLEANING state. In L, we should fix the Nova driver to not remove
node.instance_uuid, and clear it after tear down when we clear out
node.instance_info.

Also allows updates to nodes in CLEANFAIL to match the other states
in UPDATE_ALLOWED_STATES, and if the node fails CLEANING quickly, the
node.instance_uuid removal could happen here.

Change-Id: I46771041f90a1d52f6b2d8e107ca10b650b720c1
Closes-Bug: 1435605
This commit is contained in:
Josh Gachnang 2015-03-23 15:48:37 -07:00
parent d5bf6233f9
commit 47c58ea9bf
3 changed files with 41 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ironic/api/controllers/v1/node.py
View File

@ -1013,7 +1013,17 @@ class NodesController(rest.RestController):
# Check if node is transitioning state, although nodes in some states
# can be updated.
if ((rpc_node.target_power_state or rpc_node.target_provision_state)
if (rpc_node.provision_state == ir_states.CLEANING and
patch == [{'op': 'remove', 'path': '/instance_uuid'}]):
# Allow node.instance_uuid removal during cleaning, but not other
# operations.
# TODO(JoshNang) remove node.instance_uuid when removing
# instance_info stop removing node.instance_uuid in the Nova
# Ironic driver. Bug: 1436568
LOG.debug('Removing instance uuid %(instance)s from node %(node)s',
{'instance': rpc_node.instance_uuid,
'node': rpc_node.uuid})
elif ((rpc_node.target_power_state or rpc_node.target_provision_state)
and rpc_node.provision_state not in
ir_states.UPDATE_ALLOWED_STATES):
msg = _("Node %s can not be updated while a state transition "

2
ironic/common/states.py
View File

@ -150,7 +150,7 @@ INSPECTFAIL = 'inspect failed'
""" Node inspection failed. """
UPDATE_ALLOWED_STATES = (DEPLOYFAIL, INSPECTING, INSPECTFAIL)
UPDATE_ALLOWED_STATES = (DEPLOYFAIL, INSPECTING, INSPECTFAIL, CLEANFAIL)
"""Transitional states in which we allow updating a node."""

29
ironic/tests/api/v1/test_nodes.py
View File

@ -945,6 +945,35 @@ class TestPatch(test_api_base.FunctionalTest):
self.assertEqual(400, response.status_code)
self.assertTrue(response.json['error_message'])
def test_remove_instance_uuid_cleaning(self):
node = obj_utils.create_test_node(
self.context,
uuid=uuidutils.generate_uuid(),
provision_state=states.CLEANING,
target_provision_state=states.AVAILABLE)
self.mock_update_node.return_value = node
response = self.patch_json('/nodes/%s' % node.uuid,
[{'op': 'remove',
'path': '/instance_uuid'}])
self.assertEqual('application/json', response.content_type)
self.assertEqual(200, response.status_code)
self.mock_update_node.assert_called_once_with(
mock.ANY, mock.ANY, 'test-topic')
def test_add_state_in_cleaning(self):
node = obj_utils.create_test_node(
self.context,
uuid=uuidutils.generate_uuid(),
provision_state=states.CLEANING,
target_provision_state=states.AVAILABLE)
self.mock_update_node.return_value = node
response = self.patch_json('/nodes/%s' % node.uuid,
[{'path': '/extra/foo', 'value': 'bar',
'op': 'add'}], expect_errors=True)
self.assertEqual('application/json', response.content_type)
self.assertEqual(409, response.status_code)
self.assertTrue(response.json['error_message'])
def test_remove_mandatory_field(self):
response = self.patch_json('/nodes/%s' % self.node.uuid,
[{'path': '/driver', 'op': 'remove'}],