Add missing support for the root_device property

Our documentation requires it to be set for multi-disk nodes, but it's
not accepted in instackenv.json. This patch fixes it.

As a side fix, property values are no longer converted to strings. This
has never been required, and may potentially yield incorrect results
(like with root_device, which is a dictionary).

Change-Id: I4e9fbc940c424071206e96b3d90086282f381a9f
Closes-Bug: #1742905

releasenotes/notes/enroll-root-device-a172f98f50706a12.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Recognizes the ``root_device`` property when enrolling nodes. We recommend
+    it to be set for multi-disk nodes, but the enrolling procedure does not
+    actually accept it.
+  - |
+    Node properties are no longer converted to strings on enrolling. This is
+    not required by the Bare Metal service and may yield incorrect results.

tripleo_common/tests/utils/test_nodes.py

@@ -293,11 +293,13 @@ class NodesTest(base.TestCase):
 
     def test_register_all_nodes(self):
         node_list = [self._get_node()]
+        node_list[0]['root_device'] = {"serial": "abcdef"}
         node_properties = {"cpus": "1",
                            "memory_mb": "2048",
                            "local_gb": "30",
                            "cpu_arch": "amd64",
-                           "capabilities": "num_nics:6"}
+                           "capabilities": "num_nics:6",
+                           "root_device": {"serial": "abcdef"}}
         ironic = mock.MagicMock()
         nodes.register_all_nodes(node_list, client=ironic)
         pxe_node_driver_info = {"ipmi_address": "foo.bar",
@@ -441,6 +443,7 @@ class NodesTest(base.TestCase):
 
         node = self._get_node()
         node.update(interfaces)
+        node['root_device'] = {'serial': 'abcdef'}
         ironic = mock.MagicMock()
         node_map = {'mac': {'aaa': 1}}
 
@@ -454,6 +457,8 @@ class NodesTest(base.TestCase):
                 {'path': '/properties/cpu_arch', 'value': 'amd64'},
                 {'path': '/properties/cpus', 'value': '1'},
                 {'path': '/properties/capabilities', 'value': 'num_nics:6'},
+                {'path': '/properties/root_device',
+                 'value': {'serial': 'abcdef'}},
                 {'path': '/driver_info/ipmi_username', 'value': 'test'}]
             for iface, value in interfaces.items():
                 update_patch.append({'path': '/%s' % iface, 'value': value})
@@ -638,24 +643,6 @@ class NodesTest(base.TestCase):
         nodes._update_or_register_ironic_node(node, node_map, client=ironic)
         ironic.node.update.assert_called_once_with('abcdef', mock.ANY)
 
-    def test_register_ironic_node_int_values(self):
-        node_properties = {"cpus": "1",
-                           "memory_mb": "2048",
-                           "local_gb": "30",
-                           "cpu_arch": "amd64",
-                           "capabilities": "num_nics:6"}
-        node = self._get_node()
-        node['cpu'] = 1
-        node['memory'] = 2048
-        node['disk'] = 30
-        client = mock.MagicMock()
-        nodes.register_ironic_node(node, client=client)
-        client.node.create.assert_called_once_with(driver=mock.ANY,
-                                                   name='node1',
-                                                   properties=node_properties,
-                                                   resource_class='baremetal',
-                                                   driver_info=mock.ANY)
-
     def test_register_ironic_node_fake_pxe(self):
         node_properties = {"cpus": "1",
                            "memory_mb": "2048",
@@ -794,35 +781,6 @@ class NodesTest(base.TestCase):
                          'redfish_username': 'test',
                          'redfish_system_id': '/redfish/v1/Systems/1'})
 
