Reset parent_node values to uuids instead of names
So, I got myself nice and confused with testing parent_node logic when I used a name, but the ironic internals are modeled around queries involving UUID matching. We now identify names, and reset the values to be a UUID. Change-Id: I46ece586c254c58b80723bc905cad3144691fc5d
This commit is contained in:
parent
81b35931d4
commit
d1ca14289e
@ -2695,6 +2695,13 @@ class NodesController(rest.RestController):
|
|||||||
chassis = _replace_chassis_uuid_with_id(node)
|
chassis = _replace_chassis_uuid_with_id(node)
|
||||||
chassis_uuid = chassis and chassis.uuid or None
|
chassis_uuid = chassis and chassis.uuid or None
|
||||||
|
|
||||||
|
if ('parent_node' in node
|
||||||
|
and not uuidutils.is_uuid_like(node['parent_node'])):
|
||||||
|
|
||||||
|
parent_node = api_utils.check_node_policy_and_retrieve(
|
||||||
|
'baremetal:node:get', node['parent_node'])
|
||||||
|
node['parent_node'] = parent_node.uuid
|
||||||
|
|
||||||
new_node = objects.Node(context, **node)
|
new_node = objects.Node(context, **node)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@ -2721,6 +2728,7 @@ class NodesController(rest.RestController):
|
|||||||
return api_node
|
return api_node
|
||||||
|
|
||||||
def _validate_patch(self, patch, reset_interfaces):
|
def _validate_patch(self, patch, reset_interfaces):
|
||||||
|
corrected_values = {}
|
||||||
if self.from_chassis:
|
if self.from_chassis:
|
||||||
raise exception.OperationNotPermitted()
|
raise exception.OperationNotPermitted()
|
||||||
|
|
||||||
@ -2751,17 +2759,21 @@ class NodesController(rest.RestController):
|
|||||||
|
|
||||||
for network_data in network_data_fields:
|
for network_data in network_data_fields:
|
||||||
validate_network_data(network_data)
|
validate_network_data(network_data)
|
||||||
|
|
||||||
parent_node = api_utils.get_patch_values(patch, '/parent_node')
|
parent_node = api_utils.get_patch_values(patch, '/parent_node')
|
||||||
if parent_node:
|
if parent_node:
|
||||||
|
# At this point, if there *is* a parent_node, there is a value
|
||||||
|
# in the patch value list.
|
||||||
try:
|
try:
|
||||||
# Verify we can see the parent node
|
# Verify we can see the parent node
|
||||||
api_utils.check_node_policy_and_retrieve(
|
req_parent_node = api_utils.check_node_policy_and_retrieve(
|
||||||
'baremetal:node:get', parent_node[0])
|
'baremetal:node:get', parent_node[0])
|
||||||
|
# If we can't see the node, an exception gets raised.
|
||||||
except Exception:
|
except Exception:
|
||||||
msg = _("Unable to apply the requested parent_node. "
|
msg = _("Unable to apply the requested parent_node. "
|
||||||
"Requested value was invalid.")
|
"Requested value was invalid.")
|
||||||
raise exception.Invalid(msg)
|
raise exception.Invalid(msg)
|
||||||
|
corrected_values['parent_node'] = req_parent_node.uuid
|
||||||
|
return corrected_values
|
||||||
|
|
||||||
def _authorize_patch_and_get_node(self, node_ident, patch):
|
def _authorize_patch_and_get_node(self, node_ident, patch):
|
||||||
# deal with attribute-specific policy rules
|
# deal with attribute-specific policy rules
|
||||||
@ -2843,7 +2855,7 @@ class NodesController(rest.RestController):
|
|||||||
api_utils.allow_reset_interfaces()):
|
api_utils.allow_reset_interfaces()):
|
||||||
raise exception.NotAcceptable()
|
raise exception.NotAcceptable()
|
||||||
|
|
||||||
self._validate_patch(patch, reset_interfaces)
|
corrected_values = self._validate_patch(patch, reset_interfaces)
|
||||||
|
|
||||||
context = api.request.context
|
context = api.request.context
|
||||||
rpc_node = self._authorize_patch_and_get_node(node_ident, patch)
|
rpc_node = self._authorize_patch_and_get_node(node_ident, patch)
|
||||||
@ -2899,7 +2911,6 @@ class NodesController(rest.RestController):
|
|||||||
msg, status_code=http_client.CONFLICT)
|
msg, status_code=http_client.CONFLICT)
|
||||||
except exception.AllocationNotFound:
|
except exception.AllocationNotFound:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
names = api_utils.get_patch_values(patch, '/name')
|
names = api_utils.get_patch_values(patch, '/name')
|
||||||
if len(names):
|
if len(names):
|
||||||
error_msg = (_("Node %s: Cannot change name to invalid name ")
|
error_msg = (_("Node %s: Cannot change name to invalid name ")
|
||||||
@ -2915,6 +2926,12 @@ class NodesController(rest.RestController):
|
|||||||
node_dict['chassis_uuid'] = _get_chassis_uuid(rpc_node)
|
node_dict['chassis_uuid'] = _get_chassis_uuid(rpc_node)
|
||||||
|
|
||||||
node_dict = api_utils.apply_jsonpatch(node_dict, patch)
|
node_dict = api_utils.apply_jsonpatch(node_dict, patch)
|
||||||
|
if corrected_values:
|
||||||
|
# If we determined in our schema validation that someone
|
||||||
|
# provided non-fatal, but incorrect input, that we correct
|
||||||
|
# it here from the output returned from validation.
|
||||||
|
for key in corrected_values.keys():
|
||||||
|
node_dict[key] = corrected_values[key]
|
||||||
|
|
||||||
api_utils.patched_validate_with_schema(
|
api_utils.patched_validate_with_schema(
|
||||||
node_dict, node_patch_schema(), node_patch_validator)
|
node_dict, node_patch_schema(), node_patch_validator)
|
||||||
@ -2942,7 +2959,6 @@ class NodesController(rest.RestController):
|
|||||||
new_node = api.request.rpcapi.update_node(context,
|
new_node = api.request.rpcapi.update_node(context,
|
||||||
rpc_node, topic,
|
rpc_node, topic,
|
||||||
reset_interfaces)
|
reset_interfaces)
|
||||||
|
|
||||||
api_node = node_convert_with_links(new_node)
|
api_node = node_convert_with_links(new_node)
|
||||||
chassis_uuid = api_node.get('chassis_uuid')
|
chassis_uuid = api_node.get('chassis_uuid')
|
||||||
notify.emit_end_notification(context, new_node, 'update',
|
notify.emit_end_notification(context, new_node, 'update',
|
||||||
|
@ -8419,6 +8419,16 @@ class TestNodeParentNodePost(test_api_base.BaseApiTest):
|
|||||||
'/nodes', ndict, expect_errors=True, headers=headers)
|
'/nodes', ndict, expect_errors=True, headers=headers)
|
||||||
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
|
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
|
||||||
|
|
||||||
|
def test_create_node_with_named_parent_node_succeeds(self):
|
||||||
|
ndict = test_api_utils.post_get_test_node(
|
||||||
|
uuid=uuidutils.generate_uuid())
|
||||||
|
ndict['parent_node'] = 'din'
|
||||||
|
headers = {api_base.Version.string: '1.83'}
|
||||||
|
response = self.post_json('/nodes', ndict, expect_errors=True,
|
||||||
|
headers=headers)
|
||||||
|
self.assertEqual(http_client.CREATED, response.status_int)
|
||||||
|
self.assertEqual(self.node.uuid, response.json['parent_node'])
|
||||||
|
|
||||||
|
|
||||||
class TestNodeParentNodePatch(test_api_base.BaseApiTest):
|
class TestNodeParentNodePatch(test_api_base.BaseApiTest):
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
@ -8472,6 +8482,20 @@ class TestNodeParentNodePatch(test_api_base.BaseApiTest):
|
|||||||
self.mock_update_node.assert_not_called()
|
self.mock_update_node.assert_not_called()
|
||||||
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
|
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
|
||||||
|
|
||||||
|
def test_node_add_parent_node_not_uuid(self):
|
||||||
|
headers = {api_base.Version.string: '1.83'}
|
||||||
|
body = [{
|
||||||
|
'path': '/parent_node',
|
||||||
|
'value': 'djarin',
|
||||||
|
'op': 'add',
|
||||||
|
}]
|
||||||
|
|
||||||
|
self.patch_json('/nodes/%s' % self.child_node.uuid,
|
||||||
|
body, expect_errors=True, headers=headers)
|
||||||
|
self.mock_update_node.assert_called_once()
|
||||||
|
test_node = self.mock_update_node.call_args.args[2]
|
||||||
|
self.assertEqual(self.node.uuid, test_node.parent_node)
|
||||||
|
|
||||||
def test_node_remove_parent(self):
|
def test_node_remove_parent(self):
|
||||||
self.mock_update_node.return_value = self.node
|
self.mock_update_node.return_value = self.node
|
||||||
(self
|
(self
|
||||||
|
@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
The ``parent_node`` field, a newly added API field, has been constrained
|
||||||
|
to store UUIDs over the names of nodes. When names are used, the value is
|
||||||
|
changed to the UUID of the node.
|
Loading…
Reference in New Issue
Block a user