Merge "Correct handling of capabilities on node update"

releasenotes/notes/caps-fix-f6f8817a48fa5c25.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    Node update now works correctly when capabilities are specified as a dict.

tripleo_common/tests/utils/test_nodes.py

@@ -549,6 +549,53 @@ class NodesTest(base.TestCase):
         nodes._update_or_register_ironic_node(node, node_map, client=ironic)
         ironic.node.update.assert_called_once_with(1, mock.ANY)
 
+    def test_register_update_caps_dict(self):
+        interfaces = {'boot_interface': 'pxe',
+                      'console_interface': 'ipmitool-socat',
+                      'deploy_interface': 'direct',
+                      'inspect_interface': 'inspector',
+                      'management_interface': 'ipmitool',
+                      'network_interface': 'neutron',
+                      'power_interface': 'ipmitool',
+                      'raid_interface': 'agent',
+                      'rescue_interface': 'agent',
+                      'storage_interface': 'cinder',
+                      'vendor_interface': 'ipmitool'}
+
+        node = self._get_node()
+        node.update(interfaces)
+        node['root_device'] = {'serial': 'abcdef'}
+        node['capabilities'] = {'profile': 'compute', 'num_nics': 6}
+        ironic = mock.MagicMock()
+        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,profile:compute'},
+                {'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})
+            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)
+        ironic.node.update.assert_called_once_with(1, mock.ANY)
+
     def test_register_update_with_images(self):
         node = self._get_node()
         node['kernel_id'] = 'image-k'

tripleo_common/utils/nodes.py

@@ -490,7 +490,6 @@ _NON_DRIVER_FIELDS = {'cpu': '/properties/cpus',
                                     '/driver_info/rescue_kernel'],
                       'ramdisk_id': ['/driver_info/deploy_ramdisk',
                                      '/driver_info/rescue_ramdisk'],
-                      'capabilities': '/properties/capabilities',
                       'platform': '/extra/tripleo_platform',
                       }
 
@@ -516,6 +515,10 @@ def _update_or_register_ironic_node(node, node_map, client):
                 for path in paths:
                     patched[path] = value
 
+        if 'capabilities' in node:
+            patched['/properties/capabilities'] = dict_to_capabilities(
+                node.pop('capabilities'))
+
         driver_info = handler.convert(node)
         for key, value in driver_info.items():
             patched['/driver_info/%s' % key] = value
@@ -581,6 +584,10 @@ def register_all_nodes(nodes_list, client, remove=False, glance_client=None,
     return seen
 
 
+# These fields are treated specially during enrolling/updating
+_SPECIAL_NON_DRIVER_FIELDS = {'ports', 'pm_type', 'capabilities'}
+
+
 def validate_nodes(nodes_list):
     """Validate all nodes list.
 
@@ -650,7 +657,7 @@ def validate_nodes(nodes_list):
         for field in node:
             converted = handler.convert_key(field)
             if (converted is None and field not in _NON_DRIVER_FIELDS and
-                    field not in ('ports', 'pm_type')):
+                    field not in _SPECIAL_NON_DRIVER_FIELDS):
                 failures.append((index, 'Unknown field %s' % field))
 
     if failures:
@@ -660,8 +667,14 @@ def validate_nodes(nodes_list):
 
 def dict_to_capabilities(caps_dict):
     """Convert a dictionary into a string with the capabilities syntax."""
+    if isinstance(caps_dict, six.string_types):
+        return caps_dict
+
+    # NOTE(dtantsur): sort capabilities so that their order does not change
+    # between updates.
+    items = sorted(caps_dict.items(), key=lambda tpl: tpl[0])
     return ','.join(["%s:%s" % (key, value)
-                     for key, value in caps_dict.items()
+                     for key, value in items
                      if value is not None])