openstack/ironic
Code Issues Proposed changes

Merge "Reuse Redfish sessions follow up"

Browse Source
This commit is contained in:
Zuul
2018-11-20 12:45:15 +00:00
committed by Gerrit Code Review
parent e78c4f4983 635095406c
commit 70de7aef41
3 changed files with 47 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
ironic/conf/redfish.py
View File

@@ -27,7 +27,16 @@ opts = [
min=1,
default=4,
help=_('Number of seconds to wait between attempts to '
'connect to Redfish'))
'connect to Redfish')),
cfg.IntOpt('connection_cache_size',
min=0,
default=1000,
help=_('Maximum Redfish client connection cache size. '
'Redfish driver would strive to reuse authenticated '
'BMC connections (obtained through Redfish Session '
'Service). This option caps the maximum number of '
'connections to maintain. The value of `0` disables '
'client connection caching completely.'))
]

30
ironic/drivers/modules/redfish/utils.py
View File

@@ -152,9 +152,7 @@ def parse_driver_info(node):
class SessionCache(object):
"""Cache of HTTP sessions credentials"""
MAX_SESSIONS = 1000
sessions = collections.OrderedDict()
_sessions = collections.OrderedDict()
def __init__(self, driver_info):
self._driver_info = driver_info
@@ -165,7 +163,7 @@ class SessionCache(object):
def __enter__(self):
try:
return self.sessions[self._session_key]
return self.__class__._sessions[self._session_key]
except KeyError:
conn = sushy.Sushy(
@@ -175,25 +173,29 @@ class SessionCache(object):
verify=self._driver_info['verify_ca']
)
self._expire_oldest_session()
if CONF.redfish.connection_cache_size:
self.__class__._sessions[self._session_key] = conn
self.sessions[self._session_key] = conn
if (len(self.__class__._sessions) >
CONF.redfish.connection_cache_size):
self._expire_oldest_session()
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)
self.__class__._sessions.pop(self._session_key, None)
def _expire_oldest_session(self):
@classmethod
def _expire_oldest_session(cls):
"""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)
session_keys = list(cls._sessions)
session_key = next(iter(session_keys))
# NOTE(etingof): GC should cause sushy to HTTP DELETE session
# at BMC. Trouble is that contemporary sushy (1.6.0) does
# does not do that.
cls._sessions.pop(session_key, None)
def get_system(node):

30
ironic/tests/unit/drivers/modules/redfish/test_utils.py
View File

@@ -18,6 +18,7 @@ import copy
import os
import mock
from oslo_config import cfg
from oslo_utils import importutils
import requests
@@ -141,7 +142,7 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.sessions', {})
'SessionCache._sessions', {})
def test_get_system(self, mock_sushy):
fake_conn = mock_sushy.return_value
fake_system = fake_conn.get_system.return_value
@@ -152,7 +153,7 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.sessions', {})
'SessionCache._sessions', {})
def test_get_system_resource_not_found(self, mock_sushy):
fake_conn = mock_sushy.return_value
fake_conn.get_system.side_effect = (
@@ -168,7 +169,7 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
@mock.patch('time.sleep', autospec=True)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.sessions', {})
'SessionCache._sessions', {})
def test_get_system_resource_connection_error_retry(self, mock_sushy,
mock_sleep):
# Redfish specific configurations
@@ -191,7 +192,7 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.sessions', {})
'SessionCache._sessions', {})
def test_auth_auto(self, mock_sushy):
redfish_utils.get_system(self.node)
mock_sushy.assert_called_with(
@@ -202,7 +203,7 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.sessions', {})
'SessionCache._sessions', {})
def test_ensure_session_reuse(self, mock_sushy):
redfish_utils.get_system(self.node)
redfish_utils.get_system(self.node)
@@ -225,14 +226,25 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
self.assertEqual(2, mock_sushy.call_count)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.'
'SessionCache.MAX_SESSIONS', 10)
@mock.patch('ironic.drivers.modules.redfish.utils.SessionCache.sessions',
@mock.patch('ironic.drivers.modules.redfish.utils.SessionCache._sessions',
collections.OrderedDict())
def test_expire_old_sessions(self, mock_sushy):
cfg.CONF.set_override('connection_cache_size', 10, 'redfish')
for num in range(20):
self.node.driver_info['redfish_username'] = 'foo-%d' % num
redfish_utils.get_system(self.node)
self.assertEqual(mock_sushy.call_count, 20)
self.assertEqual(len(redfish_utils.SessionCache.sessions), 10)
self.assertEqual(len(redfish_utils.SessionCache._sessions), 10)
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch('ironic.drivers.modules.redfish.utils.SessionCache._sessions',
collections.OrderedDict())
def test_disabled_sessions_cache(self, mock_sushy):
cfg.CONF.set_override('connection_cache_size', 0, 'redfish')
for num in range(2):
self.node.driver_info['redfish_username'] = 'foo-%d' % num
redfish_utils.get_system(self.node)
self.assertEqual(mock_sushy.call_count, 2)
self.assertEqual(len(redfish_utils.SessionCache._sessions), 0)