diff --git a/ironic/drivers/modules/redfish/utils.py b/ironic/drivers/modules/redfish/utils.py index 108b585e8a..eadabec2c9 100644 --- a/ironic/drivers/modules/redfish/utils.py +++ b/ironic/drivers/modules/redfish/utils.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import os from oslo_log import log @@ -149,6 +150,52 @@ def parse_driver_info(node): 'node_uuid': node.uuid} +class SessionCache(object): + """Cache of HTTP sessions credentials""" + MAX_SESSIONS = 1000 + + sessions = collections.OrderedDict() + + def __init__(self, driver_info): + self._driver_info = driver_info + self._session_key = tuple( + self._driver_info.get(key) + for key in ('address', 'username', 'verify_ca') + ) + + def __enter__(self): + try: + return self.sessions[self._session_key] + + except KeyError: + conn = sushy.Sushy( + self._driver_info['address'], + username=self._driver_info['username'], + password=self._driver_info['password'], + verify=self._driver_info['verify_ca'] + ) + + self._expire_oldest_session() + + self.sessions[self._session_key] = conn + + return conn + + def __exit__(self, exc_type, exc_val, exc_tb): + # NOTE(etingof): perhaps this session token is no good + if isinstance(exc_val, sushy.exceptions.ConnectionError): + self.sessions.pop(self._session_key, None) + + def _expire_oldest_session(self): + """Expire oldest session""" + if len(self.sessions) >= self.MAX_SESSIONS: + session_keys = list(self.sessions) + session_key = session_keys[0] + # NOTE(etingof): GC should cause sushy to HTTP DELETE session + # at BMC. Trouble is that as of this commit sushy does not do that. + self.sessions.pop(session_key, None) + + def get_system(node): """Get a Redfish System that represents a node. @@ -167,13 +214,9 @@ def get_system(node): wait_fixed=CONF.redfish.connection_retry_interval * 1000) def _get_system(): try: - # TODO(lucasagomes): We should look into a mechanism to - # cache the connections (and maybe even system's instances) - # to avoid unnecessary requests to the Redfish controller - conn = sushy.Sushy(address, username=driver_info['username'], - password=driver_info['password'], - verify=driver_info['verify_ca']) - return conn.get_system(system_id) + with SessionCache(driver_info) as conn: + return conn.get_system(system_id) + except sushy.exceptions.ResourceNotFoundError as e: LOG.error('The Redfish System "%(system)s" was not found for ' 'node %(node)s. Error %(error)s', diff --git a/ironic/tests/unit/drivers/modules/redfish/test_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_utils.py index c1e100f211..e87d2a1a40 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_utils.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_utils.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import copy import os @@ -146,17 +147,22 @@ class RedfishUtilsTestCase(db_base.DbTestCase): redfish_utils.parse_driver_info, self.node) @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + @mock.patch('ironic.drivers.modules.redfish.utils.' + 'SessionCache.sessions', {}) def test_get_system(self, mock_sushy): + mock_sushy.exceptions.ConnectionError = MockedConnectionError fake_conn = mock_sushy.Sushy.return_value fake_system = fake_conn.get_system.return_value - response = redfish_utils.get_system(self.node) self.assertEqual(fake_system, response) fake_conn.get_system.assert_called_once_with( '/redfish/v1/Systems/FAKESYSTEM') @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + @mock.patch('ironic.drivers.modules.redfish.utils.' + 'SessionCache.sessions', {}) def test_get_system_resource_not_found(self, mock_sushy): + mock_sushy.exceptions.ConnectionError = MockedConnectionError fake_conn = mock_sushy.Sushy.return_value mock_sushy.exceptions.ResourceNotFoundError = ( MockedResourceNotFoundError) @@ -169,6 +175,8 @@ class RedfishUtilsTestCase(db_base.DbTestCase): @mock.patch('time.sleep', autospec=True) @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + @mock.patch('ironic.drivers.modules.redfish.utils.' + 'SessionCache.sessions', {}) def test_get_system_resource_connection_error_retry(self, mock_sushy, mock_sleep): # Redfish specific configurations @@ -191,3 +199,57 @@ class RedfishUtilsTestCase(db_base.DbTestCase): fake_conn.get_system.assert_has_calls(expected_get_system_calls) mock_sleep.assert_called_with( redfish_utils.CONF.redfish.connection_retry_interval) + + @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + @mock.patch('ironic.drivers.modules.redfish.utils.' + 'SessionCache.sessions', {}) + def test_auth_auto(self, mock_sushy): + mock_sushy.exceptions.ConnectionError = MockedConnectionError + redfish_utils.get_system(self.node) + mock_sushy.Sushy.assert_called_with( + self.parsed_driver_info['address'], + username=self.parsed_driver_info['username'], + password=self.parsed_driver_info['password'], + verify=True) + + @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + @mock.patch('ironic.drivers.modules.redfish.utils.' + 'SessionCache.sessions', {}) + def test_ensure_session_reuse(self, mock_sushy): + mock_sushy.exceptions.ConnectionError = MockedConnectionError + redfish_utils.get_system(self.node) + redfish_utils.get_system(self.node) + self.assertEqual(1, mock_sushy.Sushy.call_count) + + @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + def test_ensure_new_session_address(self, mock_sushy): + mock_sushy.exceptions.ConnectionError = MockedConnectionError + self.node.driver_info['redfish_address'] = 'http://bmc.foo' + redfish_utils.get_system(self.node) + self.node.driver_info['redfish_address'] = 'http://bmc.bar' + redfish_utils.get_system(self.node) + self.assertEqual(2, mock_sushy.Sushy.call_count) + + @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + def test_ensure_new_session_username(self, mock_sushy): + mock_sushy.exceptions.ConnectionError = MockedConnectionError + self.node.driver_info['redfish_username'] = 'foo' + redfish_utils.get_system(self.node) + self.node.driver_info['redfish_username'] = 'bar' + redfish_utils.get_system(self.node) + self.assertEqual(2, mock_sushy.Sushy.call_count) + + @mock.patch('ironic.drivers.modules.redfish.utils.sushy') + @mock.patch('ironic.drivers.modules.redfish.utils.' + 'SessionCache.MAX_SESSIONS', 10) + @mock.patch('ironic.drivers.modules.redfish.utils.SessionCache.sessions', + collections.OrderedDict()) + def test_expire_old_sessions(self, mock_sushy): + mock_sushy.exceptions.ConnectionError = MockedConnectionError + + for num in range(20): + self.node.driver_info['redfish_username'] = 'foo-%d' % num + redfish_utils.get_system(self.node) + + self.assertEqual(mock_sushy.Sushy.call_count, 20) + self.assertEqual(len(redfish_utils.SessionCache.sessions), 10) diff --git a/releasenotes/notes/persist-redfish-sessions-d521a0846fa45c40.yaml b/releasenotes/notes/persist-redfish-sessions-d521a0846fa45c40.yaml new file mode 100644 index 0000000000..a986d9e1b9 --- /dev/null +++ b/releasenotes/notes/persist-redfish-sessions-d521a0846fa45c40.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes ``redfish`` hardware type to reuse HTTP session tokens when + talking to BMC using session authentication. Prior to this fix ``redfish`` + hardware type never tried to reuse session token given out by BMC during + previous connection what may sometimes lead to session pool exhaustion + with some BMC implementations.