Merge "Use OOB inspection to fetch MACs for IB inspection"

ironic/drivers/base.py

@@ -1161,6 +1161,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."""

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)
 

ironic/drivers/modules/redfish/inspect.py

@@ -160,14 +160,7 @@ class RedfishInspect(base.InspectInterface):
         return states.MANAGEABLE
 
     def _create_ports(self, task, system):
-        if (system.ethernet_interfaces
+        enabled_macs = redfish_utils.get_enabled_macs(task, system)
-                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])
@@ -175,10 +168,7 @@ class RedfishInspect(base.InspectInterface):
             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})
+                        {'mac_data': enabled_macs, 'node': task.node.uuid})
-        else:
-            LOG.warning("No NIC information discovered "
-                        "for node %(node)s", {'node': task.node.uuid})
 
     def _detect_local_gb(self, task, system):
         simple_storage_size = 0

ironic/drivers/modules/redfish/management.py

@@ -1161,3 +1161,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)

ironic/drivers/modules/redfish/utils.py

@@ -349,3 +349,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})

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(
@@ -1478,3 +1501,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))

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

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):
 

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.