Merge "Force Virtual Media Slot 1 on iDRAC10"

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.

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.
 

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)

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)

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 <https://bugs.launchpad.net/ironic/+bug/2125571>`_
+    for details.