Cancel jobs of abandoned circular dep. change

When a change that is part of a circular dependency is abandoned we'd
set the item status to dequeued needing change. This will set all builds
as skipped, overwriting exiting builds.

This means that when the item was removed, we did not cancel any of the
builds. For normal builds this mainly waste resources, but if there are
paused builds, those will be leaked and continue running until the
executor is force-restarted.

The fix here is to cancel the jobs before setting it as dequeued needing
change.

Change-Id: If111fe1a21a1c944abcf460a6601293c255376d6

tests/fixtures/config/circular-dependencies/git/org_project7/playbooks/run.yaml

@@ -0,0 +1,7 @@
+- hosts: all
+  tasks:
+    - name: Pause and let child run
+      zuul_return:
+        data:
+          zuul:
+            pause: true

tests/fixtures/config/circular-dependencies/git/org_project7/zuul.yaml

@@ -0,0 +1,18 @@
+- job:
+    name: project7-parent-job
+    run: playbooks/run.yaml
+    deduplicate: true
+
+- job:
+    name: project7-child-job
+    deduplicate: true
+
+- project:
+    name: org/project7
+    queue: integrated
+    check:
+      jobs:
+        - project7-parent-job
+        - project7-child-job:
+            dependencies:
+              - project7-parent-job

tests/fixtures/config/circular-dependencies/main.yaml

@@ -12,6 +12,7 @@
           - org/project3
           - org/project5
           - org/project6
+          - org/project7
       github:
         untrusted-projects:
           - gh/project

tests/unit/test_circular_dependencies.py

@@ -1175,6 +1175,33 @@ class TestGerritCircularDependencies(ZuulTestCase):
         self.assertEqual(B.patchsets[-1]["approvals"][0]["type"], "Verified")
         self.assertEqual(B.patchsets[-1]["approvals"][0]["value"], "-1")
 
+    def test_abandon_cancel_jobs(self):
+        self.executor_server.hold_jobs_in_build = True
+        A = self.fake_gerrit.addFakeChange("org/project7", "master", "A")
+        B = self.fake_gerrit.addFakeChange("org/project7", "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"]
+        )
+
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.fake_gerrit.addEvent(B.getChangeAbandonedEvent())
+        self.waitUntilSettled()
+
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
+        self.assertHistory([
+            dict(name="project7-parent-job", result="ABORTED", changes="2,1 1,1"),
+        ], ordered=False)
+
     def test_cycle_merge_conflict(self):
         self.hold_merge_jobs_in_queue = True
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')

zuul/manager/__init__.py

@@ -478,6 +478,9 @@ class PipelineManager(metaclass=ABCMeta):
             for item_change in item.changes:
                 if item_change.equals(change):
                     if len(item.changes) > 1:
+                        # We need to cancel all jobs here as setting
+                        # dequeued needing change will skip all jobs.
+                        self.cancelJobs(item)
                         msg = ("Dependency cycle change "
                                f"{change.url} abandoned.")
                         item.setDequeuedNeedingChange(msg)