Merge "Fix node request failures in dependency cycles"

tests/fixtures/layouts/circular-deps-node-failure.yaml

@@ -0,0 +1,62 @@
+- queue:
+    name: integrated
+    allow-circular-dependencies: true
+
+- pipeline:
+    name: gate
+    manager: dependent
+    success-message: Build succeeded (gate).
+    require:
+      gerrit:
+        approval:
+          - Approved: 1
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - Approved: 1
+    success:
+      gerrit:
+        Verified: 2
+        submit: true
+    failure:
+      gerrit:
+        Verified: -2
+    start:
+      gerrit:
+        Verified: 0
+    precedence: high
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/run.yaml
+    nodeset:
+      nodes:
+        - label: debian
+          name: controller
+
+- job:
+    name: common-job
+
+- job:
+    name: project1-job
+
+- job:
+    name: project2-job
+
+- project:
+    name: org/project1
+    queue: integrated
+    gate:
+      jobs:
+        - common-job
+        - project1-job
+
+- project:
+    name: org/project2
+    queue: integrated
+    gate:
+      jobs:
+        - common-job
+        - project2-job

tests/unit/test_circular_dependencies.py

@@ -464,6 +464,45 @@ class TestGerritCircularDependencies(ZuulTestCase):
         self.assertEqual(A.data["status"], "NEW")
         self.assertEqual(B.data["status"], "NEW")
 
+    @simple_layout('layouts/circular-deps-node-failure.yaml')
+    def test_cycle_failed_node_request(self):
+        # Test a node request failure as part of a dependency cycle
+
+        # Pause nodepool so we can fail the node request later
+        self.fake_nodepool.pause()
+
+        A = self.fake_gerrit.addFakeChange("org/project1", "master", "A")
+        B = self.fake_gerrit.addFakeChange("org/project2", "master", "B")
+
+        # A <-> B (via commit-depends)
+        A.data["commitMessage"] = "{}\n\nDepends-On: {}\n".format(
+            A.subject, B.data["url"]
+        )
+        B.data["commitMessage"] = "{}\n\nDepends-On: {}\n".format(
+            B.subject, A.data["url"]
+        )
+
+        A.addApproval("Code-Review", 2)
+        B.addApproval("Code-Review", 2)
+        B.addApproval("Approved", 1)
+
+        self.fake_gerrit.addEvent(A.addApproval("Approved", 1))
+        self.waitUntilSettled()
+
+        # Fail the node request and unpause
+        req = self.fake_nodepool.getNodeRequests()
+        self.fake_nodepool.addFailRequest(req[0])
+
+        self.fake_nodepool.unpause()
+        self.waitUntilSettled()
+
+        self.assertEqual(A.reported, 2)
+        self.assertEqual(B.reported, 2)
+        self.assertIn("bundle", A.messages[-1])
+        self.assertIn("bundle", B.messages[-1])
+        self.assertEqual(A.data["status"], "NEW")
+        self.assertEqual(B.data["status"], "NEW")
+
     def test_failing_cycle_behind_failing_change(self):
         self.executor_server.hold_jobs_in_build = True
         A = self.fake_gerrit.addFakeChange("org/project", "master", "A")

zuul/manager/__init__.py

@@ -950,7 +950,7 @@ class PipelineManager(metaclass=ABCMeta):
         jobs_to_cancel = item.getJobs()
 
         for job in jobs_to_cancel:
-            self.sched.cancelJob(old_build_set, job)
+            self.sched.cancelJob(old_build_set, job, final=True)
 
         # Don't reset builds for a failing bundle when it has already started
         # reporting, to keep available build results. Those items will be