Merge "Enable removing name when updating node"

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):

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.