diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 0016628090..6544e39607 100644 --- a/tests/unit/test_scheduler.py +++ b/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', diff --git a/zuul/zk.py b/zuul/zk.py index e3691ce953..d267dfc348 100644 --- a/zuul/zk.py +++ b/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)