-    def test_register_ironic_node_update_int_values(self):
-        node = self._get_node()
-        ironic = mock.MagicMock()
-        node['cpu'] = 1
-        node['memory'] = 2048
-        node['disk'] = 30
-        node_map = {'mac': {'aaa': 1}}
-
-        def side_effect(*args, **kwargs):
-            update_patch = [
-                {'path': '/name', 'value': 'node1'},
-                {'path': '/driver_info/ipmi_password', 'value': 'random'},
-                {'path': '/driver_info/ipmi_address', 'value': 'foo.bar'},
-                {'path': '/properties/memory_mb', 'value': '2048'},
-                {'path': '/properties/local_gb', 'value': '30'},
-                {'path': '/properties/cpu_arch', 'value': 'amd64'},
-                {'path': '/properties/cpus', 'value': '1'},
-                {'path': '/properties/capabilities', 'value': 'num_nics:6'},
-                {'path': '/driver_info/ipmi_username', 'value': 'test'}]
-            for key in update_patch:
-                key['op'] = 'add'
-            self.assertThat(update_patch,
-                            matchers.MatchesSetwise(*(map(matchers.Equals,
-                                                      args[1]))))
-            return mock.Mock(uuid='uuid1')
-
-        ironic.node.update.side_effect = side_effect
-        nodes._update_or_register_ironic_node(node, node_map, client=ironic)
-
     def test_clean_up_extra_nodes_ironic(self):
         node = collections.namedtuple('node', ['uuid'])
         client = mock.MagicMock()
@@ -928,7 +886,8 @@ VALID_NODE_JSON = [
      'cpu': 2,
      'memory': 1024,
      'disk': 40,
-     'arch': 'x86_64'},
+     'arch': 'x86_64',
+     'root_device': {'foo': 'bar'}},
     {'pm_type': 'redfish',
      'pm_addr': '1.2.3.4',
      'pm_user': 'root',
@@ -1081,3 +1040,15 @@ class TestValidateNodes(base.TestCase):
         self.assertRaisesRegex(exception.InvalidNode,
                                'fields are missing: pm_system_id',
                                nodes.validate_nodes, nodes_json)
+
+    def test_invalid_root_device(self):
+        nodes_json = [
+            {'pm_type': 'pxe_ipmitool',
+             'pm_addr': '1.1.1.1',
+             'pm_user': 'root',
+             'pm_password': 'p@$$w0rd',
+             'root_device': 42}
+        ]
+        self.assertRaisesRegex(exception.InvalidNode,
+                               'Invalid root device',
+                               nodes.validate_nodes, nodes_json)

tripleo_common/utils/nodes.py

@@ -328,8 +328,9 @@ def register_ironic_node(node, client):
     mapping = {'cpus': 'cpu',
                'memory_mb': 'memory',
                'local_gb': 'disk',
-               'cpu_arch': 'arch'}
+               'cpu_arch': 'arch',
-    properties = {k: six.text_type(node.get(v))
+               'root_device': 'root_device'}
+    properties = {k: node[v]
                   for k, v in mapping.items()
                   if node.get(v) is not None}
 
@@ -413,6 +414,7 @@ _NON_DRIVER_FIELDS = {'cpu': '/properties/cpus',
                       'memory': '/properties/memory_mb',
                       'disk': '/properties/local_gb',
                       'arch': '/properties/cpu_arch',
+                      'root_device': '/properties/root_device',
                       'name': '/name',
                       'resource_class': '/resource_class',
                       'kernel_id': '/driver_info/deploy_kernel',
@@ -444,9 +446,8 @@ def _update_or_register_ironic_node(node, node_map, client):
         for key, value in patched.items():
             if key == 'uuid':
                 continue  # not needed during update
-            node_patch.append({'path': key,
+            node_patch.append({'path': key, 'value': value, 'op': 'add'})
-                               'value': six.text_type(value),
+
-                               'op': 'add'})
         ironic_node = client.node.update(node_uuid, node_patch)
     else:
         ironic_node = register_ironic_node(node, client)
@@ -555,6 +556,13 @@ def validate_nodes(nodes_list):
             failures.append(
                 (index, 'Invalid capabilities: %s' % node.get('capabilities')))
 
+        if node.get('root_device') is not None:
+            if not isinstance(node['root_device'], dict):
+                failures.append(
+                    (index,
+                     'Invalid root device: expected dict, got %s' %
+                     node['root_device']))
+
         for field in node:
             converted = handler.convert_key(field)
             if (converted is None and field not in _NON_DRIVER_FIELDS and