From e85fb93d1de9cd88de89b41b939f3f1028fd71d8 Mon Sep 17 00:00:00 2001
From: Tristan Cacqueray <tdecacqu@redhat.com>
Date: Mon, 16 Sep 2019 19:12:21 +0000
Subject: [PATCH] Store a list of held nodes per held build in hold request

Instead of storing a flat list of nodes per hold request, this
change updates the request nodes attribute to become a list of
dictionary with the build uuid and the held node list.

Change-Id: I9e50e7ccadc58fb80d5e80d9f5aac70eb7501a36
---
 tests/unit/test_scheduler.py |  1 +
 zuul/model.py                | 10 ++++++++++
 zuul/nodepool.py             |  7 +++++--
 zuul/scheduler.py            |  2 +-
 zuul/zk.py                   | 10 +++++++++-
 5 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index d788c94ee8..b24cb284d7 100644
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -1797,6 +1797,7 @@ class TestScheduler(ZuulTestCase):
         request2 = self.zk.getHoldRequest(request.id)
         self.assertEqual(request.current_count + 1, request2.current_count)
         self.assertEqual(1, len(request2.nodes))
+        self.assertEqual(1, len(request2.nodes[0]["nodes"]))
 
         # Another failed change should not hold any more nodes
         C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
diff --git a/zuul/model.py b/zuul/model.py
index 41730f1c7b..f137d2c94c 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -4727,6 +4727,16 @@ class HoldRequest(object):
         # When max_count == current_count, hold request can no longer be used.
         self.max_count = 1
         self.current_count = 0
+
+        # The hold request 'nodes' attribute is a list of dictionaries
+        # (one list entry per hold request count) containing the build
+        # ID (build) and a list of nodes (nodes) held for that build.
+        # Example:
+        #
+        #  hold_request.nodes = [
+        #    { 'build': 'ca01...', 'nodes': ['00000001', '00000002'] },
+        #    { 'build': 'fb72...', 'nodes': ['00000003', '00000004'] },
+        #  ]
         self.nodes = []
 
     def __str__(self):
diff --git a/zuul/nodepool.py b/zuul/nodepool.py
index c67dedfa91..eba2ecff35 100644
--- a/zuul/nodepool.py
+++ b/zuul/nodepool.py
@@ -169,7 +169,7 @@ class Nodepool(object):
             except Exception:
                 log.exception("Unable to unlock node request %s", request)
 
-    def holdNodeSet(self, nodeset, request):
+    def holdNodeSet(self, nodeset, request, build):
         '''
         Perform a hold on the given set of nodes.
 
@@ -192,7 +192,10 @@ class Nodepool(object):
                 node.hold_expiration = request.node_expiration
             self.sched.zk.storeNode(node)
 
-        request.nodes += [node.id for node in nodes]
+        request.nodes.append(dict(
+            build=build.uuid,
+            nodes=[node.id for node in nodes],
+        ))
         request.current_count += 1
 
         # Request has been used at least the maximum number of times so set
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 75fd25c70e..bc0954470e 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -1436,7 +1436,7 @@ class Scheduler(threading.Thread):
         request = self._getAutoholdRequest(build)
         self.log.debug("Got autohold %s", request)
         if request is not None:
-            self.nodepool.holdNodeSet(build.nodeset, request)
+            self.nodepool.holdNodeSet(build.nodeset, request, build)
 
     def _doBuildCompletedEvent(self, event):
         build = event.build
diff --git a/zuul/zk.py b/zuul/zk.py
index d267dfc348..4e069bd581 100644
--- a/zuul/zk.py
+++ b/zuul/zk.py
@@ -585,10 +585,18 @@ class ZooKeeper(object):
 
     def _markHeldNodesAsUsed(self, hold_request):
         '''
+        Changes the state for each held node for the hold request to 'used'.
+
         :returns: True if all nodes marked USED, False otherwise.
         '''
+        def getHeldNodeIDs(request):
+            node_ids = []
+            for data in request.nodes:
+                node_ids += data['nodes']
+            return node_ids
+
         failure = False
-        for node_id in hold_request.nodes:
+        for node_id in getHeldNodeIDs(hold_request):
             node = self.getNode(node_id)
             if not node or node['state'] == zuul.model.STATE_USED:
                 continue