openstack/ironic-inspector
Code Issues Proposed changes

Rename NodeInfo._lock to avoid conflict with Mock._lock in tests

Browse Source 
Starting with https://github.com/python/cpython/pull/98797, Python's
Mock has its own _lock. I hope they rename it to something really
private (e.g. __lock), but for now rename our attribute (and hope that
no downstream plugins relied on it, sigh).

Change-Id: I7ba858fb3f259b8e7a3becde94b7ba6b90615287
This commit is contained in:
Dmitry Tantsur 2023-01-10 14:29:37 +01:00
parent c0f9602227
commit cb1e856b9f
3 changed files with 28 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironic_inspector/node_cache.py
View File

@ -75,16 +75,16 @@ class NodeInfo(object):
# equivalent to True actually.
self._manage_boot = manage_boot if manage_boot is not None else True
# This is a lock on a node UUID, not on a NodeInfo object
self._lock = locking.get_lock(uuid)
self._node_lock = locking.get_lock(uuid)
# Whether lock was acquired using this NodeInfo object
self._fsm = None
self._options = None
def __del__(self):
if self._lock.is_locked():
if self._node_lock.is_locked():
LOG.warning('BUG: node lock was not released by the moment '
'node info object is deleted')
self._lock.release()
self._node_lock.release()
def __str__(self):
"""Self represented as an UUID and a state."""
@ -103,13 +103,13 @@ class NodeInfo(object):
return immediately.
:returns: boolean value, whether lock was acquired successfully
"""
if self._lock.is_locked():
if self._node_lock.is_locked():
LOG.debug('Attempting to acquire lock already held',
node_info=self)
return True
LOG.debug('Attempting to acquire lock', node_info=self)
if self._lock.acquire(blocking):
if self._node_lock.acquire(blocking):
LOG.debug('Successfully acquired lock', node_info=self)
return True
else:
@ -121,9 +121,9 @@ class NodeInfo(object):
Does nothing if lock was not acquired using this NodeInfo object.
"""
if self._lock.is_locked():
if self._node_lock.is_locked():
LOG.debug('Successfully released lock', node_info=self)
self._lock.release()
self._node_lock.release()
@property
def version_id(self):
@ -640,7 +640,7 @@ def release_lock(func):
finally:
# FIXME(milan) hacking the test cases to work
# with release_lock.assert_called_once...
if node_info._lock.is_locked():
if node_info._node_lock.is_locked():
node_info.release_lock()
return inner

1
ironic_inspector/test/unit/test_introspect.py
View File

@ -41,6 +41,7 @@ class BaseTest(test_base.NodeTestBase):
self.node_info = mock.Mock(uuid=self.uuid, options={})
self.node_info.ports.return_value = self.ports_dict
self.node_info.node.return_value = self.node
driver_fixture = self.useFixture(fixtures.MockPatchObject(
pxe_filter, 'driver', autospec=True))
driver_mock = driver_fixture.mock.return_value

38
ironic_inspector/test/unit/test_node_cache.py
View File

