x
/
proliantutils
Code Issues Proposed changes

Fixes caching of ilo instances to use admin credentials 

Currently the IloClient object cached uses iLO host as the key.
This can be issue if the iLO credentials are changed. The cached
node will not use new credentials leading to failure in
communication with iLO.

Change-Id: Ie1ea10aae78c4dab307d8a31996322597046ecb1
Closes-Bug: 1852688
This commit is contained in:
Shivanand Tendulker 2019-11-15 00:49:29 -05:00 committed by Shivanand Tendulker
parent 36f90a22d5
commit 086b3485c2
2 changed files with 32 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
proliantutils/ilo/client.py
View File

@ -136,12 +136,15 @@ def cache_node(cache=True):
self.cls = cls self.cls = cls
self._instances = collections.OrderedDict() self._instances = collections.OrderedDict()
def _if_not_exists(self, address): def _if_not_exists(self, ilo_info):
return (address not in self._instances) return (ilo_info not in self._instances)
def _create_instance(self, *args, **kwargs): def _create_instance(self, *args, **kwargs):
address = args[0] address = args[0]
self._instances[address] = self.cls(*args, **kwargs) admin = args[1]
admin_pass = args[2]
self._instances[(address, admin, admin_pass)] = (
self.cls(*args, **kwargs))
# Check for max_cache_size # Check for max_cache_size
if len(self._instances) > self.MAX_CACHE_SIZE: if len(self._instances) > self.MAX_CACHE_SIZE:
LOG.debug("Node cache hit the maximum size of %d." % ( LOG.debug("Node cache hit the maximum size of %d." % (
@ -152,20 +155,23 @@ def cache_node(cache=True):
if not args: if not args:
LOG.error("Error creating iLO object.") LOG.error("Error creating iLO object.")
address = args[0] address = args[0]
admin = args[1]
admin_pass = args[2]
if self._if_not_exists(address): if self._if_not_exists((address, admin, admin_pass)):
LOG.debug("Creating iLO object for node %(address)s.", LOG.debug("Creating iLO object for node %(address)s.",
{'address': address}) {'address': address})
self._create_instance(*args, **kwargs) self._create_instance(*args, **kwargs)
else: else:
LOG.debug("Using existing object for node " LOG.debug("Using existing object for node "
"%(address)s.", {'address': address}) "%(address)s.", {'address': address})
return self._instances[address] return self._instances[(address, admin, admin_pass)]
def _pop_oldest_node(self): def _pop_oldest_node(self):
node_keys = list(self._instances) node_keys = list(self._instances)
node_key = next(iter(node_keys)) node_key = next(iter(node_keys))
LOG.debug("Removed oldest node %s from cache" % (node_key)) LOG.debug("Removed oldest node {} from "
"cache".format(node_key))
rnode = self._instances.pop(node_key, None) rnode = self._instances.pop(node_key, None)
if rnode: if rnode:
del rnode del rnode

39
proliantutils/tests/ilo/test_client.py
View File

@ -55,9 +55,10 @@ class IloCacheNodeTestCase(testtools.TestCase):
class IloClientWrapperTestCase(testtools.TestCase): class IloClientWrapperTestCase(testtools.TestCase):
class DummyClass(object): class DummyClass(object):
def __init__(self, ip, name): def __init__(self, ip, name, password):
self._ip = ip self._ip = ip
self._name = name self._name = name
self._password = password
original_cls, decorated_cls = get_cls_wrapper(DummyClass) original_cls, decorated_cls = get_cls_wrapper(DummyClass)
@ -65,28 +66,28 @@ class IloClientWrapperTestCase(testtools.TestCase):
@mock.patch.object(wrapper_cls, '_create_instance') @mock.patch.object(wrapper_cls, '_create_instance')
@mock.patch.object(wrapper_cls, '_if_not_exists') @mock.patch.object(wrapper_cls, '_if_not_exists')
def test___call___new(self, exists_mock, create_mock): def test___call___already_created(self, exists_mock, create_mock):
exists_mock.return_value = True exists_mock.return_value = True
try: try:
wrapper_obj = IloClientWrapperTestCase.wrapper_cls( wrapper_obj = IloClientWrapperTestCase.wrapper_cls(
IloClientWrapperTestCase.original_cls) IloClientWrapperTestCase.original_cls)
wrapper_obj('a.b.c.d', 'abcd') wrapper_obj('a.b.c.d', 'abcd', 'deaf')
except KeyError: except KeyError:
pass pass
exists_mock.assert_called_once_with('a.b.c.d') exists_mock.assert_called_once_with(('a.b.c.d', 'abcd', 'deaf'))
create_mock.assert_called_once_with('a.b.c.d', 'abcd') create_mock.assert_called_once_with('a.b.c.d', 'abcd', 'deaf')
@mock.patch.object(wrapper_cls, '_create_instance') @mock.patch.object(wrapper_cls, '_create_instance')
@mock.patch.object(wrapper_cls, '_if_not_exists') @mock.patch.object(wrapper_cls, '_if_not_exists')
def test___call___already_created(self, exists_mock, create_mock): def test___call___new(self, exists_mock, create_mock):
exists_mock.return_value = False exists_mock.return_value = False
try: try:
wrapper_obj = IloClientWrapperTestCase.wrapper_cls( wrapper_obj = IloClientWrapperTestCase.wrapper_cls(
IloClientWrapperTestCase.original_cls) IloClientWrapperTestCase.original_cls)
wrapper_obj('a.b.c.d', 'abcd') wrapper_obj('a.b.c.d', 'abcd', 'deaf')
except KeyError: except KeyError:
pass pass
exists_mock.assert_called_once_with('a.b.c.d') exists_mock.assert_called_once_with(('a.b.c.d', 'abcd', 'deaf'))
create_mock.assert_not_called() create_mock.assert_not_called()
@mock.patch.object(original_cls, '__init__') @mock.patch.object(original_cls, '__init__')
@ -96,8 +97,8 @@ class IloClientWrapperTestCase(testtools.TestCase):
wrapper_obj = IloClientWrapperTestCase.wrapper_cls( wrapper_obj = IloClientWrapperTestCase.wrapper_cls(
IloClientWrapperTestCase.original_cls) IloClientWrapperTestCase.original_cls)
wrapper_obj.MAX_CACHE_SIZE = 2 wrapper_obj.MAX_CACHE_SIZE = 2
wrapper_obj._create_instance('a.b.c.d', 'abcd') wrapper_obj._create_instance('a.b.c.d', 'abcd', 'defe')
init_mock.assert_called_once_with('a.b.c.d', 'abcd') init_mock.assert_called_once_with('a.b.c.d', 'abcd', 'defe')
pop_mock.assert_not_called() pop_mock.assert_not_called()
@mock.patch.object(original_cls, '__init__') @mock.patch.object(original_cls, '__init__')
@ -107,21 +108,21 @@ class IloClientWrapperTestCase(testtools.TestCase):
wrapper_obj = IloClientWrapperTestCase.wrapper_cls( wrapper_obj = IloClientWrapperTestCase.wrapper_cls(
IloClientWrapperTestCase.original_cls) IloClientWrapperTestCase.original_cls)
wrapper_obj.MAX_CACHE_SIZE = 2 wrapper_obj.MAX_CACHE_SIZE = 2
wrapper_obj._create_instance('a.b.c.d', 'abcd') wrapper_obj._create_instance('a.b.c.d', 'abcd', 'deaf')
wrapper_obj._create_instance('e.f.g.h', 'efgh') wrapper_obj._create_instance('e.f.g.h', 'efgh', 'deaf')
wrapper_obj._create_instance('i.j.k.l', 'ijkl') wrapper_obj._create_instance('i.j.k.l', 'ijkl', 'deaf')
pop_mock.assert_called_once_with() pop_mock.assert_called_once_with()
def test__pop_oldest_node(self): def test__pop_oldest_node(self):
wrapper_obj = IloClientWrapperTestCase.wrapper_cls( wrapper_obj = IloClientWrapperTestCase.wrapper_cls(
IloClientWrapperTestCase.original_cls) IloClientWrapperTestCase.original_cls)
wrapper_obj.MAX_CACHE_SIZE = 2 wrapper_obj.MAX_CACHE_SIZE = 2
wrapper_obj('a.b.c.d', 'abcd') wrapper_obj('a.b.c.d', 'abcd', 'deaf')
wrapper_obj('e.f.g.h', 'efgh') wrapper_obj('e.f.g.h', 'efgh', 'deaf')
wrapper_obj('i.j.k.l', 'ijkl') wrapper_obj('i.j.k.l', 'ijkl', 'deaf')
self.assertIn('i.j.k.l', wrapper_obj._instances) self.assertIn(('i.j.k.l', 'ijkl', 'deaf'), wrapper_obj._instances)
self.assertIn('e.f.g.h', wrapper_obj._instances) self.assertIn(('e.f.g.h', 'efgh', 'deaf'), wrapper_obj._instances)
self.assertNotIn('a.b.c.d', wrapper_obj._instances) self.assertNotIn(('a.b.c.d', 'ijkl', 'deaf'), wrapper_obj._instances)
class IloClientInitTestCase(testtools.TestCase): class IloClientInitTestCase(testtools.TestCase):