Merge "Clarify terminology around node request locks" into feature/zuulv3

nodepool/launcher.py

@@ -432,11 +432,11 @@ class CleanupWorker(BaseCleanupWorker):
         zk = self._nodepool.getZK()
         requests = zk.getNodeRequests()
         now = time.time()
-        for lock in zk.nodeRequestLockIterator():
-            if lock.id in requests:
+        for lock_stat in zk.nodeRequestLockStatsIterator():
+            if lock_stat.lock_id in requests:
                 continue
-            if (now - lock.stat.mtime/1000) > LOCK_CLEANUP:
-                zk.deleteNodeRequestLock(lock.id)
+            if (now - lock_stat.stat.mtime/1000) > LOCK_CLEANUP:
+                zk.deleteNodeRequestLock(lock_stat.lock_id)
 
     def _cleanupLeakedInstances(self):
         '''

nodepool/tests/__init__.py

@@ -425,7 +425,7 @@ class DBTestCase(BaseTestCase):
     def waitForNodeRequestLockDeletion(self, request_id):
         while True:
             exists = False
-            for lock_id in self.zk.getNodeRequestLocks():
+            for lock_id in self.zk.getNodeRequestLockIDs():
                 if request_id == lock_id:
                     exists = True
                     break

nodepool/tests/test_zk.py

@@ -589,29 +589,29 @@ class TestZooKeeper(tests.DBTestCase):
         with testtools.ExpectedException(StopIteration):
             next(i)
 
-    def test_getNodeRequestLocks(self):
+    def test_getNodeRequestLockIDs(self):
         req = self._create_node_request()
         self.zk.lockNodeRequest(req, blocking=False)
-        locks = self.zk.getNodeRequestLocks()
-        self.assertEqual(1, len(locks))
-        self.assertEqual(req.id, locks[0])
+        lock_ids = self.zk.getNodeRequestLockIDs()
+        self.assertEqual(1, len(lock_ids))
+        self.assertEqual(req.id, lock_ids[0])
         self.zk.unlockNodeRequest(req)
         self.zk.deleteNodeRequest(req)
 
-    def test_getNodeRequestLock(self):
+    def test_getNodeRequestLockStats(self):
         req = self._create_node_request()
         self.zk.lockNodeRequest(req, blocking=False)
-        lock = self.zk.getNodeRequestLock(req.id)
-        self.assertEqual(lock.id, req.id)
-        self.assertIsNotNone(lock.stat)
+        lock_stats = self.zk.getNodeRequestLockStats(req.id)
+        self.assertEqual(lock_stats.lock_id, req.id)
+        self.assertIsNotNone(lock_stats.stat)
         self.zk.unlockNodeRequest(req)
         self.zk.deleteNodeRequest(req)
 
-    def test_nodeRequestLockIterator(self):
+    def test_nodeRequestLockStatsIterator(self):
         req = self._create_node_request()
         self.zk.lockNodeRequest(req, blocking=False)
-        i = self.zk.nodeRequestLockIterator()
-        self.assertEqual(zk.NodeRequestLock(req.id), next(i))
+        i = self.zk.nodeRequestLockStatsIterator()
+        self.assertEqual(zk.NodeRequestLockStats(req.id), next(i))
         with testtools.ExpectedException(StopIteration):
             next(i)
         self.zk.unlockNodeRequest(req)
@@ -634,11 +634,11 @@ class TestZooKeeper(tests.DBTestCase):
         self.zk.deleteNodeRequest(req)
 
         # We expect the lock to linger even after the request is deleted
-        locks = self.zk.getNodeRequestLocks()
-        self.assertEqual(1, len(locks))
-        self.assertEqual(req.id, locks[0])
-        self.zk.deleteNodeRequestLock(locks[0])
-        self.assertEqual([], self.zk.getNodeRequestLocks())
+        lock_ids = self.zk.getNodeRequestLockIDs()
+        self.assertEqual(1, len(lock_ids))
+        self.assertEqual(req.id, lock_ids[0])
+        self.zk.deleteNodeRequestLock(lock_ids[0])
+        self.assertEqual([], self.zk.getNodeRequestLockIDs())
 
 
 class TestZKModel(tests.BaseTestCase):

nodepool/zk.py

@@ -330,25 +330,25 @@ class ImageUpload(BaseModel):
         return o
 
 
-class NodeRequestLock(object):
+class NodeRequestLockStats(object):
     '''
