From 47e18981b42ec8669ca6fe8f9383f66b472435a5 Mon Sep 17 00:00:00 2001 From: Kan Date: Thu, 14 Jan 2016 06:12:49 +0000 Subject: [PATCH] Enable removing name when updating node Name is an optional property and should be removable(set back to None). Enable removing name when updating node with 'remove name' argument. Also add test cases for that. Change-Id: Ie89b6bd80ac773b357861ca8dc77f72fdc137aed Closes-Bug: #1533248 --- ironic/api/controllers/v1/node.py | 26 ++++++++++--------- ironic/api/controllers/v1/utils.py | 2 +- ironic/tests/unit/api/v1/test_nodes.py | 12 +++++++++ ironic/tests/unit/api/v1/test_utils.py | 18 +++++++++++++ .../node-name-remove-720aa8007f2f8b75.yaml | 4 +++ 5 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/node-name-remove-720aa8007f2f8b75.yaml 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.