diff --git a/ironic/drivers/modules/drac/inspect.py b/ironic/drivers/modules/drac/inspect.py index aa2ad3dae1..8ba0be088b 100644 --- a/ironic/drivers/modules/drac/inspect.py +++ b/ironic/drivers/modules/drac/inspect.py @@ -68,7 +68,7 @@ class DracRedfishInspect(redfish_inspect.RedfishInspect): # versions of the firmware where the port state is not being # reported correctly. - ethernet_interfaces_mac = self._get_mac_address(task) + ethernet_interfaces_mac = list(self._get_mac_address(task).values()) inspect_utils.create_ports_if_not_exist(task, ethernet_interfaces_mac) return super(DracRedfishInspect, self).inspect_hardware(task) @@ -76,7 +76,7 @@ class DracRedfishInspect(redfish_inspect.RedfishInspect): """Get a list of MAC addresses :param task: a TaskManager instance. - :returns: Returns list of MAC addresses. + :returns: a mapping of interface identities to MAC addresses. """ system = redfish_utils.get_system(task.node) # Get dictionary of ethernet interfaces diff --git a/ironic/drivers/modules/ilo/inspect.py b/ironic/drivers/modules/ilo/inspect.py index 7f79d3bcdb..b279bd4718 100644 --- a/ironic/drivers/modules/ilo/inspect.py +++ b/ironic/drivers/modules/ilo/inspect.py @@ -299,7 +299,8 @@ class IloInspect(base.InspectInterface): task.node.save() # Create ports for the nics detected. - inspect_utils.create_ports_if_not_exist(task, result['macs']) + inspect_utils.create_ports_if_not_exist( + task, list(result['macs'].values())) LOG.debug("Node properties for %(node)s are updated as " "%(properties)s", diff --git a/ironic/drivers/modules/inspect_utils.py b/ironic/drivers/modules/inspect_utils.py index 05aae921ee..911c5b4025 100644 --- a/ironic/drivers/modules/inspect_utils.py +++ b/ironic/drivers/modules/inspect_utils.py @@ -21,8 +21,7 @@ from ironic import objects LOG = logging.getLogger(__name__) -def create_ports_if_not_exist( - task, macs, get_mac_address=lambda x: x[1]): +def create_ports_if_not_exist(task, macs): """Create ironic ports from MAC addresses data dict. Creates ironic ports from MAC addresses data returned with inspection or @@ -31,14 +30,10 @@ def create_ports_if_not_exist( pair. :param task: A TaskManager instance. - :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. + :param macs: A sequence of MAC addresses. """ node = task.node - for k_v_pair in macs.items(): - mac = get_mac_address(k_v_pair) + for mac in macs: port_dict = {'address': mac, 'node_id': node.id} port = objects.Port(task.context, **port_dict) diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 1b866d0d5c..fe27c46e5e 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -246,8 +246,7 @@ class Inspector(base.InspectInterface): 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]) + inspect_utils.create_ports_if_not_exist(task, enabled_macs) else: LOG.warning("Not attempting to create any port as no NICs " "were discovered in 'enabled' state for node " diff --git a/ironic/drivers/modules/redfish/inspect.py b/ironic/drivers/modules/redfish/inspect.py index 4e968e4b36..4c5f7c3448 100644 --- a/ironic/drivers/modules/redfish/inspect.py +++ b/ironic/drivers/modules/redfish/inspect.py @@ -189,8 +189,7 @@ class RedfishInspect(base.InspectInterface): def _create_ports(self, task, system): 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]) + inspect_utils.create_ports_if_not_exist(task, list(enabled_macs)) else: LOG.warning("Not attempting to create any port as no NICs " "were discovered in 'enabled' state for node " diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index 29844fae90..5a94d1564d 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -1114,12 +1114,11 @@ class RedfishManagement(base.ManagementInterface): :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 + :returns: A list of MAC addresses for the node """ try: system = redfish_utils.get_system(task.node) - return redfish_utils.get_enabled_macs(task, system) + return list(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') diff --git a/ironic/drivers/modules/redfish/utils.py b/ironic/drivers/modules/redfish/utils.py index 607de4366d..a590494ae1 100644 --- a/ironic/drivers/modules/redfish/utils.py +++ b/ironic/drivers/modules/redfish/utils.py @@ -381,7 +381,7 @@ def get_enabled_macs(task, system): :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 + in a {'mac': } format, where is a sushy constant """ enabled_macs = {} @@ -398,11 +398,11 @@ def get_enabled_macs(task, system): "reported", {'node': task.node.uuid}) continue enabled_macs[nic_mac] = nic_state - if enabled_macs: - return enabled_macs - LOG.debug("No ethernet interface information is available " - "for node %(node)s", {'node': task.node.uuid}) + if not enabled_macs: + LOG.debug("No ethernet interface information is available " + "for node %(node)s", {'node': task.node.uuid}) + return enabled_macs def wait_until_get_system_ready(node): diff --git a/ironic/tests/unit/drivers/modules/drac/test_inspect.py b/ironic/tests/unit/drivers/modules/drac/test_inspect.py index 97dcf30f9d..d12adba34f 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/drac/test_inspect.py @@ -652,7 +652,7 @@ class DracRedfishInspectionTestCase(test_utils.BaseDracTest): return_value = task.driver.inspect.inspect_hardware(task) self.assertEqual(states.MANAGEABLE, return_value) mock_create_ports_if_not_exist.assert_called_once_with( - task, ethernet_interfaces_mac) + task, ['24:6E:96:70:49:00']) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test__get_mac_address_with_ethernet_interfaces(self, mock_get_system): diff --git a/ironic/tests/unit/drivers/modules/ilo/test_inspect.py b/ironic/tests/unit/drivers/modules/ilo/test_inspect.py index 376c0d601d..1a85d5b282 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_inspect.py @@ -81,7 +81,8 @@ class IloInspectTestCase(test_common.BaseIloTest): ilo_object_mock) get_capabilities_mock.assert_called_once_with(task.node, ilo_object_mock) - create_port_mock.assert_called_once_with(task, macs) + create_port_mock.assert_called_once_with( + task, ['aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb']) @mock.patch.object(ilo_inspect.LOG, 'warning', spec_set=True, autospec=True) @@ -126,7 +127,8 @@ class IloInspectTestCase(test_common.BaseIloTest): self.assertTrue(log_mock.called) get_capabilities_mock.assert_called_once_with(task.node, ilo_object_mock) - create_port_mock.assert_called_once_with(task, macs) + create_port_mock.assert_called_once_with( + task, ['aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb']) @mock.patch.object(ilo_inspect.LOG, 'warning', spec_set=True, autospec=True) @@ -168,7 +170,8 @@ class IloInspectTestCase(test_common.BaseIloTest): self.assertTrue(log_mock.called) get_capabilities_mock.assert_called_once_with(task.node, ilo_object_mock) - create_port_mock.assert_called_once_with(task, macs) + create_port_mock.assert_called_once_with( + task, ['aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb']) @mock.patch.object(ilo_inspect.LOG, 'warning', spec_set=True, autospec=True) @@ -217,7 +220,8 @@ class IloInspectTestCase(test_common.BaseIloTest): self.assertFalse(log_mock.called) get_capabilities_mock.assert_called_once_with(task.node, ilo_object_mock) - create_port_mock.assert_called_once_with(task, macs) + create_port_mock.assert_called_once_with( + task, ['aa:aa:aa:aa:aa:aa']) @mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True, autospec=True) @@ -256,7 +260,8 @@ class IloInspectTestCase(test_common.BaseIloTest): ilo_object_mock) get_capabilities_mock.assert_called_once_with(task.node, ilo_object_mock) - create_port_mock.assert_called_once_with(task, macs) + create_port_mock.assert_called_once_with( + task, ['aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb']) @mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True, autospec=True) @@ -295,7 +300,8 @@ class IloInspectTestCase(test_common.BaseIloTest): ilo_object_mock) get_capabilities_mock.assert_called_once_with(task.node, ilo_object_mock) - create_port_mock.assert_called_once_with(task, macs) + create_port_mock.assert_called_once_with( + task, ['aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb']) @mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True, autospec=True) @@ -341,7 +347,8 @@ class IloInspectTestCase(test_common.BaseIloTest): ilo_object_mock) get_capabilities_mock.assert_called_once_with(task.node, ilo_object_mock) - create_port_mock.assert_called_once_with(task, macs) + create_port_mock.assert_called_once_with( + task, ['aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb']) class TestInspectPrivateMethods(test_common.BaseIloTest): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_inspect.py b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py index de084c3414..352b1d4e98 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py @@ -118,7 +118,7 @@ class RedfishInspectTestCase(db_base.DbTestCase): task.driver.inspect.inspect_hardware(task) result = task.driver.management.get_mac_addresses(task) inspect_utils.create_ports_if_not_exist.assert_called_once_with( - task, result, mock.ANY) + task, result) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_inspect_hardware_fail_missing_cpu(self, mock_get_system): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index 3e7d1a1d45..f67efa9f07 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -1362,13 +1362,11 @@ class RedfishManagementTestCase(db_base.DbTestCase): @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, + self.assertEqual(['00:11:22:33:44:55'], task.driver.management.get_mac_addresses(task)) @mock.patch.object(redfish_utils, 'get_system', autospec=True) @@ -1378,4 +1376,5 @@ class RedfishManagementTestCase(db_base.DbTestCase): system_mock.ethernet_interfaces.summary = None with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - self.assertIsNone(task.driver.management.get_mac_addresses(task)) + self.assertEqual([], + task.driver.management.get_mac_addresses(task)) diff --git a/ironic/tests/unit/drivers/modules/test_inspect_utils.py b/ironic/tests/unit/drivers/modules/test_inspect_utils.py index aa7bd79be6..3c443c3fcc 100644 --- a/ironic/tests/unit/drivers/modules/test_inspect_utils.py +++ b/ironic/tests/unit/drivers/modules/test_inspect_utils.py @@ -39,7 +39,7 @@ class InspectFunctionTestCase(db_base.DbTestCase): @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(self, port_mock, log_mock): - macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'} + macs = {'aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb'} node_id = self.node.id port_dict1 = {'address': 'aa:aa:aa:aa:aa:aa', 'node_id': node_id} port_dict2 = {'address': 'bb:bb:bb:bb:bb:bb', 'node_id': node_id} @@ -61,7 +61,7 @@ class InspectFunctionTestCase(db_base.DbTestCase): create_mock, log_mock): create_mock.side_effect = exception.MACAlreadyExists('f') - macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'} + macs = {'aa:aa:aa:aa:aa:aa', 'bb:bb:bb:bb:bb:bb'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: utils.create_ports_if_not_exist(task, macs) @@ -71,15 +71,13 @@ class InspectFunctionTestCase(db_base.DbTestCase): @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} + macs = {'aa:bb:cc:dd:ee:ff', 'aa:bb:aa:aa:aa:aa'} 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]) + utils.create_ports_if_not_exist(task, macs) self.assertTrue(log_info_mock.called) expected_calls = [mock.call(task.context, **port_dict1), mock.call(task.context, **port_dict2)]