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

@@ -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():
-            if not api_utils.allow_node_logical_names():
+            raise exception.NotAcceptable()
-                raise exception.NotAcceptable()
+        if not api_utils.is_valid_node_name(name):
-            if not api_utils.is_valid_node_name(name):
+            raise wsme.exc.ClientSideError(
-                raise wsme.exc.ClientSideError(
+                error_msg, status_code=http_client.BAD_REQUEST)
-                    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 "
+        if node.name:
-                      "%(name)s") % {'name': node.name}
+            error_msg = _("Cannot create node with invalid name "
-        self._check_name_acceptable(node.name, error_msg)
+                          "%(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 "
+        if name:
-                      "name '%(name)s'") % {'node': node_ident, 'name': name}
+            error_msg = _("Node %(node)s: Cannot change name to invalid "
-        self._check_name_acceptable(name, error_msg)
+                          "name '%(name)s'") % {'node': node_ident,
+                                                'name': name}
+            self._check_name_acceptable(name, error_msg)
         try:
             node_dict = rpc_node.as_dict()
             # NOTE(lucasagomes):

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']
 
 

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

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']

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.