From a1d19d97b5973898b07ac05bb8c2c3cf5c199605 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 4 Sep 2017 14:43:13 +0200 Subject: [PATCH] Do not rely on the older ipmi_address field on lookup We need to always use bmc_address from inventory, the ipmi_address field is there only for compatibility with older processing hooks. Change-Id: Ibf00ecd9278af4ee9318ada44c7c670d13ac22aa Closes-Bug: #1714944 --- ironic_inspector/plugins/standard.py | 6 +++--- ironic_inspector/process.py | 2 +- ironic_inspector/test/unit/test_process.py | 12 ++++++++++++ .../notes/empty-ipmi-address-2-4d57c34aec7d14e2.yaml | 8 ++++++++ 4 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/empty-ipmi-address-2-4d57c34aec7d14e2.yaml diff --git a/ironic_inspector/plugins/standard.py b/ironic_inspector/plugins/standard.py index 37089e77d..33b47eafc 100644 --- a/ironic_inspector/plugins/standard.py +++ b/ironic_inspector/plugins/standard.py @@ -222,9 +222,9 @@ class ValidateInterfacesHook(base.ProcessingHook): """Validate information about network interfaces.""" bmc_address = utils.get_ipmi_address_from_data(introspection_data) - if bmc_address: - introspection_data['ipmi_address'] = bmc_address - else: + # Overwrite the old ipmi_address field to avoid inconsistency + introspection_data['ipmi_address'] = bmc_address + if not bmc_address: LOG.debug('No BMC address provided in introspection data, ' 'assuming virtual environment', data=introspection_data) diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index 068070d57..f68b72115 100644 --- a/ironic_inspector/process.py +++ b/ironic_inspector/process.py @@ -83,7 +83,7 @@ def _store_logs(introspection_data, node_info): def _find_node_info(introspection_data, failures): try: return node_cache.find_node( - bmc_address=introspection_data.get('ipmi_address'), + bmc_address=utils.get_ipmi_address_from_data(introspection_data), mac=utils.get_valid_macs(introspection_data)) except utils.NotFoundInCacheError as exc: not_found_hook = plugins_base.node_not_found_hook_manager() diff --git a/ironic_inspector/test/unit/test_process.py b/ironic_inspector/test/unit/test_process.py index 09027b668..b983a7640 100644 --- a/ironic_inspector/test/unit/test_process.py +++ b/ironic_inspector/test/unit/test_process.py @@ -112,6 +112,18 @@ class TestProcess(BaseProcessTest): self.process_mock.assert_called_once_with(self.node_info, self.node, self.data) + def test_ipmi_not_detected_with_old_field(self): + self.inventory['bmc_address'] = '0.0.0.0' + self.data['ipmi_address'] = '0.0.0.0' + process.process(self.data) + + self.find_mock.assert_called_once_with(bmc_address=None, mac=mock.ANY) + actual_macs = self.find_mock.call_args[1]['mac'] + self.assertEqual(sorted(self.all_macs), sorted(actual_macs)) + self.cli.node.get.assert_called_once_with(self.uuid) + self.process_mock.assert_called_once_with(self.node_info, self.node, + self.data) + def test_not_found_in_cache(self): self.find_mock.side_effect = utils.Error('not found') self.assertRaisesRegex(utils.Error, diff --git a/releasenotes/notes/empty-ipmi-address-2-4d57c34aec7d14e2.yaml b/releasenotes/notes/empty-ipmi-address-2-4d57c34aec7d14e2.yaml new file mode 100644 index 000000000..6588769ef --- /dev/null +++ b/releasenotes/notes/empty-ipmi-address-2-4d57c34aec7d14e2.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + The older ``ipmi_address`` field in the introspection data no longer has + priority over the newer ``bmc_address`` inventory field during lookup. + This fixes lookup based on MAC addresses, when the BMC address is reported + as ``0.0.0.0`` for any reason (see `bug 1714944 + `_).