diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 90c4f0d651..a2c29be195 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1042,12 +1042,11 @@ class NodesController(rest.RestController): :raises: exception.NotAcceptable :raises: wsme.exc.ClientSideError """ - if name: - if not api_utils.allow_node_logical_names(): - raise exception.NotAcceptable() - if not api_utils.is_valid_node_name(name): - raise wsme.exc.ClientSideError( - error_msg, status_code=http_client.BAD_REQUEST) + if not api_utils.allow_node_logical_names(): + raise exception.NotAcceptable() + if not api_utils.is_valid_node_name(name): + raise wsme.exc.ClientSideError( + error_msg, status_code=http_client.BAD_REQUEST) def _update_changed_fields(self, node, rpc_node): """Update rpc_node based on changed fields in a node. @@ -1220,9 +1219,10 @@ class NodesController(rest.RestController): e.code = http_client.BAD_REQUEST raise e - error_msg = _("Cannot create node with invalid name " - "%(name)s") % {'name': node.name} - self._check_name_acceptable(node.name, error_msg) + if node.name: + error_msg = _("Cannot create node with invalid name " + "%(name)s") % {'name': node.name} + self._check_name_acceptable(node.name, error_msg) node.provision_state = api_utils.initial_node_provision_state() new_node = objects.Node(pecan.request.context, @@ -1270,9 +1270,11 @@ class NodesController(rest.RestController): msg % node_ident, status_code=http_client.CONFLICT) name = api_utils.get_patch_value(patch, '/name') - error_msg = _("Node %(node)s: Cannot change name to invalid " - "name '%(name)s'") % {'node': node_ident, 'name': name} - self._check_name_acceptable(name, error_msg) + if name: + error_msg = _("Node %(node)s: Cannot change name to invalid " + "name '%(name)s'") % {'node': node_ident, + 'name': name} + self._check_name_acceptable(name, error_msg) try: node_dict = rpc_node.as_dict() # NOTE(lucasagomes): diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 538ca4540d..b88fe65219 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -68,7 +68,7 @@ def apply_jsonpatch(doc, patch): def get_patch_value(patch, path): for p in patch: - if p['path'] == path: + if p['path'] == path and p['op'] != 'remove': return p['value'] diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py index 46ea9c1dbe..70246d0cf3 100644 --- a/ironic/tests/unit/api/v1/test_nodes.py +++ b/ironic/tests/unit/api/v1/test_nodes.py @@ -1276,6 +1276,18 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.CONFLICT, response.status_code) self.assertTrue(response.json['error_message']) + @mock.patch.object(api_node.NodesController, '_check_name_acceptable') + def test_patch_name_remove_ok(self, cna_mock): + self.mock_update_node.return_value = self.node + response = self.patch_json('/nodes/%s' % self.node.uuid, + [{'path': '/name', + 'op': 'remove'}], + headers={api_base.Version.string: + "1.5"}) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + self.assertFalse(cna_mock.called) + @mock.patch.object(api_utils, 'get_rpc_node') def test_patch_update_drive_console_enabled(self, mock_rpc_node): self.node.console_enabled = True diff --git a/ironic/tests/unit/api/v1/test_utils.py b/ironic/tests/unit/api/v1/test_utils.py index f1e09c02eb..45bb6609de 100644 --- a/ironic/tests/unit/api/v1/test_utils.py +++ b/ironic/tests/unit/api/v1/test_utils.py @@ -56,6 +56,24 @@ class TestApiUtils(base.TestCase): utils.validate_sort_dir, 'fake-sort') + def test_get_patch_value_no_path(self): + patch = [{'path': '/name', 'op': 'update', 'value': 'node-0'}] + path = '/invalid' + value = utils.get_patch_value(patch, path) + self.assertIsNone(value) + + def test_get_patch_value_remove(self): + patch = [{'path': '/name', 'op': 'remove'}] + path = '/name' + value = utils.get_patch_value(patch, path) + self.assertIsNone(value) + + def test_get_patch_value_success(self): + patch = [{'path': '/name', 'op': 'replace', 'value': 'node-x'}] + path = '/name' + value = utils.get_patch_value(patch, path) + self.assertEqual('node-x', value) + def test_check_for_invalid_fields(self): requested = ['field_1', 'field_3'] supported = ['field_1', 'field_2', 'field_3'] diff --git a/releasenotes/notes/node-name-remove-720aa8007f2f8b75.yaml b/releasenotes/notes/node-name-remove-720aa8007f2f8b75.yaml new file mode 100644 index 0000000000..492cbadddf --- /dev/null +++ b/releasenotes/notes/node-name-remove-720aa8007f2f8b75.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixes an issue that prevented the node name to be + removed as part of the node update.