From e8cb96efc5c5528f360add4ecf15ad258f98f941 Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Wed, 12 Jul 2023 09:13:05 -0700
Subject: [PATCH] Detect ilo6 and redirect to redfish

We got a report of ilo6 BMCs not working with the ilo hardware type,
and while information is limited, it seems reasonable to at least log
some information on this subject and start discussion of what should
we do.

Hence the creation of this patch.

See: https://meetings.opendev.org/irclogs/%23openstack-ironic/%23openstack-ironic.2023-07-05.log.html#t2023-07-05T11:06:08
Change-Id: I2d81ef736aca44f455f8233c32c0c8d12267d57f
---
 ironic/drivers/modules/ilo/power.py           | 11 ++++++++++-
 .../unit/drivers/modules/ilo/test_power.py    | 19 +++++++++++++++++++
 ...ning-in-ilo-for-ilo6-c400b35f55b81f50.yaml |  9 +++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/add-warning-in-ilo-for-ilo6-c400b35f55b81f50.yaml

diff --git a/ironic/drivers/modules/ilo/power.py b/ironic/drivers/modules/ilo/power.py
index 909fa5a8c6..d5713620b0 100644
--- a/ironic/drivers/modules/ilo/power.py
+++ b/ironic/drivers/modules/ilo/power.py
@@ -79,12 +79,21 @@ def _get_power_state(node):
     # Check the current power state.
     try:
         power_status = ilo_object.get_host_power_status()
-
     except ilo_error.IloError as ilo_exception:
         LOG.error("iLO get_power_state failed for node %(node_id)s with "
                   "error: %(error)s.",
                   {'node_id': node.uuid, 'error': ilo_exception})
         operation = _('iLO get_power_status')
+        if 'RIBCLI is disabled' in str(ilo_exception):
+            warn_msg = ("Node %s appears to have a disabled API "
+                        "required for the \'%s\' hardware type to function. "
+                        "Please consider using the \'redfish\' hardware "
+                        "type." % (node.uuid, node.driver))
+            manager_utils.node_history_record(node, event=warn_msg,
+                                              event_type=node.provision_state,
+                                              error=True)
+            raise exception.IloOperationError(operation=operation,
+                                              error=warn_msg)
         raise exception.IloOperationError(operation=operation,
                                           error=ilo_exception)
 
diff --git a/ironic/tests/unit/drivers/modules/ilo/test_power.py b/ironic/tests/unit/drivers/modules/ilo/test_power.py
index ff49b34ad6..d8aa6534da 100644
--- a/ironic/tests/unit/drivers/modules/ilo/test_power.py
+++ b/ironic/tests/unit/drivers/modules/ilo/test_power.py
@@ -73,6 +73,25 @@ class IloPowerInternalMethodsTestCase(test_common.BaseIloTest):
                           self.node)
         ilo_mock_object.get_host_power_status.assert_called_once_with()
 
+    @mock.patch.object(manager_utils, 'node_history_record', autospec=True)
+    def test__get_power_state_ilo6_redirect(self, record_history_mock,
+                                            get_ilo_object_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+        exc = ilo_error.IloError('Error: RIBCLI is disabled.')
+        ilo_mock_object.get_host_power_status.side_effect = exc
+        self.assertRaisesRegex(exception.IloOperationError,
+                               'appears to have a disabled API required',
+                               ilo_power._get_power_state,
+                               self.node)
+        ilo_mock_object.get_host_power_status.assert_called_once_with()
+        record_history_mock.assert_called_once_with(
+            mock.ANY,
+            event=('Node %s appears to have a disabled API required for the '
+                   '\'ilo\' hardware type to function. Please consider using '
+                   'the \'redfish\' hardware type.' % self.node.uuid),
+            event_type='available',
+            error=True)
+
     def test__set_power_state_invalid_state(self, get_ilo_object_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
diff --git a/releasenotes/notes/add-warning-in-ilo-for-ilo6-c400b35f55b81f50.yaml b/releasenotes/notes/add-warning-in-ilo-for-ilo6-c400b35f55b81f50.yaml
new file mode 100644
index 0000000000..82d4d7b398
--- /dev/null
+++ b/releasenotes/notes/add-warning-in-ilo-for-ilo6-c400b35f55b81f50.yaml
@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixes a vague error when attempting to use the ``ilo`` hardware type with
+    iLO6 hardware, by returning a more specific error suggesting action to
+    take in order to remedy the issue. Specifically, one of the API's
+    used by the ``ilo`` hardware type is disabled in iLO6 BMCs in favor
+    of users utilizing Redfish. Operators are advised to utilize the
+    ``redfish`` hardware type for these machines.