From 2c225f1f2a0d371ef9603a62df6cd6210d656d67 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. Change-Id: I098409e5ff63b3c3719563de8144fda5bf482bdf Closes-Bug: #1802318 --- .../notes/caps-fix-f6f8817a48fa5c25.yaml | 4 ++ tripleo_common/tests/utils/test_nodes.py | 47 +++++++++++++++++++ tripleo_common/utils/nodes.py | 19 ++++++-- 3 files changed, 67 insertions(+), 3 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 651ca439b..450debb80 100644 --- a/tripleo_common/tests/utils/test_nodes.py +++ b/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' diff --git a/tripleo_common/utils/nodes.py b/tripleo_common/utils/nodes.py index e9cb47ec5..060d8948a 100644 --- a/tripleo_common/utils/nodes.py +++ b/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])