zuul/zuul
Code Issues Proposed changes

Attempt to preserve triggering event across re-enqueues

Browse Source 
When a dependency cycle is updated, we will re-enqueue the changes
in the cycle so that each of the changes goes thorugh the process
of being added to the queue with the updated contents of the cycle.
That may mean omitting changes from the cycle, or adding new ones,
or even splitting into two cycles.

In that case, in order to preserve the idea of the
"triggering change", carry over the triggering change from when the
cycle was originally enqueued.

Note that this now exposes us to the novel idea that the triggering
change may not be part of the queue item.

Change-Id: I9e00009040f91d7edc31f4928e632edde4b2745f
This commit is contained in:
James E. Blair
2024-03-12 10:55:57 -07:00
parent 802c5a8ca6
commit 6ccbdacdf2
3 changed files with 47 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
tests/unit/test_circular_dependencies.py
View File

@@ -16,6 +16,7 @@
import re
import textwrap
import json
from zuul.model import PromoteEvent
@@ -4263,6 +4264,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
for item in pipeline.getAllItems():
cycle = {c.number for c in item.changes}
self.assertEqual(expected_cycle, cycle)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
# Now we remove the dependency on E. This re-enqueues ABC and E.
@@ -4291,6 +4294,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
self.assertEqual(len(list(pipeline.getAllItems())), 3)
for item in pipeline.getAllItems():
self.assertEqual(len(item.changes), 1)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
# Remove the first change from the queue by forcing a
# dependency on D.
@@ -4302,6 +4307,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
self.assertEqual(len(list(pipeline.getAllItems())), 2)
for item in pipeline.getAllItems():
self.assertEqual(len(item.changes), 1)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
# B and C are still in the queue. Put them
# back into a bundle.
@@ -4320,6 +4327,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
for item in pipeline.getAllItems():
cycle = {c.number for c in item.changes}
self.assertEqual(expected_cycle, cycle)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
# All done.
self.executor_server.hold_jobs_in_build = False
@@ -4352,6 +4361,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
self.assertEqual(len(list(pipeline.getAllItems())), 1)
for item in pipeline.getAllItems():
self.assertEqual(len(item.changes), 1)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '1')
B.body = "{}\n\nDepends-On: {}\n".format(
B.subject, A.url
@@ -4369,6 +4380,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
for item in pipeline.getAllItems():
cycle = {c.number for c in item.changes}
self.assertEqual(expected_cycle, cycle)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '1')
# All done.
self.executor_server.hold_jobs_in_build = False
@@ -4431,6 +4444,9 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
abce = [A, B, C, E]
self.assertEqual(len(pipeline.queues), 1)
self.assertQueueCycles(pipeline, 0, [abce])
for item in pipeline.getAllItems():
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
# Now we remove the dependency on E.
@@ -4444,6 +4460,9 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
abc = [A, B, C]
# ABC<nonlive>, E<live>
self.assertQueueCycles(pipeline, 0, [abc, [E]])
for item in pipeline.getAllItems():
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
# Now remove all dependencies for the three remaining changes.
A.body = A.subject
@@ -4462,6 +4481,9 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
self.assertQueueCycles(pipeline, 0, [[A], [B]])
self.assertQueueCycles(pipeline, 1, [[A], [B], [C]])
self.assertQueueCycles(pipeline, 2, [[A]])
for item in pipeline.getAllItems():
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
# Verify that we can put B and C into a bundle.
C.body = "{}\n\nDepends-On: {}\n".format(
@@ -4469,11 +4491,17 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
)
self.fake_github.emitEvent(C.getPullRequestEditedEvent(C.body))
self.waitUntilSettled()
for item in pipeline.getAllItems():
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
B.body = "{}\n\nDepends-On: {}\n".format(
B.subject, C.url
)
self.fake_github.emitEvent(B.getPullRequestEditedEvent(B.body))
self.waitUntilSettled()
for item in pipeline.getAllItems():
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '3')
self.assertEqual(len(pipeline.queues), 2)
bc = [B, C]
self.assertQueueCycles(pipeline, 0, [[A]])
@@ -4508,6 +4536,8 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
self.assertEqual(len(list(pipeline.getAllItems())), 1)
for item in pipeline.getAllItems():
self.assertEqual(len(item.changes), 1)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '1')
B.body = "{}\n\nDepends-On: {}\n".format(
B.subject, A.url
@@ -4521,6 +4551,10 @@ class TestGithubAppCircularDependencies(ZuulGithubAppTestCase):
self.fake_github.emitEvent(A.getPullRequestEditedEvent(A.body))
self.waitUntilSettled()
self.assertEqual(len(list(pipeline.getAllItems())), 1)
for item in pipeline.getAllItems():
self.assertEqual(len(item.changes), 2)
# Assert we get the same triggering change every time
self.assertEqual(json.loads(item.event.ref)['stable_id'], '1')
self.assertEqual(len(pipeline.queues), 1)
ab = [A, B]
self.assertQueueCycles(pipeline, 0, [ab])

6
zuul/manager/__init__.py
View File

@@ -489,11 +489,15 @@ class PipelineManager(metaclass=ABCMeta):
def reEnqueueChanges(self, item, changes):
for change in changes:
orig_ref = None
if item.event:
orig_ref = item.event.ref
event = EnqueueEvent(self.pipeline.tenant.name,
self.pipeline.name,
change.project.canonical_hostname,
change.project.name,
change=change._id())
change=change._id(),
orig_ref=orig_ref)
event.zuul_event_id = item.event.zuul_event_id
self.sched.pipeline_management_events[
self.pipeline.tenant.name][self.pipeline.name].put(event)

9
zuul/model.py
View File

@@ -1235,6 +1235,8 @@ class ChangeQueue(zkobject.ZKObject):
event_ref_cache_key = None
if isinstance(event, EventInfo):
event_ref_cache_key = event.ref
elif getattr(event, 'orig_ref', None):
event_ref_cache_key = event.orig_ref
elif hasattr(event, 'canonical_project_name'):
trusted, project = self.pipeline.tenant.getProject(
event.canonical_project_name)
@@ -6607,11 +6609,13 @@ class ChangeManagementEvent(ManagementEvent):
:arg str ref: optional, the ref
:arg str oldrev: optional, the old revision
:arg str newrev: optional, the new revision
:arg dict orig_ref: optional, the cache key reference of the
ref of the original triggering event
"""
def __init__(self, tenant_name, pipeline_name, project_hostname,
project_name, change=None, ref=None, oldrev=None,
newrev=None):
newrev=None, orig_ref=None):
super().__init__()
self.type = None
self.tenant_name = tenant_name
@@ -6626,6 +6630,7 @@ class ChangeManagementEvent(ManagementEvent):
self.ref = ref
self.oldrev = oldrev
self.newrev = newrev
self.orig_ref = orig_ref
self.timestamp = time.time()
span = trace.get_current_span()
self.span_context = tracing.getSpanContext(span)
@@ -6647,6 +6652,7 @@ class ChangeManagementEvent(ManagementEvent):
d["newrev"] = self.newrev
d["timestamp"] = self.timestamp
d["span_context"] = self.span_context
d["orig_ref"] = self.orig_ref
return d
def updateFromDict(self, d):
@@ -6666,6 +6672,7 @@ class ChangeManagementEvent(ManagementEvent):
data.get("ref"),
data.get("oldrev"),
data.get("newrev"),
data.get("orig_ref"),
)
event.updateFromDict(data)
return event