From c2103f7058792fb3a55ffb9db2f8712162cdfe18 Mon Sep 17 00:00:00 2001
From: "James E. Blair" <jim@acmegating.com>
Date: Wed, 13 Mar 2024 08:59:57 -0700
Subject: [PATCH] Reset jobs behind non-mergeable cycle

In the case of a dependency cycle, we check the mergeability of
each change in the item before we try to merge any of them, and
dequeue the item if it looks like one of them won't be able to
merge.  However, that bypasses the normal behavior where we reset
changes behind failing items, which could lead to merging changes
that were tested with changes ahead that did not merge.

To correct this, update the cycle-can-not-be-merged dequeue stanza
with a reset, to mirror the stanza below which handles the failure
of any individual change to merge.

Change-Id: I52a9fc2da4dd89131722d69d2b5dea886eb3d51c
---
 tests/unit/test_circular_dependencies.py | 25 ++++++++++++++++++++++++
 zuul/manager/__init__.py                 |  5 +++++
 2 files changed, 30 insertions(+)

diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py
index 2d63dbee3f..1842cc7bef 100644
--- a/tests/unit/test_circular_dependencies.py
+++ b/tests/unit/test_circular_dependencies.py
@@ -4031,8 +4031,10 @@ class TestGithubCircularDependencies(ZuulTestCase):
         self.executor_server.hold_jobs_in_build = True
         A = self.fake_github.openFakePullRequest("gh/project", "master", "A")
         B = self.fake_github.openFakePullRequest("gh/project1", "master", "B")
+        C = self.fake_github.openFakePullRequest("gh/project1", "master", "B")
         A.addReview('derp', 'APPROVED')
         B.addReview('derp', 'APPROVED')
+        C.addReview('derp', 'APPROVED')
         B.addLabel("approved")
 
         # A <-> B
@@ -4045,6 +4047,10 @@ class TestGithubCircularDependencies(ZuulTestCase):
 
         self.fake_github.emitEvent(A.addLabel("approved"))
         self.waitUntilSettled()
+        # Put an indepedente change behind this pair to test that it
+        # gets reset.
+        self.fake_github.emitEvent(C.addLabel("approved"))
+        self.waitUntilSettled()
 
         # Change draft status of A so it can no longer merge. Note that we
         # don't send an event to test the "github doesn't send an event"
@@ -4060,6 +4066,7 @@ class TestGithubCircularDependencies(ZuulTestCase):
         self.assertEqual(len(B.comments), 2)
         self.assertFalse(A.is_merged)
         self.assertFalse(B.is_merged)
+        self.assertTrue(C.is_merged)
 
         self.assertIn("failed to merge",
                       A.comments[-1])
@@ -4078,6 +4085,24 @@ class TestGithubCircularDependencies(ZuulTestCase):
                       B.comments[-1]))
         self.assertFalse(re.search('Change .*? is needed',
                                    B.comments[-1]))
+        self.assertHistory([
+            dict(name="project1-job", result="SUCCESS",
+                 changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"),
+            dict(name="project-vars-job", result="SUCCESS",
+                 changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"),
+            dict(name="project-job", result="SUCCESS",
+                 changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"),
+            dict(name="project1-job", result="SUCCESS",
+                 changes=(f"{B.number},{B.head_sha} {A.number},{A.head_sha} "
+                          f"{C.number},{C.head_sha}")),
+            dict(name="project-vars-job", result="SUCCESS",
+                 changes=(f"{B.number},{B.head_sha} {A.number},{A.head_sha} "
+                          f"{C.number},{C.head_sha}")),
+            dict(name="project1-job", result="SUCCESS",
+                 changes=f"{C.number},{C.head_sha}"),
+            dict(name="project-vars-job", result="SUCCESS",
+                 changes=f"{C.number},{C.head_sha}"),
+        ], ordered=False)
 
     def test_dependency_refresh(self):
         # Test that when two changes are put into a cycle, the
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index c21cd1d455..62c0aec477 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -1733,6 +1733,11 @@ class PipelineManager(metaclass=ABCMeta):
                     self.reportItem(item)
                 except exceptions.MergeFailure:
                     pass
+                for item_behind in item.items_behind:
+                    log.info("Resetting builds for %s because the "
+                             "item ahead, %s, can not be merged" %
+                             (item_behind, item))
+                    self.cancelJobs(item_behind)
                 self.dequeueItem(item)
                 return (True, nnfi)