Remove redfish cache entry upon errors

Some transient errors can ultimately cause the client to need to be
completely restarted due to cached connection data.

Ironic now explicitly removes the cache entry when a sushy
AccessError or python AttributeError is detected originating
from the library. This will now result in the prior cached connection
object to be discarded, and upon the next attempt to interact with
the same node, a new connection will be launched.

This will result in new sessions being created, but in all likelihood
the prior session had already timed out or had been administratively
removed. Sushy's code, as of
https://review.opendev.org/c/openstack/sushy/+/820076
will raise SessionService lookup access errors as AccessErrors.
Prior to that change, they should have been raised as AttributeError
as the previous call sould have returned None to be used as an object.

*Also* Includes follow-up change Ia59f774c9340e3a6fa63418afedf12098c709052
squashed into this change, and necessary backport friendly mock of
AccessError which had changed since this release of ironic.

Change-Id: Icc6e5dd74d9f15e679a7e764fe49238ed6b8dc1e
Story: 2009719
Task: 44107
(cherry picked from commit 1439af27ba2bd31fb85369754c648a45ee9ca14b)
(cherry picked from commit 01997c8418b9e6ade47437b09dd9412310b90eac)
(cherry picked from commit e3e7deaf48a5315fe10350e8a87b2fcc4e189406)
(cherry picked from commit ab5495eecb409b37ad853a679032d6cdb4fefc72)
(cherry picked from commit e57aa7598b220bb2d1fd8fcc0aab6132926d5cce)

2 files changed, 74 insertions, 1 deletions

diff --git a/ironic/tests/unit/drivers/modules/redfish/test_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_utils.py
index d818fcab3..372dfdc43 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_utils.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_utils.py
@@ -16,6 +16,7 @@
 import collections
 import copy
 import os
+import time
 from unittest import mock
 
 from oslo_config import cfg
@@ -211,8 +212,9 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
         # Redfish specific configurations
         self.config(connection_attempts=3, group='redfish')
 
-        fake_conn = mock_sushy.return_value
+        fake_conn = mock.Mock()
         fake_conn.get_system.side_effect = sushy.exceptions.ConnectionError()
+        mock_sushy.return_value = fake_conn
 
         self.assertRaises(exception.RedfishConnectionError,
                           redfish_utils.get_system, self.node)
@@ -226,6 +228,75 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
         mock_sleep.assert_called_with(
             redfish_utils.CONF.redfish.connection_retry_interval)
 
+    @mock.patch.object(time, 'sleep', lambda seconds: None)
+    @mock.patch.object(sushy, 'Sushy', autospec=True)
+    @mock.patch('ironic.drivers.modules.redfish.utils.'
+                'SessionCache._sessions', {})
+    def test_get_system_resource_access_error_retry(self, mock_sushy):
+
+        # Sushy access errors HTTP Errors
+        class fake_response(object):
+            status_code = 401
+            body = None
+
+            def json():
+                return {}
+
+        fake_conn = mock_sushy.return_value
+        fake_system = mock.Mock()
+        fake_conn.get_system.side_effect = iter(
+            [
+                sushy.exceptions.AccessError(
+                    method='GET',
+                    url='http://path/to/url',
+                    response=fake_response),
+                fake_system,
+            ])
+
+        self.assertRaises(exception.RedfishError,
+                          redfish_utils.get_system, self.node)
+        # Retry, as in next power sync perhaps
+        client = redfish_utils.get_system(self.node)
+        client('foo')
+
+        expected_get_system_calls = [
+            mock.call(self.parsed_driver_info['system_id']),
+            mock.call(self.parsed_driver_info['system_id']),
+        ]
+        fake_conn.get_system.assert_has_calls(expected_get_system_calls)
+        fake_system.assert_called_with('foo')
+        self.assertEqual(fake_conn.get_system.call_count, 2)
+
+    @mock.patch.object(time, 'sleep', lambda seconds: None)
+    @mock.patch.object(sushy, 'Sushy', autospec=True)
+    @mock.patch('ironic.drivers.modules.redfish.utils.'
+                'SessionCache._sessions', {})
+    def test_get_system_resource_attribute_error(self, mock_sushy):
+
+        fake_conn = mock_sushy.return_value
+        fake_system = mock.Mock()
+        fake_conn.get_system.side_effect = iter(
+            [
+                AttributeError,
+                fake_system,
+            ])
+        # We need to check for AttributeError explicitly as
+        # otherwise we break existing tests if we try to catch
+        # it explicitly.
+        self.assertRaises(exception.RedfishError,
+                          redfish_utils.get_system, self.node)
+        # Retry, as in next power sync perhaps
+        client = redfish_utils.get_system(self.node)
+        client('bar')
+        expected_get_system_calls = [
+            mock.call(self.parsed_driver_info['system_id']),
+            mock.call(self.parsed_driver_info['system_id']),
+        ]
+
+        fake_conn.get_system.assert_has_calls(expected_get_system_calls)
+        fake_system.assert_called_once_with('bar')
+        self.assertEqual(fake_conn.get_system.call_count, 2)
+
     @mock.patch.object(sushy, 'Sushy', autospec=True)
     @mock.patch('ironic.drivers.modules.redfish.utils.'
                 'SessionCache._sessions', {})
diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py
index 94c5b2442..f45b6922c 100644
--- a/ironic/tests/unit/drivers/third_party_driver_mocks.py
+++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py
@@ -229,6 +229,8 @@ if not sushy:
         type('SushyError', (MockKwargsException,), {}))
     sushy.exceptions.ConnectionError = (
         type('ConnectionError', (sushy.exceptions.SushyError,), {}))
+    sushy.exceptions.AccessError = (
+        type('AccessError', (sushy.exceptions.SushyError,), {}))
     sushy.exceptions.ResourceNotFoundError = (
         type('ResourceNotFoundError', (sushy.exceptions.SushyError,), {}))
     sushy.exceptions.MissingAttributeError = (