From 05df3d7aa4aa7a1fd25a2f2d55726197e1b5f9df Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Mon, 8 Feb 2021 15:44:36 +1000 Subject: [PATCH] Use OOB inspection to fetch MACs for IB inspection This change adds get_mac_addresses call to the ManagementInterface which will be used by both out-of-band inspection and in-band inspection with ironic-inspector. This will remove the necessity of manually defining MAC addresses for nodes and/or enabling IPMI functionality on Redfish-based systems. Change-Id: I3debcd1f32a2627dafd8456ec73a71fc7c402ebb Story: 2008038 Task: 40699 --- ironic/drivers/base.py | 11 ++++ ironic/drivers/modules/inspector.py | 18 +++++- ironic/drivers/modules/redfish/inspect.py | 26 +++------ ironic/drivers/modules/redfish/management.py | 19 +++++++ ironic/drivers/modules/redfish/utils.py | 23 ++++++++ .../modules/redfish/test_management.py | 45 +++++++++++++++ .../unit/drivers/modules/test_inspector.py | 56 ++++++++++++++++--- ironic/tests/unit/drivers/test_base.py | 7 +++ ...ish-autocreate-ports-53712a46dadd8203.yaml | 6 ++ 9 files changed, 183 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/redfish-autocreate-ports-53712a46dadd8203.yaml diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 7d137f9c4d..2ca07508cb 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -1159,6 +1159,17 @@ class ManagementInterface(BaseInterface): raise exception.UnsupportedDriverExtension( driver=task.node.driver, extension='detect_vendor') + def get_mac_addresses(self, task): + """Get MAC address information for the node. + + :param task: A TaskManager instance containing the node to act on. + :raises: UnsupportedDriverExtension + :returns: A list of MAC addresses for the node + + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='get_mac_addresses') + class InspectInterface(BaseInterface): """Interface for inspection-related actions.""" diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 0da29c63cd..fa60412e29 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -34,7 +34,7 @@ from ironic.conductor import utils as cond_utils from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules import deploy_utils - +from ironic.drivers.modules import inspect_utils LOG = logging.getLogger(__name__) @@ -243,6 +243,22 @@ class Inspector(base.InspectInterface): :returns: states.INSPECTWAIT :raises: HardwareInspectionFailure on failure """ + try: + enabled_macs = task.driver.management.get_mac_addresses(task) + if enabled_macs: + inspect_utils.create_ports_if_not_exist( + task, enabled_macs, get_mac_address=lambda x: x[0]) + else: + LOG.warning("Not attempting to create any port as no NICs " + "were discovered in 'enabled' state for node " + "%(node)s: %(mac_data)s", + {'mac_data': enabled_macs, + 'node': task.node.uuid}) + + except exception.UnsupportedDriverExtension: + LOG.info('Pre-creating ports prior to inspection not supported' + ' on node %s.', task.node.uuid) + ironic_manages_boot = _ironic_manages_boot( task, raise_exc=CONF.inspector.require_managed_boot) diff --git a/ironic/drivers/modules/redfish/inspect.py b/ironic/drivers/modules/redfish/inspect.py index a0d7cf4859..10344c95db 100644 --- a/ironic/drivers/modules/redfish/inspect.py +++ b/ironic/drivers/modules/redfish/inspect.py @@ -160,25 +160,15 @@ class RedfishInspect(base.InspectInterface): return states.MANAGEABLE def _create_ports(self, task, system): - if (system.ethernet_interfaces - and system.ethernet_interfaces.summary): - macs = system.ethernet_interfaces.summary - - # Create ports for the discovered NICs being in 'enabled' state - enabled_macs = {nic_mac: nic_state - for nic_mac, nic_state in macs.items() - if nic_state == sushy.STATE_ENABLED} - if enabled_macs: - inspect_utils.create_ports_if_not_exist( - task, enabled_macs, get_mac_address=lambda x: x[0]) - else: - LOG.warning("Not attempting to create any port as no NICs " - "were discovered in 'enabled' state for node " - "%(node)s: %(mac_data)s", - {'mac_data': macs, 'node': task.node.uuid}) + enabled_macs = redfish_utils.get_enabled_macs(task, system) + if enabled_macs: + inspect_utils.create_ports_if_not_exist( + task, enabled_macs, get_mac_address=lambda x: x[0]) else: - LOG.warning("No NIC information discovered " - "for node %(node)s", {'node': task.node.uuid}) + LOG.warning("Not attempting to create any port as no NICs " + "were discovered in 'enabled' state for node " + "%(node)s: %(mac_data)s", + {'mac_data': enabled_macs, 'node': task.node.uuid}) def _detect_local_gb(self, task, system): simple_storage_size = 0 diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index 1188291027..70d64cd57b 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -1148,3 +1148,22 @@ class RedfishManagement(base.ManagementInterface): self._reset_keys(task, sushy.SECURE_BOOT_RESET_KEYS_DELETE_ALL) LOG.info('Secure boot keys have been removed from node %s', task.node.uuid) + + def get_mac_addresses(self, task): + """Get MAC address information for the node. + + :param task: A TaskManager instance containing the node to act on. + :raises: RedfishConnectionError when it fails to connect to Redfish + :raises: RedfishError on an error from the Sushy library + :returns: a dictionary containing MAC addresses of enabled interfaces + in a {'mac': 'state'} format + """ + try: + system = redfish_utils.get_system(task.node) + return redfish_utils.get_enabled_macs(task, system) + except sushy.exceptions.SushyError as exc: + msg = (_('Failed to get network interface information on node ' + '%(node)s: %(exc)s') + % {'node': task.node.uuid, 'exc': exc}) + LOG.error(msg) + raise exception.RedfishError(error=msg) diff --git a/ironic/drivers/modules/redfish/utils.py b/ironic/drivers/modules/redfish/utils.py index 705fc997e2..fde450d25c 100644 --- a/ironic/drivers/modules/redfish/utils.py +++ b/ironic/drivers/modules/redfish/utils.py @@ -330,3 +330,26 @@ def _get_connection(node, lambda_fun, *args): 'node %(node)s. Error: %(error)s', {'address': driver_info['address'], 'node': node.uuid, 'error': e}) + + +def get_enabled_macs(task, system): + """Get information on MAC addresses of enabled ports using Redfish. + + :param task: a TaskManager instance containing the node to act on. + :param system: a Redfish System object + :returns: a dictionary containing MAC addresses of enabled interfaces + in a {'mac': 'state'} format + """ + + if (system.ethernet_interfaces + and system.ethernet_interfaces.summary): + macs = system.ethernet_interfaces.summary + + # Identify ports for the NICs being in 'enabled' state + enabled_macs = {nic_mac: nic_state + for nic_mac, nic_state in macs.items() + if nic_state == sushy.STATE_ENABLED} + return enabled_macs + else: + LOG.warning("No NIC information discovered " + "for node %(node)s", {'node': task.node.uuid}) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index e0f174c849..fb397e6156 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -17,6 +17,7 @@ import datetime from unittest import mock from oslo_utils import importutils +from oslo_utils import units from ironic.common import boot_devices from ironic.common import boot_modes @@ -56,6 +57,28 @@ class RedfishManagementTestCase(db_base.DbTestCase): self.chassis_uuid = 'XXX-YYY-ZZZ' self.drive_uuid = 'ZZZ-YYY-XXX' + def init_system_mock(self, system_mock, **properties): + + system_mock.reset() + + system_mock.boot.mode = 'uefi' + + system_mock.memory_summary.size_gib = 2 + + system_mock.processors.summary = '8', 'MIPS' + + system_mock.simple_storage.disks_sizes_bytes = ( + 1 * units.Gi, units.Gi * 3, units.Gi * 5) + system_mock.storage.volumes_sizes_bytes = ( + 2 * units.Gi, units.Gi * 4, units.Gi * 6) + + system_mock.ethernet_interfaces.summary = { + '00:11:22:33:44:55': sushy.STATE_ENABLED, + '66:77:88:99:AA:BB': sushy.STATE_DISABLED, + } + + return system_mock + @mock.patch.object(redfish_mgmt, 'sushy', None) def test_loading_error(self): self.assertRaisesRegex( @@ -1446,3 +1469,25 @@ class RedfishManagementTestCase(db_base.DbTestCase): self.assertRaises( exception.UnsupportedDriverExtension, task.driver.management.clear_secure_boot_keys, task) + + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_get_mac_addresses_success(self, mock_get_system): + expected_properties = {'00:11:22:33:44:55': 'enabled'} + + self.init_system_mock(mock_get_system.return_value) + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.assertEqual(expected_properties, + task.driver.management.get_mac_addresses(task)) + + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_get_mac_addresses_no_ports_found(self, mock_get_system): + expected_properties = None + + system_mock = self.init_system_mock(mock_get_system.return_value) + system_mock.ethernet_interfaces.summary = None + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.assertEqual(expected_properties, + task.driver.management.get_mac_addresses(task)) diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py index 6b2313daba..b318049b8c 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/test_inspector.py @@ -20,7 +20,9 @@ from ironic.common import exception from ironic.common import states from ironic.common import utils from ironic.conductor import task_manager +from ironic.drivers.modules import inspect_utils from ironic.drivers.modules import inspector +from ironic.drivers.modules.redfish import utils as redfish_utils from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils @@ -71,7 +73,7 @@ class BaseTestCase(db_base.DbTestCase): self.task.shared = False self.task.node = self.node self.task.driver = mock.Mock( - spec=['boot', 'network', 'inspect', 'power'], + spec=['boot', 'network', 'inspect', 'power', 'management'], inspect=self.iface) self.driver = self.task.driver @@ -126,14 +128,23 @@ class InspectHardwareTestCase(BaseTestCase): self.assertRaises(exception.InvalidParameterValue, self.iface.validate, self.task) - def test_validate_require_managed_boot(self, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_validate_require_managed_boot(self, mock_get_system, + mock_create_ports_if_not_exist, + mock_client): CONF.set_override('require_managed_boot', True, group='inspector') self.driver.boot.validate_inspection.side_effect = ( exception.UnsupportedDriverExtension('')) self.assertRaises(exception.UnsupportedDriverExtension, self.iface.validate, self.task) - def test_unmanaged_ok(self, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_unmanaged_ok(self, mock_create_ports_if_not_exist, + mock_get_system, mock_client): self.driver.boot.validate_inspection.side_effect = ( exception.UnsupportedDriverExtension('')) mock_introspect = mock_client.return_value.start_introspection @@ -148,7 +159,11 @@ class InspectHardwareTestCase(BaseTestCase): self.assertFalse(self.driver.power.set_power_state.called) @mock.patch.object(task_manager, 'acquire', autospec=True) - def test_unmanaged_error(self, mock_acquire, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_unmanaged_error(self, mock_create_ports_if_not_exist, + mock_get_system, mock_acquire, mock_client): mock_acquire.return_value.__enter__.return_value = self.task self.driver.boot.validate_inspection.side_effect = ( exception.UnsupportedDriverExtension('')) @@ -164,7 +179,11 @@ class InspectHardwareTestCase(BaseTestCase): self.assertFalse(self.driver.boot.clean_up_ramdisk.called) self.assertFalse(self.driver.power.set_power_state.called) - def test_require_managed_boot(self, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_require_managed_boot(self, mock_create_ports_if_not_exist, + mock_get_system, mock_client): CONF.set_override('require_managed_boot', True, group='inspector') self.driver.boot.validate_inspection.side_effect = ( exception.UnsupportedDriverExtension('')) @@ -179,7 +198,11 @@ class InspectHardwareTestCase(BaseTestCase): self.assertFalse(self.driver.boot.clean_up_ramdisk.called) self.assertFalse(self.driver.power.set_power_state.called) - def test_managed_ok(self, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_managed_ok(self, mock_create_ports_if_not_exist, + mock_get_system, mock_client): endpoint = 'http://192.169.0.42:5050/v1' mock_client.return_value.get_endpoint.return_value = endpoint mock_introspect = mock_client.return_value.start_introspection @@ -200,7 +223,12 @@ class InspectHardwareTestCase(BaseTestCase): self.assertFalse(self.driver.network.remove_inspection_network.called) self.assertFalse(self.driver.boot.clean_up_ramdisk.called) - def test_managed_custom_params(self, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_managed_custom_params(self, mock_get_system, + mock_create_ports_if_not_exist, + mock_client): CONF.set_override('extra_kernel_params', 'ipa-inspection-collectors=default,logs ' 'ipa-collect-dhcp=1', @@ -230,7 +258,12 @@ class InspectHardwareTestCase(BaseTestCase): @mock.patch('ironic.drivers.modules.deploy_utils.get_ironic_api_url', autospec=True) - def test_managed_fast_track(self, mock_ironic_url, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_managed_fast_track(self, mock_create_ports_if_not_exist, + mock_get_system, mock_ironic_url, + mock_client): CONF.set_override('fast_track', True, group='deploy') CONF.set_override('extra_kernel_params', 'ipa-inspection-collectors=default,logs ' @@ -262,7 +295,12 @@ class InspectHardwareTestCase(BaseTestCase): self.assertFalse(self.driver.boot.clean_up_ramdisk.called) @mock.patch.object(task_manager, 'acquire', autospec=True) - def test_managed_error(self, mock_acquire, mock_client): + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', + autospec=True) + def test_managed_error(self, mock_get_system, + mock_create_ports_if_not_exist, mock_acquire, + mock_client): endpoint = 'http://192.169.0.42:5050/v1' mock_client.return_value.get_endpoint.return_value = endpoint mock_acquire.return_value.__enter__.return_value = self.task diff --git a/ironic/tests/unit/drivers/test_base.py b/ironic/tests/unit/drivers/test_base.py index d1e33b9b1c..2d41174b1a 100644 --- a/ironic/tests/unit/drivers/test_base.py +++ b/ironic/tests/unit/drivers/test_base.py @@ -831,6 +831,13 @@ class TestManagementInterface(base.TestCase): expected, management.get_indicator_state( task_mock, components.CHASSIS, 'led-0')) + def test_get_mac_addresses(self): + management = fake.FakeManagement() + task_mock = mock.MagicMock(spec_set=['node']) + + self.assertRaises(exception.UnsupportedDriverExtension, + management.get_mac_addresses, task_mock) + class TestBareDriver(base.TestCase): diff --git a/releasenotes/notes/redfish-autocreate-ports-53712a46dadd8203.yaml b/releasenotes/notes/redfish-autocreate-ports-53712a46dadd8203.yaml new file mode 100644 index 0000000000..4f3ce50369 --- /dev/null +++ b/releasenotes/notes/redfish-autocreate-ports-53712a46dadd8203.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds support for automatic creation of ports for ``redfish`` enabled + bare metal nodes using prior to ironic-inspector introspection. This + feature is a part of ``redfish`` management interface.