diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py index efcbb6a6f0..28ca585588 100644 --- a/tests/unit/test_circular_dependencies.py +++ b/tests/unit/test_circular_dependencies.py @@ -16,7 +16,6 @@ import re import textwrap -from unittest import skip from zuul.model import PromoteEvent @@ -4113,7 +4112,6 @@ class TestGithubCircularDependencies(ZuulTestCase): self.assertHistory([ dict(name="project-job", result="ABORTED", changes=f"{A.number},{A.head_sha}"), - # TODO: changed for safety check dict(name="project-job", result="ABORTED", changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), dict(name="project-job", result="SUCCESS", @@ -4165,16 +4163,12 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.subject)) self.waitUntilSettled() - # Validate that the Github check-run is still in progress - # and wasn't cancelled. - - # TODO: we actually do cancel the check-run now due to the - # safety check in processOneItem. In the future we should be - # able to avoid that. + # Validate that the Github check-run was cancelled and a new + # one restarted. check_runs = self.fake_github.getCommitChecks("gh/project", A.head_sha) - self.assertEqual(len(check_runs), 1) - check_run = check_runs[0] - # self.assertEqual(check_run["status"], "in_progress") + self.assertEqual(len(check_runs), 2) + self.assertEqual(check_runs[0]["status"], "in_progress") + self.assertEqual(check_runs[1]["status"], "completed") self.executor_server.hold_jobs_in_build = False self.executor_server.release() @@ -4183,7 +4177,6 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): self.assertHistory([ dict(name="project-job", result="ABORTED", changes=f"{A.number},{A.head_sha}"), - # TODO: changed for safety check dict(name="project-job", result="ABORTED", changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), dict(name="project-job", result="SUCCESS", @@ -4193,8 +4186,7 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}", ref="refs/pull/2/head"), ], ordered=False) - # TODO: We shouldn't need this in the future, but for now, - # verify that since we are issuing two check runs, they both + # Verify that since we are issuing two check runs, they both # complete. check_runs = self.fake_github.getCommitChecks("gh/project", A.head_sha) self.assertEqual(len(check_runs), 2) @@ -4212,182 +4204,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): self.assertEqual(check_run["status"], "completed") self.assertNotEqual(check_runs[0]["id"], check_runs[1]["id"]) - @skip("Disabled due to safety check") - @simple_layout('layouts/dependency_removal_gate.yaml', driver='github') - def test_dependency_removal_gate_orig(self): - # Dependency cycles can be updated without uploading new - # patchsets. This test exercises such changes. - - self.executor_server.hold_jobs_in_build = True - tenant = self.scheds.first.sched.abide.tenants.get("tenant-one") - pipeline = tenant.layout.pipelines["gate"] - - A = self.fake_github.openFakePullRequest("gh/project1", "master", "A") - B = self.fake_github.openFakePullRequest("gh/project2", "master", "B") - C = self.fake_github.openFakePullRequest("gh/project3", "master", "C") - D = self.fake_github.openFakePullRequest("gh/project", "master", "D") - E = self.fake_github.openFakePullRequest("gh/project", "master", "E") - - # Because we call setConfiguration immediately upon removing a - # change, in order to fully exercise setConfiguration on a - # queue that has been reloaded from ZK with potentially stale - # bundle data, we need to remove a change from the bundle - # twice. ABC is the main bundle we're interested in. E is a - # sacrificial change to force a bundle update when it's - # removed. - - # E->C, A->E, B->A, C->B - E.body = "{}\n\nDepends-On: {}\n".format( - E.subject, C.url - ) - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, E.url - ) - B.body = "{}\n\nDepends-On: {}\n".format( - B.subject, A.url - ) - C.body = "{}\n\nDepends-On: {}\n".format( - C.subject, B.url - ) - - A.addLabel("approved") - B.addLabel("approved") - D.addLabel("approved") - E.addLabel("approved") - - self.fake_github.emitEvent(C.addLabel("approved")) - self.waitUntilSettled() - expected_cycle = {A.number, B.number, C.number, E.number} - self.assertEqual(len(list(pipeline.getAllItems())), 4) - for item in pipeline.getAllItems(): - cycle = {i.change.number for i in item.bundle.items} - self.assertEqual(expected_cycle, cycle) - - # Now we remove the dependency on E. The change E itself - # won't be removed yet. - - # A->C, B->A, C->B - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, C.url - ) - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - self.assertEqual(len(list(pipeline.getAllItems())), 4) - self.assertIsNone(pipeline.getAllItems()[0].bundle) - # E at the head is alone - # The next 3 are a cycle - expected_cycle = {A.number, B.number, C.number} - for item in pipeline.getAllItems()[1:]: - cycle = {i.change.number for i in item.bundle.items} - self.assertEqual(expected_cycle, cycle) - - # Now remove E from the queue by forcing a dependency on D, - # which is not enqueued. - E.body = "{}\n\nDepends-On: {}\n".format( - E.subject, D.url - ) - self.fake_github.emitEvent(E.getPullRequestEditedEvent(E.body)) - self.waitUntilSettled() - self.assertEqual(len(list(pipeline.getAllItems())), 3) - expected_cycle = {A.number, B.number, C.number} - for item in pipeline.getAllItems(): - cycle = {i.change.number for i in item.bundle.items} - self.assertEqual(expected_cycle, cycle) - - # Now remove all dependencies for the three remaining changes. - A.body = A.subject - B.body = B.subject - C.body = C.subject - - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body)) - self.waitUntilSettled() - self.fake_github.emitEvent(C.getPullRequestEditedEvent(C.body)) - self.waitUntilSettled() - self.assertEqual(len(list(pipeline.getAllItems())), 3) - for item in pipeline.getAllItems(): - self.assertIsNone(item.bundle) - - # Remove the first change from the queue by forcing a - # dependency on D. - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, D.url - ) - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - self.assertEqual(len(list(pipeline.getAllItems())), 2) - for item in pipeline.getAllItems(): - self.assertIsNone(item.bundle) - - # B and C are still in the queue. Verify that we can put them - # back into a bundle. - C.body = "{}\n\nDepends-On: {}\n".format( - C.subject, B.url - ) - self.fake_github.emitEvent(C.getPullRequestEditedEvent(C.body)) - self.waitUntilSettled() - B.body = "{}\n\nDepends-On: {}\n".format( - B.subject, C.url - ) - self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body)) - self.waitUntilSettled() - self.assertEqual(len(list(pipeline.getAllItems())), 2) - expected_cycle = {B.number, C.number} - for item in pipeline.getAllItems(): - cycle = {i.change.number for i in item.bundle.items} - self.assertEqual(expected_cycle, cycle) - - # All done. - self.executor_server.hold_jobs_in_build = False - self.executor_server.release() - self.waitUntilSettled() - - self.assertHistory([ - dict(name="project-job", result="ABORTED", - changes=f"{E.number},{E.head_sha} {A.number},{A.head_sha} " - f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="common-job", result="ABORTED", - changes=f"{E.number},{E.head_sha} {A.number},{A.head_sha} " - f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="project1-job", result="ABORTED", - changes=f"{E.number},{E.head_sha} {A.number},{A.head_sha} " - f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="project2-job", result="ABORTED", - changes=f"{E.number},{E.head_sha} {A.number},{A.head_sha} " - f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="project3-job", result="ABORTED", - changes=f"{E.number},{E.head_sha} {A.number},{A.head_sha} " - f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - - dict(name="common-job", result="ABORTED", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha} " - f"{C.number},{C.head_sha}"), - dict(name="project1-job", result="ABORTED", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha} " - f"{C.number},{C.head_sha}"), - dict(name="project2-job", result="ABORTED", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha} " - f"{C.number},{C.head_sha}"), - dict(name="project3-job", result="ABORTED", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha} " - f"{C.number},{C.head_sha}"), - - dict(name="common-job", result="SUCCESS", - changes=f"{B.number},{B.head_sha}"), - dict(name="project2-job", result="SUCCESS", - changes=f"{B.number},{B.head_sha}"), - - dict(name="common-job", result="SUCCESS", - changes=f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="project3-job", result="SUCCESS", - changes=f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - ], ordered=False) - @simple_layout('layouts/dependency_removal_gate.yaml', driver='github') def test_dependency_removal_gate(self): - # TODO: After removal of the safety check, remove this test - # and use the _orig version. # Dependency cycles can be updated without uploading new # patchsets. This test exercises such changes. @@ -4504,73 +4322,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): for build in self.history[-3:]: self.assertEqual(build.result, 'SUCCESS') - @skip("Disabled due to safety check") - @simple_layout('layouts/dependency_removal_gate.yaml', driver='github') - def test_dependency_addition_gate_orig(self): - # Dependency cycles can be updated without uploading new - # patchsets. This test exercises such changes. - - self.executor_server.hold_jobs_in_build = True - tenant = self.scheds.first.sched.abide.tenants.get("tenant-one") - pipeline = tenant.layout.pipelines["gate"] - - A = self.fake_github.openFakePullRequest("gh/project1", "master", "A") - B = self.fake_github.openFakePullRequest("gh/project2", "master", "B") - - B.addLabel("approved") - - self.fake_github.emitEvent(A.addLabel("approved")) - self.waitUntilSettled() - - expected_cycle = {A.number} - self.assertEqual(len(list(pipeline.getAllItems())), 1) - for item in pipeline.getAllItems(): - self.assertIsNone(item.bundle) - - B.body = "{}\n\nDepends-On: {}\n".format( - B.subject, A.url - ) - self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body)) - self.waitUntilSettled() - - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, B.url - ) - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - self.assertEqual(len(list(pipeline.getAllItems())), 2) - expected_cycle = {A.number, B.number} - for item in pipeline.getAllItems(): - cycle = {i.change.number for i in item.bundle.items} - self.assertEqual(expected_cycle, cycle) - - # All done. - self.executor_server.hold_jobs_in_build = False - self.executor_server.release() - self.waitUntilSettled() - - self.assertHistory([ - # Change 1 first enqueue - dict(name="project1-job", result="ABORTED", - changes=f"{A.number},{A.head_sha}"), - dict(name="common-job", result="ABORTED", - changes=f"{A.number},{A.head_sha}"), - - # Change 1 bundle update - dict(name="project1-job", result="SUCCESS", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), - dict(name="common-job", result="SUCCESS", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), - - # Change 2 bundle update - dict(name="project2-job", result="SUCCESS", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), - ], ordered=False) - @simple_layout('layouts/dependency_removal_gate.yaml', driver='github') def test_dependency_addition_gate(self): - # TODO: After removal of the safety check, remove this test - # and use the _orig version. # Dependency cycles can be updated without uploading new # patchsets. This test exercises such changes. @@ -4627,167 +4380,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): expected_cycle = {c.number for c in bundles[x]} self.assertEqual(expected_cycle, cycle) - @skip("Disabled due to safety check") - @simple_layout('layouts/dependency_removal_check.yaml', driver='github') - def test_dependency_removal_check_orig(self): - # Dependency cycles can be updated without uploading new - # patchsets. This test exercises such changes. - - self.executor_server.hold_jobs_in_build = True - tenant = self.scheds.first.sched.abide.tenants.get("tenant-one") - pipeline = tenant.layout.pipelines["check"] - - A = self.fake_github.openFakePullRequest("gh/project1", "master", "A") - B = self.fake_github.openFakePullRequest("gh/project2", "master", "B") - C = self.fake_github.openFakePullRequest("gh/project3", "master", "C") - D = self.fake_github.openFakePullRequest("gh/project", "master", "D") - E = self.fake_github.openFakePullRequest("gh/project", "master", "E") - - # Because we call setConfiguration immediately upon removing a - # change, in order to fully exercise setConfiguration on a - # queue that has been reloaded from ZK with potentially stale - # bundle data, we need to remove a change from the bundle - # twice. ABC is the main bundle we're interested in. E is a - # sacrificial change to force a bundle update when it's - # removed. - - # E->C, A->E, B->A, C->B - E.body = "{}\n\nDepends-On: {}\n".format( - E.subject, C.url - ) - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, E.url - ) - B.body = "{}\n\nDepends-On: {}\n".format( - B.subject, A.url - ) - C.body = "{}\n\nDepends-On: {}\n".format( - C.subject, B.url - ) - - self.fake_github.emitEvent(C.getPullRequestOpenedEvent()) - self.waitUntilSettled() - abce = [A, B, C, E] - self.assertEqual(len(pipeline.queues), 1) - self.assertQueueCycles(pipeline, 0, [abce, abce, abce, abce]) - - # Now we remove the dependency on E. - - # A->C, B->A, C->B - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, C.url - ) - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - - abc = [A, B, C] - # A,B,C - self.assertQueueCycles(pipeline, 0, [abc, abc, abc]) - # B,C,A - self.assertQueueCycles(pipeline, 1, [abc, abc, abc]) - - # Now remove all dependencies for the three remaining changes. - A.body = A.subject - B.body = B.subject - C.body = C.subject - - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body)) - self.waitUntilSettled() - self.fake_github.emitEvent(C.getPullRequestEditedEvent(C.body)) - self.waitUntilSettled() - - # A, B, C individually (not in that order) - self.assertEqual(len(pipeline.queues), 3) - self.assertQueueCycles(pipeline, 0, [None]) - self.assertQueueCycles(pipeline, 1, [None]) - self.assertQueueCycles(pipeline, 2, [None]) - - # Remove the first change from the queue by forcing a - # dependency on D. - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, D.url - ) - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - self.assertEqual(len(pipeline.queues), 2) - self.assertQueueCycles(pipeline, 0, [None]) - self.assertQueueCycles(pipeline, 1, [None]) - - # Verify that we can put B and C into a bundle. - C.body = "{}\n\nDepends-On: {}\n".format( - C.subject, B.url - ) - self.fake_github.emitEvent(C.getPullRequestEditedEvent(C.body)) - self.waitUntilSettled() - B.body = "{}\n\nDepends-On: {}\n".format( - B.subject, C.url - ) - self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body)) - self.waitUntilSettled() - self.assertEqual(len(pipeline.queues), 2) - bc = [B, C] - self.assertQueueCycles(pipeline, 0, [bc, bc]) - self.assertQueueCycles(pipeline, 1, [bc, bc]) - - # All done. - self.executor_server.hold_jobs_in_build = False - self.executor_server.release() - self.waitUntilSettled() - - self.assertHistory([ - dict(name="common-job", result="ABORTED", - changes=f"{E.number},{E.head_sha} {A.number},{A.head_sha} " - f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="project3-job", result="ABORTED", - changes=f"{E.number},{E.head_sha} {A.number},{A.head_sha} " - f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - - # Relaunched - dict(name="common-job", result="ABORTED", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha} " - f"{C.number},{C.head_sha}"), - dict(name="project3-job", result="ABORTED", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha} " - f"{C.number},{C.head_sha}"), - - dict(name="project1-job", result="ABORTED", - changes=f"{B.number},{B.head_sha} {C.number},{C.head_sha} " - f"{A.number},{A.head_sha}"), - # common-job deduplicated from stanza above - - dict(name="common-job", result="ABORTED", - changes=f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="project3-job", result="ABORTED", - changes=f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - - dict(name="common-job", result="ABORTED", - changes=f"{A.number},{A.head_sha}"), - dict(name="project1-job", result="ABORTED", - changes=f"{A.number},{A.head_sha}"), - dict(name="common-job", result="ABORTED", - changes=f"{B.number},{B.head_sha}"), - dict(name="project2-job", result="ABORTED", - changes=f"{B.number},{B.head_sha}"), - dict(name="common-job", result="ABORTED", - changes=f"{C.number},{C.head_sha}"), - dict(name="project3-job", result="ABORTED", - changes=f"{C.number},{C.head_sha}"), - - dict(name="common-job", result="SUCCESS", - changes=f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - dict(name="project3-job", result="SUCCESS", - changes=f"{B.number},{B.head_sha} {C.number},{C.head_sha}"), - # common-job deduplicated - dict(name="project2-job", result="SUCCESS", - changes=f"{C.number},{C.head_sha} {B.number},{B.head_sha}"), - ], ordered=False) - @simple_layout('layouts/dependency_removal_check.yaml', driver='github') def test_dependency_removal_check(self): - # TODO: After removal of the safety check, remove this test - # and use the _orig version. # Dependency cycles can be updated without uploading new # patchsets. This test exercises such changes. @@ -4854,12 +4448,10 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): self.waitUntilSettled() # A, B, C individually - # ABC E, A, B, C - self.assertEqual(len(pipeline.queues), 4) - self.assertQueueCycles(pipeline, 0, [abc, [E]]) - self.assertQueueCycles(pipeline, 1, [[A]]) - self.assertQueueCycles(pipeline, 2, [[B]]) - self.assertQueueCycles(pipeline, 3, [[C]]) + self.assertEqual(len(pipeline.queues), 3) + self.assertQueueCycles(pipeline, 0, [[A], [B]]) + self.assertQueueCycles(pipeline, 1, [[A], [B], [C]]) + self.assertQueueCycles(pipeline, 2, [[A]]) # Verify that we can put B and C into a bundle. C.body = "{}\n\nDepends-On: {}\n".format( @@ -4872,93 +4464,24 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase): ) self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body)) self.waitUntilSettled() - self.assertEqual(len(pipeline.queues), 3) + self.assertEqual(len(pipeline.queues), 2) bc = [B, C] - self.assertQueueCycles(pipeline, 0, [abc, [E]]) - self.assertQueueCycles(pipeline, 1, [[A]]) - self.assertQueueCycles(pipeline, 2, [bc]) + self.assertQueueCycles(pipeline, 0, [[A]]) + self.assertQueueCycles(pipeline, 1, [bc]) # All done. self.executor_server.hold_jobs_in_build = False self.executor_server.release() self.waitUntilSettled() - for build in self.history[:-7]: + for build in self.history[:-5]: self.assertEqual(build.result, 'ABORTED') # Changes A, B, C in the end - for build in self.history[-7:]: + for build in self.history[-5:]: self.assertEqual(build.result, 'SUCCESS') - @skip("Disabled due to safety check") - @simple_layout('layouts/dependency_removal_check.yaml', driver='github') - def test_dependency_addition_check_orig(self): - # Dependency cycles can be updated without uploading new - # patchsets. This test exercises such changes. - - self.executor_server.hold_jobs_in_build = True - tenant = self.scheds.first.sched.abide.tenants.get("tenant-one") - pipeline = tenant.layout.pipelines["check"] - - A = self.fake_github.openFakePullRequest("gh/project1", "master", "A") - B = self.fake_github.openFakePullRequest("gh/project2", "master", "B") - - self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) - self.waitUntilSettled() - - expected_cycle = {A.number} - self.assertEqual(len(list(pipeline.getAllItems())), 1) - for item in pipeline.getAllItems(): - self.assertIsNone(item.bundle) - - B.body = "{}\n\nDepends-On: {}\n".format( - B.subject, A.url - ) - self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body)) - self.waitUntilSettled() - - A.body = "{}\n\nDepends-On: {}\n".format( - A.subject, B.url - ) - self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body)) - self.waitUntilSettled() - self.assertEqual(len(list(pipeline.getAllItems())), 4) - self.assertEqual(len(pipeline.queues), 2) - ab = [A, B] - self.assertQueueCycles(pipeline, 0, [ab, ab]) - self.assertQueueCycles(pipeline, 1, [ab, ab]) - - expected_cycle = {A.number, B.number} - for item in pipeline.getAllItems(): - cycle = {i.change.number for i in item.bundle.items} - self.assertEqual(expected_cycle, cycle) - - # All done. - self.executor_server.hold_jobs_in_build = False - self.executor_server.release() - self.waitUntilSettled() - - self.assertHistory([ - # Change 1 first enqueue - dict(name="project1-job", result="ABORTED", - changes=f"{A.number},{A.head_sha}"), - dict(name="common-job", result="ABORTED", - changes=f"{A.number},{A.head_sha}"), - - # Change 1 bundle update - dict(name="project1-job", result="SUCCESS", - changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"), - dict(name="common-job", result="SUCCESS", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), - - # Change 2 bundle update - dict(name="project2-job", result="SUCCESS", - changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), - ], ordered=False) - @simple_layout('layouts/dependency_removal_check.yaml', driver='github') def test_dependency_addition_check(self): - # TODO: After removal of the safety check, remove this test - # and use the _orig version. # Dependency cycles can be updated without uploading new # patchsets. This test exercises such changes. diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index 773ccf4439..a6c6c3d82d 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -28,7 +28,7 @@ from zuul.lib.tarjan import strongly_connected_components import zuul.lib.tracing as tracing from zuul.model import ( Change, PipelineState, PipelineChangeList, QueueItem, - filter_severity + filter_severity, EnqueueEvent ) from zuul.zk.change_cache import ChangeKey from zuul.zk.exceptions import LockException @@ -445,23 +445,29 @@ class PipelineManager(metaclass=ABCMeta): return None def findOldVersionOfChangeAlreadyInQueue(self, change): + # Return the item and the old version of the change for item in self.pipeline.getAllItems(): if not item.live: continue for item_change in item.changes: if change.isUpdateOf(item_change): - return item - return None + return item, item_change + return None, None def removeOldVersionsOfChange(self, change, event): if not self.pipeline.dequeue_on_new_patchset: return - old_item = self.findOldVersionOfChangeAlreadyInQueue(change) + old_item, old_change = self.findOldVersionOfChangeAlreadyInQueue( + change) if old_item: log = get_annotated_logger(self.log, event) log.debug("Change %s is a new version, removing %s", change, old_item) self.removeItem(old_item) + if old_item.live: + changes = old_item.changes[:] + changes.remove(old_change) + self.reEnqueueChanges(old_item, changes) def removeAbandonedChange(self, change, event): log = get_annotated_logger(self.log, event) @@ -481,44 +487,16 @@ class PipelineManager(metaclass=ABCMeta): pass self.removeItem(item) - def reEnqueueIfDepsPresent(self, item, needs_changes, log, - skip_presence_check=True): - # This item is about to be dequeued because it's missing - # changes. Try to re-enqueue it before dequeing. - # Return whether the dequeue should be quiet. - if not item.live: - return False - if not all(self.isChangeAlreadyInPipeline(c) for c in needs_changes): - return False - # We enqueue only the first change in the item, presuming - # that the remaining changes will be pulled in as - # appropriate. Because we skip the presence check, we - # can't enqueue all of the items changes directly since we - # would end up with new items for every change. We only - # want one new item. - change = item.changes[0] - # Check if there is another live item with the change already. - for other_item in self.pipeline.getAllItems(): - if other_item is item: - continue - if not other_item.live: - continue - for item_change in other_item.changes: - if item_change.equals(change): - return True - # Try enqueue, if that succeeds, keep this dequeue quiet - try: - log.info("Attempting re-enqueue of %s", item) - return self.addChange( - change, item.event, - enqueue_time=item.enqueue_time, - quiet=True, - skip_presence_check=skip_presence_check) - except Exception: - log.exception("Unable to re-enqueue %s " - "which is missing dependencies", - item) - return False + def reEnqueueChanges(self, item, changes): + for change in changes: + event = EnqueueEvent(self.pipeline.tenant.name, + self.pipeline.name, + change.project.canonical_hostname, + change.project.name, + change=change._id()) + event.zuul_event_id = item.event.zuul_event_id + self.sched.pipeline_management_events[ + self.pipeline.tenant.name][self.pipeline.name].put(event) @abstractmethod def getChangeQueue(self, change, event, existing=None): @@ -1644,6 +1622,20 @@ class PipelineManager(metaclass=ABCMeta): dependency_graph = collections.OrderedDict() self.getDependencyGraph(item.changes[0], dependency_graph, item.event, quiet=True) + + # Verify that the cycle dependency graph is correct + cycle = self.cycleForChange( + item.changes[0], dependency_graph, item.event, debug=False) + cycle = cycle or [item.changes[0]] + item_cycle = set(item.changes) + if set(cycle) != item_cycle: + log.info("Item cycle has changed: %s, now: %s, was: %s", item, + set(cycle), item_cycle) + self.removeItem(item) + if item.live: + self.reEnqueueChanges(item, item.changes) + return (True, nnfi) + abort, needs_changes = self.getMissingNeededChanges( item.changes, change_queue, item.event, dependency_graph=dependency_graph) @@ -1652,7 +1644,6 @@ class PipelineManager(metaclass=ABCMeta): log.info("Dequeuing %s because " "it can no longer merge" % item) self.cancelJobs(item) - quiet_dequeue = False if not meets_reqs: item.setDequeuedMissingRequirements() else: @@ -1662,14 +1653,7 @@ class PipelineManager(metaclass=ABCMeta): else: msg = f'Change {clist} is needed.' item.setDequeuedNeedingChange(msg) - # If all the dependencies are already in the pipeline - # (but not ahead of this change), then we probably - # just added updated versions of them, possibly - # updating a cycle. In that case, attempt to - # re-enqueue this change with the updated deps. - quiet_dequeue = self.reEnqueueIfDepsPresent( - item, needs_changes, log) - if item.live and not quiet_dequeue: + if item.live: try: self.reportItem(item) except exceptions.MergeFailure: @@ -1677,20 +1661,6 @@ class PipelineManager(metaclass=ABCMeta): self.dequeueItem(item) return (True, nnfi) - # Safety check: verify that the cycle dependency graph is correct - cycle = self.cycleForChange( - item.changes[0], dependency_graph, item.event, debug=False) - cycle = cycle or [item.changes[0]] - item_cycle = set(item.changes) - if (item.live and set(cycle) != item_cycle): - log.info("Item cycle has changed: %s, now: %s, was: %s", item, - set(cycle), item_cycle) - self.removeItem(item) - if item.live: - self.reEnqueueIfDepsPresent(item, needs_changes, log, - skip_presence_check=False) - return (True, nnfi) - actionable = change_queue.isActionable(item) item.updateAttributes(self.current_context, active=actionable)