diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py index a49d8895b2..6632cf883f 100644 --- a/tests/unit/test_circular_dependencies.py +++ b/tests/unit/test_circular_dependencies.py @@ -1414,3 +1414,37 @@ class TestGithubCircularDependencies(ZuulTestCase): self.assertEqual(len(B.comments), 2) self.assertTrue(A.is_merged) self.assertTrue(B.is_merged) + + def test_cycle_failed_reporting(self): + 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") + A.addReview('derp', 'APPROVED') + B.addReview('derp', 'APPROVED') + B.addLabel("approved") + + # A <-> B + A.body = "{}\n\nDepends-On: {}\n".format( + A.subject, B.url + ) + B.body = "{}\n\nDepends-On: {}\n".format( + B.subject, A.url + ) + + self.fake_github.emitEvent(A.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" + # case. + A.draft = True + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(len(A.comments), 2) + self.assertEqual(len(B.comments), 2) + self.assertFalse(A.is_merged) + self.assertFalse(B.is_merged) diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index 2d9abde279..d85dcd5d91 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -486,6 +486,10 @@ class PipelineManager(metaclass=ABCMeta): queue_config.allow_circular_dependencies ) + def canMergeCycle(self, bundle): + """Check if the cycle still fulfills the pipeline's ready criteria.""" + return True + def updateBundle(self, item, change_queue, cycle): if not cycle: return @@ -1088,6 +1092,19 @@ class PipelineManager(metaclass=ABCMeta): can_report = can_report and ( item.isBundleFailing() or item.didBundleFinish() ) + # Before starting to merge the cycle items, make sure they + # can still be merged, to reduce the chance of a partial merge. + if ( + can_report + and not item.bundle.started_reporting + and not self.canMergeCycle(item.bundle) + ): + item.bundle.cannot_merge = True + failing_reasons.append("cycle can not be merged") + log.debug( + "Dequeuing item %s because cycle can no longer merge", + item + ) item.bundle.started_reporting = can_report if can_report: @@ -1100,7 +1117,10 @@ class PipelineManager(metaclass=ABCMeta): "item ahead, %s, failed to merge" % (item_behind.change, item)) self.cancelJobs(item_behind) - if item.bundle and not item.isBundleFailing(): + # Only re-reported items in the cycle when we encounter a merge + # failure for a successful bundle. + if (item.bundle and not ( + item.isBundleFailing() or item.cannotMergeBundle())): item.bundle.failed_reporting = True self.reportProcessedBundleItems(item) self.dequeueItem(item) @@ -1379,6 +1399,10 @@ class PipelineManager(metaclass=ABCMeta): log.debug("No jobs for change %s", item.change) actions = self.pipeline.no_jobs_actions item.setReportedResult('NO_JOBS') + elif item.cannotMergeBundle(): + log.debug("Bundle can not be merged") + actions = self.pipeline.failure_actions + item.setReportedResult("FAILURE") elif item.isBundleFailing(): log.debug("Bundle is failing") actions = self.pipeline.failure_actions diff --git a/zuul/manager/dependent.py b/zuul/manager/dependent.py index ea0b4ab3a0..7b6667fef4 100644 --- a/zuul/manager/dependent.py +++ b/zuul/manager/dependent.py @@ -50,10 +50,25 @@ class DependentPipelineManager(SharedQueuePipelineManager): source = change.project.source if not source.canMerge(change, self.getSubmitAllowNeeds(), event=event): - log.debug("Change %s can not merge, ignoring", change) + log.debug("Change %s can not merge", change) return False return True + def canMergeCycle(self, bundle): + """Check if the cycle still fulfills the pipeline's ready criteria.""" + for item in bundle.items: + source = item.change.project.source + if not source.canMerge( + item.change, + self.getSubmitAllowNeeds(), + event=item.event, + allow_refresh=True, + ): + log = get_annotated_logger(self.log, item.event) + log.debug("Change %s can no longer be merged", item.change) + return False + return True + def enqueueChangesBehind(self, change, event, quiet, ignore_requirements, change_queue, history=None, dependency_graph=None): diff --git a/zuul/model.py b/zuul/model.py index 3114cebc3f..291c2c4aed 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -2432,6 +2432,11 @@ class QueueItem(object): return self.bundle.started_reporting return False + def cannotMergeBundle(self): + if self.bundle: + return self.bundle.cannot_merge + return False + def didMergerFail(self): return self.current_build_set.unable_to_merge @@ -3208,6 +3213,7 @@ class Bundle: self.items = [] self.started_reporting = False self.failed_reporting = False + self.cannot_merge = False def __repr__(self): return '