Mark nodes as USED when deleting autohold

Marking the nodes as USED will allow nodepool to delete them.
If we are unsuccessful in marking any of the held nodes as used,
we simply log the error and try again at some future point until
all nodes are eventually marked, allowing the hold request to be
deleted.

Change-Id: Idd41c58b5cce0aa9b6cd186fa5c33066012790b8

tests/unit/test_scheduler.py

@@ -1783,7 +1783,6 @@ class TestScheduler(ZuulTestCase):
                 held_node = node
                 break
         self.assertIsNotNone(held_node)
-
         # Validate node has recorded the failed job
         self.assertEqual(
             held_node['hold_job'],
@@ -1819,6 +1818,12 @@ class TestScheduler(ZuulTestCase):
         request3 = self.zk.getHoldRequest(request2.id)
         self.assertEqual(request2.current_count, request3.current_count)
 
+        # Deleting hold request should set held nodes to used
+        self.zk.deleteHoldRequest(request3)
+        node_states = [n['state'] for n in self.fake_nodepool.getNodes()]
+        self.assertEqual(3, len(node_states))
+        self.assertEqual([zuul.model.STATE_USED] * 3, node_states)
+
     @simple_layout('layouts/autohold.yaml')
     def test_autohold_info(self):
         client = zuul.rpcclient.RPCClient('127.0.0.1',

zuul/zk.py

@@ -583,12 +583,58 @@ class ZooKeeper(object):
             path = self.HOLD_REQUEST_ROOT + "/" + hold_request.id
             self.client.set(path, hold_request.serialize())
 
+    def _markHeldNodesAsUsed(self, hold_request):
+        '''
+        :returns: True if all nodes marked USED, False otherwise.
+        '''
+        failure = False
+        for node_id in hold_request.nodes:
+            node = self.getNode(node_id)
+            if not node or node['state'] == zuul.model.STATE_USED:
+                continue
+
+            node['state'] = zuul.model.STATE_USED
+
+            name = None
+            label = None
+            if 'name' in node:
+                name = node['name']
+            if 'label' in node:
+                label = node['label']
+
+            node_obj = zuul.model.Node(name, label)
+            node_obj.updateFromDict(node)
+
+            try:
+                self.lockNode(node_obj, blocking=False)
+                self.storeNode(node_obj)
+            except Exception:
+                self.log.exception("Cannot change HELD node state to USED "
+                                   "for node %s in request %s",
+                                   node_obj.id, hold_request.id)
+                failure = True
+            finally:
+                try:
+                    if node_obj.lock:
+                        self.unlockNode(node_obj)
+                except Exception:
+                    self.log.exception(
+                        "Failed to unlock HELD node %s for request %s",
+                        node_obj.id, hold_request.id)
+
+        return not failure
+
     def deleteHoldRequest(self, hold_request):
         '''
         Delete a hold request.
 
         :param HoldRequest hold_request: Object representing the hold request.
         '''
+        if not self._markHeldNodesAsUsed(hold_request):
+            self.log.info("Unable to delete hold request %s because "
+                          "not all nodes marked as USED.", hold_request.id)
+            return
+
         path = self.HOLD_REQUEST_ROOT + "/" + hold_request.id
         try:
             self.client.delete(path, recursive=True)