Fix ironic port creation after Redfish inspection

Change non-existent ``eth_summary`` property to ``summary`` on
ethernet_interfaces resource of Sushy.

Also amended the helper method, ``create_ports_if_not_exist`` in
inspect_utils module to handle Sushy returned ethernet summary
results.

Change-Id: I59a3d269c3508cc1e86d0ad5af3707585b108fa0
Story: 2004638
Task: 28574

6 files changed, 67 insertions, 29 deletions

diff --git a/ironic/drivers/modules/inspect_utils.py b/ironic/drivers/modules/inspect_utils.py
index 8a7f6f557..41390b63a 100644
--- a/ironic/drivers/modules/inspect_utils.py
+++ b/ironic/drivers/modules/inspect_utils.py
@@ -21,31 +21,32 @@ from ironic import objects
 LOG = logging.getLogger(__name__)
 
 
-def create_ports_if_not_exist(task, macs):
-    """Create ironic ports for the mac addresses.
+def create_ports_if_not_exist(
+        task, macs, get_mac_address=lambda x: x[1]):
+    """Create ironic ports from MAC addresses data dict.
 
-    Creates ironic ports for the mac addresses returned with inspection
-    or as requested by operator.
+    Creates ironic ports from MAC addresses data returned with inspection or
+    as requested by operator. Helper argument to detect the MAC address
+    ``get_mac_address`` defaults to 'value' part of MAC address dict key-value
+    pair.
 
     :param task: A TaskManager instance.