-    Class representing a node request lock.
+    Class holding the stats of a node request lock znode.
 
     This doesn't need to derive from BaseModel since this class exists only
     to associate the znode stats with the lock.
     '''
-    def __init__(self, id=None):
-        self.id = id
+    def __init__(self, lock_id=None):
+        self.lock_id = lock_id
         self.stat = None
 
     def __eq__(self, other):
-        if isinstance(other, NodeRequestLock):
-            return (self.id == other.id)
+        if isinstance(other, NodeRequestLockStats):
+            return (self.lock_id == other.lock_id)
         else:
             return False
 
     def __repr__(self):
-        return '<NodeRequestLock %s>' % self.id
+        return '<NodeRequestLockStats %s>' % self.lock_id
 
 
 class NodeRequest(BaseModel):
@@ -1338,17 +1338,17 @@ class ZooKeeper(object):
 
         return sorted(requests)
 
-    def getNodeRequestLocks(self):
+    def getNodeRequestLockIDs(self):
         '''
-        Get the current list of all node request locks.
+        Get the current list of all node request lock ids.
         '''
         try:
-            locks = self.client.get_children(self.REQUEST_LOCK_ROOT)
+            lock_ids = self.client.get_children(self.REQUEST_LOCK_ROOT)
         except kze.NoNodeError:
             return []
-        return locks
+        return lock_ids
 
-    def getNodeRequestLock(self, lock):
+    def getNodeRequestLockStats(self, lock_id):
         '''
         Get the data for a specific node request lock.
 
@@ -1356,26 +1356,26 @@ class ZooKeeper(object):
         main purpose for this method is to get the ZK stat data for the lock
         so we can inspect it and use it for lock deletion.
 
-        :param str lock: The node request lock ID.
+        :param str lock_id: The node request lock ID.
 
-        :returns: A NodeRequestLock object.
+        :returns: A NodeRequestLockStats object.
         '''
-        path = self._requestLockPath(lock)
+        path = self._requestLockPath(lock_id)
         try:
             data, stat = self.client.get(path)
         except kze.NoNodeError:
             return None
-        d = NodeRequestLock(lock)
+        d = NodeRequestLockStats(lock_id)
         d.stat = stat
         return d
 
-    def deleteNodeRequestLock(self, lock):
+    def deleteNodeRequestLock(self, lock_id):
         '''
-        Delete the znode for a node request lock.
+        Delete the znode for a node request lock id.
 
-        :param str lock: The lock ID.
+        :param str lock_id: The lock ID.
         '''
-        path = self._requestLockPath(lock)
+        path = self._requestLockPath(lock_id)
         try:
             self.client.delete(path, recursive=True)
         except kze.NoNodeError:
@@ -1646,14 +1646,14 @@ class ZooKeeper(object):
             if node:
                 yield node
 
-    def nodeRequestLockIterator(self):
+    def nodeRequestLockStatsIterator(self):
         '''
         Utility generator method for iterating through all nodes request locks.
         '''
-        for lock_id in self.getNodeRequestLocks():
-            lock = self.getNodeRequestLock(lock_id)
-            if lock:
-                yield lock
+        for lock_id in self.getNodeRequestLockIDs():
+            lock_stats = self.getNodeRequestLockStats(lock_id)
+            if lock_stats:
+                yield lock_stats
 
     def nodeRequestIterator(self):
         '''