Record held node IDs with autohold request

These node IDs will be output with the 'zuul autohold-info' command.

Change-Id: I8f52d2b87b3bec6d3b8ecc2f69507049d905cad5

4 files changed, 10 insertions, 0 deletions

diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index dd73dd8b1..15bef5621 100644
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -1643,6 +1643,7 @@ class TestScheduler(ZuulTestCase):
         self.assertEqual('reason text', request.reason)
         self.assertEqual(1, request.max_count)
         self.assertEqual(0, request.current_count)
+        self.assertEqual([], request.nodes)
 
         # First check that successful jobs do not autohold
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
@@ -1693,8 +1694,10 @@ class TestScheduler(ZuulTestCase):
         self.assertEqual(held_node['comment'], "reason text")
 
         # The hold request current_count should have incremented
+        # and we should have recorded the held node ID.
         request2 = self.zk.getHoldRequest(request.id)
         self.assertEqual(request.current_count + 1, request2.current_count)
+        self.assertEqual(1, len(request2.nodes))
 
         # Another failed change should not hold any more nodes
         C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
diff --git a/zuul/cmd/client.py b/zuul/cmd/client.py
index e8f457966..79c4e6184 100755
--- a/zuul/cmd/client.py
+++ b/zuul/cmd/client.py
@@ -470,6 +470,7 @@ class Client(zuul.cmd.ZuulApp):
         print("Current Count: %s" % request['current_count'])
         print("Node Expiration: %s" % request['node_expiration'])
         print("Reason: %s" % request['reason'])
+        print("Held Nodes: %s" % request['nodes'])
 
         return True
 
diff --git a/zuul/model.py b/zuul/model.py
index 0191d2c0e..563d4f478 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -4658,6 +4658,7 @@ class HoldRequest(object):
         # When max_count == current_count, hold request can no longer be used.
         self.max_count = 1
         self.current_count = 0
+        self.nodes = []
 
     def __str__(self):
         return "<HoldRequest %s: tenant=%s project=%s job=%s ref_filter=%s>" \
@@ -4677,6 +4678,7 @@ class HoldRequest(object):
         obj.current_count = data.get('current_count')
         obj.reason = data.get('reason')
         obj.node_expiration = data.get('node_expiration')
+        obj.nodes = data.get('nodes', [])
         return obj
 
     def toDict(self):
@@ -4693,6 +4695,7 @@ class HoldRequest(object):
         d['current_count'] = self.current_count
         d['reason'] = self.reason
         d['node_expiration'] = self.node_expiration
+        d['nodes'] = self.nodes
         return d
 
     def updateFromDict(self, d):
diff --git a/zuul/nodepool.py b/zuul/nodepool.py
index 703e7937f..53de2b1b5 100644
--- a/zuul/nodepool.py
+++ b/zuul/nodepool.py
@@ -191,6 +191,7 @@ class Nodepool(object):
                 node.hold_expiration = request.node_expiration
             self.sched.zk.storeNode(node)
 
+        request.nodes += [node.id for node in nodes]
         request.current_count += 1
 
         # Give ourselves a few seconds to try to obtain the lock rather than
@@ -203,6 +204,8 @@ class Nodepool(object):
             # If we fail to update the request count, we won't consider it
             # a real autohold error by passing the exception up. It will
             # just get used more than the original count specified.
+            # It's possible to leak some held nodes, though, which would
+            # require manual node deletes.
             self.log.exception("Unable to update hold request %s:", request)
         finally:
             # Although any exceptions thrown here are handled higher up in