@ -55,7 +55,7 @@ class TestNodeCache(test_base.NodeTestBase):
(datetime.datetime.utcnow() - datetime.timedelta(seconds=60)
< node.started_at <
datetime.datetime.utcnow() + datetime.timedelta(seconds=60)))
self.assertFalse(node._lock.is_locked())
self.assertFalse(node._node_lock.is_locked())
res = set((r.uuid, r.started_at) for r in db.get_nodes())
@ -196,7 +196,7 @@ class TestNodeCacheFind(test_base.NodeTestBase):
datetime.datetime.utcnow() - datetime.timedelta(seconds=60)
< res.started_at <
datetime.datetime.utcnow() + datetime.timedelta(seconds=1))
self.assertTrue(res._lock.is_locked())
self.assertTrue(res._node_lock.is_locked())
def test_same_bmc_different_macs(self):
uuid2 = uuidutils.generate_uuid()
@ -227,7 +227,7 @@ class TestNodeCacheFind(test_base.NodeTestBase):
datetime.datetime.utcnow() - datetime.timedelta(seconds=60)
< res.started_at <
datetime.datetime.utcnow() + datetime.timedelta(seconds=1))
self.assertTrue(res._lock.is_locked())
self.assertTrue(res._node_lock.is_locked())
def test_macs_not_found(self):
self.assertRaises(utils.Error, node_cache.find_node,
@ -250,7 +250,7 @@ class TestNodeCacheFind(test_base.NodeTestBase):
datetime.datetime.utcnow() - datetime.timedelta(seconds=60)
< res.started_at <
datetime.datetime.utcnow() + datetime.timedelta(seconds=1))
self.assertTrue(res._lock.is_locked())
self.assertTrue(res._node_lock.is_locked())
def test_inconsistency(self):
db.delete_node(uuid=self.uuid)
@ -388,7 +388,7 @@ class TestNodeCacheGetNode(test_base.NodeTestBase):
self.assertEqual(started_at, info.started_at)
self.assertIsNone(info.finished_at)
self.assertIsNone(info.error)
self.assertFalse(info._lock.is_locked())
self.assertFalse(info._node_lock.is_locked())
def test_not_found(self):
self.assertRaises(utils.Error, node_cache.get_node,
@ -410,7 +410,7 @@ class TestNodeCacheGetNode(test_base.NodeTestBase):
self.assertEqual(started_at, info.started_at)
self.assertIsNone(info.finished_at)
self.assertIsNone(info.error)
self.assertFalse(info._lock.is_locked())
self.assertFalse(info._node_lock.is_locked())
ironic.get_node.assert_called_once_with('name')
@ -448,7 +448,7 @@ class TestNodeInfoFinished(test_base.NodeTestBase):
def test_release_lock(self):
self.node_info.acquire_lock()
self.node_info.finished(istate.Events.finish)
self.assertFalse(self.node_info._lock.is_locked())
self.assertFalse(self.node_info._node_lock.is_locked())
class TestNodeInfoOptions(test_base.NodeTestBase):
@ -812,40 +812,40 @@ class TestInternalLock(test_base.NodeTestBase):
def test_acquire(self, lock_mock):
node_info = node_cache.NodeInfo(self.uuid)
self.addCleanup(node_info.release_lock)
self.assertFalse(node_info._lock.is_locked())
self.assertFalse(node_info._node_lock.is_locked())
lock_mock.assert_called_once_with('node-{}'.format(self.uuid),
semaphores=mock.ANY)
self.assertFalse(lock_mock.return_value.acquire.called)
self.assertTrue(node_info.acquire_lock())
self.assertTrue(node_info._lock.is_locked())
self.assertTrue(node_info._node_lock.is_locked())
self.assertTrue(node_info.acquire_lock())
self.assertTrue(node_info._lock.is_locked())
self.assertTrue(node_info._node_lock.is_locked())
lock_mock.return_value.acquire.assert_called_once_with(blocking=True)
def test_release(self, lock_mock):
node_info = node_cache.NodeInfo(self.uuid)
node_info.acquire_lock()
self.assertTrue(node_info._lock.is_locked())
self.assertTrue(node_info._node_lock.is_locked())
node_info.release_lock()
self.assertFalse(node_info._lock.is_locked())
self.assertFalse(node_info._node_lock.is_locked())
node_info.release_lock()
self.assertFalse(node_info._lock.is_locked())
self.assertFalse(node_info._node_lock.is_locked())
lock_mock.return_value.acquire.assert_called_once_with(blocking=True)
lock_mock.return_value.release.assert_called_once_with()
def test_acquire_non_blocking(self, lock_mock):
node_info = node_cache.NodeInfo(self.uuid)
self.addCleanup(node_info.release_lock)
self.assertFalse(node_info._lock.is_locked())
self.assertFalse(node_info._node_lock.is_locked())
lock_mock.return_value.acquire.side_effect = iter([False, True])
self.assertFalse(node_info.acquire_lock(blocking=False))
self.assertFalse(node_info._lock.is_locked())
self.assertFalse(node_info._node_lock.is_locked())
self.assertTrue(node_info.acquire_lock(blocking=False))
self.assertTrue(node_info._lock.is_locked())
self.assertTrue(node_info._node_lock.is_locked())
self.assertTrue(node_info.acquire_lock(blocking=False))
self.assertTrue(node_info._lock.is_locked())
self.assertTrue(node_info._node_lock.is_locked())
lock_mock.return_value.acquire.assert_called_with(blocking=False)
self.assertEqual(2, lock_mock.return_value.acquire.call_count)
@ -1123,7 +1123,7 @@ class TestFsmEvent(test_base.NodeStateTest):
def test_unlock(self):
@node_cache.release_lock
def func(node_info):
self.assertTrue(node_info._lock.is_locked())
self.assertTrue(node_info._node_lock.is_locked())
self.node_info.acquire_lock(blocking=True)
with mock.patch.object(self.node_info, 'release_lock',
@ -1134,7 +1134,7 @@ class TestFsmEvent(test_base.NodeStateTest):
def test_unlock_unlocked(self):
@node_cache.release_lock
def func(node_info):
self.assertFalse(node_info._lock.is_locked())
self.assertFalse(node_info._node_lock.is_locked())
self.node_info.release_lock()
with mock.patch.object(self.node_info, 'release_lock',