From 4f0991110ae64ab272f98c4c50fbda98ab6ccb89 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 8 Nov 2018 16:00:06 +0100 Subject: [PATCH] Correct handling of capabilities on node update When updating nodes we do not convert capabilities from the supported dict representation to internal string representation. This may result in incorrect capabilities after an update. This patch fixes it. Also enforce alphabetical order in capabilities to prevent them from reordering on updates. Conflicts: tripleo_common/utils/nodes.py Change-Id: I098409e5ff63b3c3719563de8144fda5bf482bdf Closes-Bug: #1802318 (cherry picked from commit 2c225f1f2a0d371ef9603a62df6cd6210d656d67) --- .../notes/caps-fix-f6f8817a48fa5c25.yaml | 4 ++ tripleo_common/tests/utils/test_nodes.py | 46 +++++++++++++++++++ tripleo_common/utils/nodes.py | 22 +++++++-- 3 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/caps-fix-f6f8817a48fa5c25.yaml diff --git a/releasenotes/notes/caps-fix-f6f8817a48fa5c25.yaml b/releasenotes/notes/caps-fix-f6f8817a48fa5c25.yaml new file mode 100644 index 000000000..9d715a049 --- /dev/null +++ b/releasenotes/notes/caps-fix-f6f8817a48fa5c25.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Node update now works correctly when capabilities are specified as a dict. diff --git a/tripleo_common/tests/utils/test_nodes.py b/tripleo_common/tests/utils/test_nodes.py index 3f09bb562..26013568c 100644 --- a/tripleo_common/tests/utils/test_nodes.py +++ b/tripleo_common/tests/utils/test_nodes.py @@ -511,6 +511,52 @@ 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', + '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' diff --git a/tripleo_common/utils/nodes.py b/tripleo_common/utils/nodes.py index 4502e1cd0..21606d4c6 100644 --- a/tripleo_common/utils/nodes.py +++ b/tripleo_common/utils/nodes.py @@ -451,8 +451,8 @@ _NON_DRIVER_FIELDS = {'cpu': '/properties/cpus', 'name': '/name', 'resource_class': '/resource_class', 'kernel_id': '/driver_info/deploy_kernel', - 'ramdisk_id': '/driver_info/deploy_ramdisk', - 'capabilities': '/properties/capabilities'} + 'ramdisk_id': '/driver_info/deploy_ramdisk'} + _NON_DRIVER_FIELDS.update({field: '/%s' % field for field in _KNOWN_INTERFACE_FIELDS}) @@ -471,6 +471,10 @@ def _update_or_register_ironic_node(node, node_map, client): if field in node: patched[path] = node.pop(field) + 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 @@ -536,6 +540,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 = {'mac', 'pm_type', 'capabilities'} + + def validate_nodes(nodes_list): """Validate all nodes list. @@ -599,7 +607,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 ('mac', 'pm_type')): + field not in _SPECIAL_NON_DRIVER_FIELDS): failures.append((index, 'Unknown field %s' % field)) if failures: @@ -609,8 +617,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])