-    :param macs: A dictionary of port numbers to mac addresses
-                 returned by node inspection.
-
+    :param macs: A dictionary of MAC addresses returned by node inspection.
+    :param get_mac_address: a function to get the MAC address from mac item.
+        A mac item is the dict key-value pair of the previous ``macs``
+        argument.
     """
     node = task.node
-    for port_num, mac in macs.items():
-        # TODO(etingof): detect --pxe-enabled flag
+    for k_v_pair in macs.items():
+        mac = get_mac_address(k_v_pair)
         port_dict = {'address': mac, 'node_id': node.id}
         port = objects.Port(task.context, **port_dict)
 
         try:
             port.create()
-            LOG.info("Port %(port_num)s created for MAC address %(address)s "
-                     "for node %(node)s", {'address': mac, 'node': node.uuid,
-                                           'port_num': port_num})
+            LOG.info("Port created for MAC address %(address)s for node "
+                     "%(node)s", {'address': mac, 'node': node.uuid})
         except exception.MACAlreadyExists:
-            LOG.warning("Port %(port_num)s already exists for "
-                        "MAC address %(address)s for node "
-                        "%(node)s", {'address': mac,
-                                     'node': node.uuid,
-                                     'port_num': port_num})
+            LOG.warning("Port already exists for MAC address %(address)s "
+                        "for node %(node)s",
+                        {'address': mac, 'node': node.uuid})
diff --git a/ironic/drivers/modules/redfish/inspect.py b/ironic/drivers/modules/redfish/inspect.py
index a9f625a5a..7f26b4722 100644
--- a/ironic/drivers/modules/redfish/inspect.py
+++ b/ironic/drivers/modules/redfish/inspect.py
@@ -205,12 +205,21 @@ class RedfishInspect(base.InspectInterface):
                                      'node': task.node.uuid})
 
         if (system.ethernet_interfaces and
-                system.ethernet_interfaces.eth_summary):
-            macs = system.ethernet_interfaces.eth_summary
-
-            # Create ports for the nics detected.
-            inspect_utils.create_ports_if_not_exist(task, macs)
-
+                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.info("Not attempted to create any port as no NICs being "
+                         "discovered in 'enabled' state for node %(node)s: "
+                         "%(mac_data)s", {'mac_data': macs,
+                                          'node': task.node.uuid})
         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_inspect.py b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py
index af90ba3f1..d07b8ae40 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_inspect.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py
@@ -58,8 +58,9 @@ class RedfishInspectTestCase(db_base.DbTestCase):
         system_mock.storage.volumes_sizes_bytes = (
             2 * units.Gi, units.Gi * 4, units.Gi * 6)
 
-        system_mock.ethernet_interfaces.eth_summary = {
-            '1': '00:11:22:33:44:55', '2': '66:77:88:99:AA:BB'
+        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
@@ -88,13 +89,12 @@ class RedfishInspectTestCase(db_base.DbTestCase):
             'local_gb': '3', 'memory_mb': '2048'
         }
 
-        system = self.init_system_mock(mock_get_system.return_value)
+        self.init_system_mock(mock_get_system.return_value)
 
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             task.driver.inspect.inspect_hardware(task)
-            mock_create_ports_if_not_exist.assert_called_once_with(
-                task, system.ethernet_interfaces.eth_summary)
+            self.assertEqual(1, mock_create_ports_if_not_exist.call_count)
             mock_get_system.assert_called_once_with(task.node)
             self.assertEqual(expected_properties, task.node.properties)
 
@@ -169,7 +169,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
     def test_inspect_hardware_ignore_missing_nics(
             self, mock_create_ports_if_not_exist, mock_get_system):
         system_mock = self.init_system_mock(mock_get_system.return_value)
-        system_mock.ethernet_interfaces.eth_summary = None
+        system_mock.ethernet_interfaces.summary = None
 
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
diff --git a/ironic/tests/unit/drivers/modules/test_inspect_utils.py b/ironic/tests/unit/drivers/modules/test_inspect_utils.py
index 9bbe8c3d0..c43e996ba 100644
--- a/ironic/tests/unit/drivers/modules/test_inspect_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_inspect_utils.py
@@ -15,6 +15,7 @@
 
 
 import mock
+from oslo_utils import importutils
 
 from ironic.common import exception
 from ironic.conductor import task_manager
@@ -23,6 +24,8 @@ from ironic import objects
 from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.objects import utils as obj_utils
 
+sushy = importutils.try_import('sushy')
+
 
 @mock.patch('time.sleep', lambda sec: None)
 class InspectFunctionTestCase(db_base.DbTestCase):
@@ -63,3 +66,22 @@ class InspectFunctionTestCase(db_base.DbTestCase):
                                   shared=False) as task:
             utils.create_ports_if_not_exist(task, macs)
         self.assertEqual(2, log_mock.call_count)
+
+    @mock.patch.object(utils.LOG, 'info', spec_set=True, autospec=True)
+    @mock.patch.object(objects, 'Port', spec_set=True, autospec=True)
+    def test_create_ports_if_not_exist_attempts_port_creation_blindly(
+            self, port_mock, log_info_mock):
+        macs = {'aa:bb:cc:dd:ee:ff': sushy.STATE_ENABLED,
+                'aa:bb:aa:aa:aa:aa': sushy.STATE_DISABLED}
+        node_id = self.node.id
+        port_dict1 = {'address': 'aa:bb:cc:dd:ee:ff', 'node_id': node_id}
+        port_dict2 = {'address': 'aa:bb:aa:aa:aa:aa', 'node_id': node_id}
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            utils.create_ports_if_not_exist(
+                task, macs, get_mac_address=lambda x: x[0])
+            self.assertTrue(log_info_mock.called)
+            expected_calls = [mock.call(task.context, **port_dict1),
+                              mock.call(task.context, **port_dict2)]
+            port_mock.assert_has_calls(expected_calls, any_order=True)
+            self.assertEqual(2, port_mock.return_value.create.call_count)
diff --git a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py
index 1511064a4..6a5f906af 100644
--- a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py
+++ b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py
@@ -142,6 +142,9 @@ SUSHY_SPEC = (
     'PROCESSOR_ARCH_ARM',
     'PROCESSOR_ARCH_MIPS',
     'PROCESSOR_ARCH_OEM',
+    'STATE_ENABLED',
+    'STATE_DISABLED',
+    'STATE_ABSENT',
 )
 
 SUSHY_AUTH_SPEC = (
diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py
index 56c3a3878..568a65deb 100644
--- a/ironic/tests/unit/drivers/third_party_driver_mocks.py
+++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py
@@ -224,6 +224,9 @@ if not sushy:
         PROCESSOR_ARCH_ARM='ARM',
         PROCESSOR_ARCH_MIPS='MIPS',
         PROCESSOR_ARCH_OEM='OEM-defined',
+        STATE_ENABLED='enabled',
+        STATE_DISABLED='disabled',
+        STATE_ABSENT='absent',
     )
 
     sys.modules['sushy'] = sushy