From af0b9751d2e2835b035867db1937b99443dfecec Mon Sep 17 00:00:00 2001 From: Iury Gregory Melo Ferreira Date: Mon, 22 Sep 2025 11:42:53 -0300 Subject: [PATCH] Force Virtual Media Slot 1 on iDRAC10 iDRAC10 requires using the Virtual Media Slot1, the problem is that some bmcs doesn't return the resrouces ordered, this causes the provisioning to fail because we inserted the iso in the wrong slot. This patch adds support to detect the version of idrac, this is based on redfish information acessible via manager.model. With the information about the idrac version, we can force ironic to use a specific Virtual Media slot. Closes-Bug: #2125571 Assisted-By: Claude Code - Claude Sonnet 4 Change-Id: I38c8286c644d93a6f16137bd73f6e267948642b1 Signed-off-by: Iury Gregory Melo Ferreira --- ironic/drivers/modules/drac/boot.py | 53 +++++++++++ ironic/drivers/modules/redfish/boot.py | 37 ++++++++ .../unit/drivers/modules/drac/test_boot.py | 81 ++++++++++++++++ .../unit/drivers/modules/redfish/test_boot.py | 93 +++++++++++++++++++ ...idrac10-vmedia-slot1-5d15433b84843a10.yaml | 8 ++ 5 files changed, 272 insertions(+) create mode 100644 releasenotes/notes/idrac10-vmedia-slot1-5d15433b84843a10.yaml diff --git a/ironic/drivers/modules/drac/boot.py b/ironic/drivers/modules/drac/boot.py index dd91ad5e2e..0420e255b2 100644 --- a/ironic/drivers/modules/drac/boot.py +++ b/ironic/drivers/modules/drac/boot.py @@ -69,6 +69,59 @@ class DracRedfishVirtualMediaBoot(redfish_boot.RedfishVirtualMediaBoot): def _validate_vendor(self, task, managers): pass # assume people are doing the right thing + @staticmethod + def _get_idrac_version_from_model(model): + """Extract iDRAC version from the hardware model string. + + :param model: The hardware model string from the manager. + :returns: The iDRAC version as an integer, or None if unable to + determine. + """ + if not model: + return None + + try: + generation = int(model[:2]) + # Map hardware generation to iDRAC version + if generation > 16: + return 10 # iDRAC 10 + elif generation in (16, 15, 14): + return 9 # iDRAC 9 + elif generation in (12, 13): + return 8 # iDRAC 8 + else: + return None # Unknown or unsupported version + except (ValueError, TypeError): + LOG.debug("Unable to parse iDRAC version from model string: %s", + model) + return None + + def _get_acceptable_media_id(self, task, resource): + """Get acceptable virtual media IDs for iDRAC systems. + + For iDRAC10 systems, only virtual media ID "1" is acceptable + for virtual media insertion due to hardware limitations. + + :param task: A TaskManager instance containing the node to act on. + :param resource: A redfish resource (System or Manager) containing + virtual media. + :returns: "1" for iDRAC10 systems, None otherwise. + """ + # In case the resource is System, we need to check the managers + if resource.managers: + for manager in resource.managers: + # Check the iDRAC version based on the hardware model + if manager.model: + idrac_version = self._get_idrac_version_from_model( + manager.model) + if idrac_version == 10: + return "1" + return None + else: + # In case the resource is Manager, we don't need to check anything. + # iDRAC10 doesn't have Virtual Media in Managers, so we ignore. + return None + @classmethod def _set_boot_device(cls, task, device, persistent=False): """Set boot device for a node. diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index 2ecdba9e13..1a4a0c5372 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -301,7 +301,30 @@ def _insert_vmedia_in_resource(task, resource, boot_url, boot_device, :raises: InvalidParameterValue, if no suitable virtual CD or DVD is found on the node. """ + # Get acceptable virtual media IDs from the boot interface + # This allows vendor-specific implementations to restrict which + # virtual media devices can be used + try: + acceptable_id = task.driver.boot._get_acceptable_media_id(task, + resource) + except AttributeError: + LOG.warning("Boot interface %s does not implement " + "_get_acceptable_media_id method. This method should be " + "implemented for compatibility.", + task.driver.boot.__class__.__name__) + acceptable_id = None + for v_media in resource.virtual_media.get_members(): + # Skip virtual media if the ID is not in the acceptable set + if acceptable_id is not None and v_media.identity != acceptable_id: + LOG.debug("Boot Interface %(name)s returned %(acceptable_id)s as " + "Virtual Media Slot ID, current slot is %(slot)s, " + "skipping", + {'name': task.driver.boot.__class__.__name__, + 'acceptable_id': acceptable_id, + 'slot': v_media.identity}) + continue + if boot_device not in v_media.media_types: # NOTE(janders): this conditional allows v_media that only # support DVD MediaType and NOT CD to also be used. @@ -671,6 +694,20 @@ class RedfishVirtualMediaBoot(base.BootInterface): 'node': task.node.uuid, 'vendor': vendor, 'fwv': bmc_manager[0].firmware_version}) + def _get_acceptable_media_id(self, task, resource): + """Get acceptable virtual media IDs for the resource. + + This method can be overridden by vendor-specific implementations + to return specific virtual media IDs that should be used. + + :param task: A TaskManager instance containing the node to act on. + :param resource: A redfish resource (System or Manager) containing + virtual media. + :returns: An acceptable virtual media ID, or None if any ID + is acceptable. + """ + return None + def validate(self, task): """Validate the deployment information for the task's node. diff --git a/ironic/tests/unit/drivers/modules/drac/test_boot.py b/ironic/tests/unit/drivers/modules/drac/test_boot.py index 5f18a44b23..e3d8ddedc2 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_boot.py +++ b/ironic/tests/unit/drivers/modules/drac/test_boot.py @@ -25,6 +25,7 @@ import sushy from ironic.common import boot_devices from ironic.conductor import task_manager from ironic.drivers.modules import deploy_utils +from ironic.drivers.modules.drac import boot as idrac_boot from ironic.drivers.modules.redfish import utils as redfish_utils from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.drivers.modules.drac import utils as test_utils @@ -112,3 +113,83 @@ class DracBootTestCase(test_utils.BaseDracTest): task.driver.boot._set_boot_device(task, boot_devices.DISK) self.assertFalse(mock_system.called) + + @mock.patch.object(idrac_boot.LOG, 'debug', autospec=True) + def test__get_idrac_version_from_model(self, mock_log_debug, + mock_get_system): + # Use shorter alias for readability and line length + idrac_redfish_boot = idrac_boot.DracRedfishVirtualMediaBoot + + # Test cases that should return None without logging + none_models = [None, "", "10G", "10ABC", "10G1XPT"] + for model in none_models: + version = idrac_redfish_boot._get_idrac_version_from_model(model) + self.assertIsNone(version) + + # Test cases causing TypeError/ValueError and should log debug message + error_models = [(5, 6, 7), [1, 2], "ABC", "X1Z", "9GTP"] + + for model in error_models: + version = idrac_redfish_boot._get_idrac_version_from_model(model) + self.assertIsNone(version) + # Verify that debug logging was called for each error model + self.assertEqual(mock_log_debug.call_count, len(error_models)) + # Verify the debug log calls have the correct format + expected_calls = [ + mock.call( + "Unable to parse iDRAC version from model string: %s", model) + for model in error_models + ] + mock_log_debug.assert_has_calls(expected_calls) + + idrac8 = ["12G Modular", "13G Modular"] + idrac9 = ["14G Monolithic", "15G Monolithic", "16G Monolithic", + "16G DCS"] + idrac10 = ["17G Monolithic", "18G Monolithic"] + for model in idrac8: + version = idrac_redfish_boot._get_idrac_version_from_model(model) + self.assertEqual(version, 8) + for model in idrac9: + version = idrac_redfish_boot._get_idrac_version_from_model(model) + self.assertEqual(version, 9) + for model in idrac10: + version = idrac_redfish_boot._get_idrac_version_from_model(model) + self.assertEqual(version, 10) + + def test__get_acceptable_media_id(self, mock_get_system): + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + + # Case 1: System resource with manager model "17G Monolithic" + # (iDRAC 10) returns "1" + mock_manager_idrac10 = mock.Mock() + mock_manager_idrac10.model = "17G Monolithic" + mock_system_resource = mock.Mock() + mock_system_resource.managers = [mock_manager_idrac10] + + result = task.driver.boot._get_acceptable_media_id( + task, mock_system_resource) + self.assertEqual("1", result) + + # Case 2: System resource with manager model "16G Monolithic" + # (iDRAC 9) returns None + mock_manager_idrac9 = mock.Mock() + mock_manager_idrac9.model = "16G Monolithic" + mock_system_resource_idrac9 = mock.Mock() + mock_system_resource_idrac9.managers = [mock_manager_idrac9] + + result = task.driver.boot._get_acceptable_media_id( + task, mock_system_resource_idrac9) + self.assertIsNone(result) + + # Case 3: Manager resource with model "15G Monolithic" + # Should return None (Manager resources) + mock_manager_resource = mock.Mock() + mock_manager_resource.model = "15G Monolithic" + # Simulate Manager resource by setting managers to None + mock_manager_resource.managers = None + + result = task.driver.boot._get_acceptable_media_id( + task, mock_manager_resource) + self.assertIsNone(result) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index cef06aa13f..c32d7ccbf1 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -3007,3 +3007,96 @@ class RedfishVirtualMediaBootViaSystemTestCase(db_base.DbTestCase): redfish_boot._has_vmedia_device( [mock_manager], sushy.VIRTUAL_MEDIA_FLOPPY, inserted=True, system=mock_system)) + + def test__insert_vmedia_in_resource_skips_unacceptable_ids(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + + # Mock the boot interface to return acceptable_id "1" + task.driver.boot._get_acceptable_media_id = mock.MagicMock( + return_value="1") + + # Create two virtual media devices with IDs "2" and "3" + mock_vmedia_2 = mock.MagicMock() + mock_vmedia_2.identity = "2" + mock_vmedia_2.media_types = [sushy.VIRTUAL_MEDIA_CD] + mock_vmedia_2.inserted = False + + mock_vmedia_3 = mock.MagicMock() + mock_vmedia_3.identity = "3" + mock_vmedia_3.media_types = [sushy.VIRTUAL_MEDIA_CD] + mock_vmedia_3.inserted = False + + # Mock resource with virtual media collection + mock_resource = mock.MagicMock() + mock_resource.virtual_media.get_members.return_value = [ + mock_vmedia_2, mock_vmedia_3 + ] + + # Create error messages list + err_msgs = [] + + # Call _insert_vmedia_in_resource + result = redfish_boot._insert_vmedia_in_resource( + task, mock_resource, 'http://example.com/boot.iso', + sushy.VIRTUAL_MEDIA_CD, err_msgs) + + # Should return False since no acceptable media found + self.assertFalse(result) + + # Verify that _get_acceptable_media_id was called + task.driver.boot._get_acceptable_media_id.assert_called_once_with( + task, mock_resource) + + # Verify that no insert_media was called on either device + # since they were skipped due to unacceptable IDs + mock_vmedia_2.insert_media.assert_not_called() + mock_vmedia_3.insert_media.assert_not_called() + + def test__insert_vmedia_in_resource_uses_acceptable_id(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + + # Mock the boot interface to return acceptable_id "1" + task.driver.boot._get_acceptable_media_id = mock.MagicMock( + return_value="1") + + # Create two virtual media devices with IDs "2" and "1" + mock_vmedia_2 = mock.MagicMock() + mock_vmedia_2.identity = "2" + mock_vmedia_2.media_types = [sushy.VIRTUAL_MEDIA_CD] + mock_vmedia_2.inserted = False + + mock_vmedia_1 = mock.MagicMock() + mock_vmedia_1.identity = "1" + mock_vmedia_1.media_types = [sushy.VIRTUAL_MEDIA_CD] + mock_vmedia_1.inserted = False + + # Mock resource with virtual media collection + mock_resource = mock.MagicMock() + mock_resource.virtual_media.get_members.return_value = [ + mock_vmedia_2, mock_vmedia_1 + ] + + # Create error messages list + err_msgs = [] + + # Call _insert_vmedia_in_resource + result = redfish_boot._insert_vmedia_in_resource( + task, mock_resource, 'http://example.com/boot.iso', + sushy.VIRTUAL_MEDIA_CD, err_msgs) + + # Should return True since acceptable media was found and used + self.assertTrue(result) + + # Verify that _get_acceptable_media_id was called + task.driver.boot._get_acceptable_media_id.assert_called_once_with( + task, mock_resource) + + # Verify that first device was skipped (ID "2" != acceptable "1") + mock_vmedia_2.insert_media.assert_not_called() + + # Verify that second device was used (ID "1" == acceptable "1") + mock_vmedia_1.insert_media.assert_called_once_with( + 'http://example.com/boot.iso', inserted=True, + write_protected=True) diff --git a/releasenotes/notes/idrac10-vmedia-slot1-5d15433b84843a10.yaml b/releasenotes/notes/idrac10-vmedia-slot1-5d15433b84843a10.yaml new file mode 100644 index 0000000000..354238112e --- /dev/null +++ b/releasenotes/notes/idrac10-vmedia-slot1-5d15433b84843a10.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes an issue where some iDRAC10 machines requires a specific Virtual + Media Slot. Ironic will attempt to automatically identify the version, + and ensures the right slot is used to insert the iso. + See `bug 2125571 `_ + for details.