zuul
/
zuul
Code Issues Proposed changes

Merge "Consider queue settings for topic dependencies"

This commit is contained in:
Zuul 2022-12-13 15:07:49 +00:00 committed by Gerrit Code Review
parent 532c30469f cb3cc6bd4b
commit d14f1e9202
13 changed files with 218 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
tests/fixtures/config/circular-dependencies/git/common-config/zuul.yaml vendored
View File

@ -89,6 +89,12 @@
- job:
name: project3-job
- job:
name: project5-job-t1
- job:
name: project6-job-t1
- project:
name: common-config
queue: integrated
@ -144,3 +150,16 @@
gate:
jobs:
- project3-job
- project:
name: org/project5
queue: integrated
check:
jobs:
- project5-job-t1
- project:
name: org/project6
check:
jobs:
- project6-job-t1

1
tests/fixtures/config/circular-dependencies/git/common-tenant-two/playbooks/run.yaml vendored Normal file
View File

@ -0,0 +1 @@
---

96
tests/fixtures/config/circular-dependencies/git/common-tenant-two/zuul.yaml vendored Normal file
View File

@ -0,0 +1,96 @@
- queue:
name: integrated
allow-circular-dependencies: true
dependencies-by-topic: true
- pipeline:
name: check
manager: independent
trigger:
gerrit:
- event: patchset-created
github:
- event: pull_request
action:
- opened
- changed
- reopened
- edited
success:
gerrit:
Verified: 1
github:
status: success
failure:
gerrit:
Verified: -1
github:
status: failure
- pipeline:
name: gate
manager: dependent
success-message: Build succeeded (gate).
require:
gerrit:
approval:
- Approved: 1
github:
label: approved
trigger:
gerrit:
- event: comment-added
approval:
- Approved: 1
github:
- event: pull_request
action: edited
- event: pull_request
action: labeled
label: approved
success:
gerrit:
Verified: 2
submit: true
github:
merge: true
failure:
gerrit:
Verified: -2
github: {}
start:
gerrit:
Verified: 0
github: {}
precedence: high
- job:
name: base
parent: null
run: playbooks/run.yaml
- job:
name: project5-job-t2
- job:
name: project6-job-t2
- project:
name: org/project5
queue: integrated
check:
jobs:
- project5-job-t2
gate:
jobs:
- project5-job-t2
- project:
name: org/project6
queue: integrated
check:
jobs:
- project6-job-t2
gate:
jobs:
- project6-job-t2

1
tests/fixtures/config/circular-dependencies/git/org_project5/README vendored Normal file
View File

@ -0,0 +1 @@
test

1
tests/fixtures/config/circular-dependencies/git/org_project6/README vendored Normal file
View File

@ -0,0 +1 @@
test

6
tests/fixtures/config/circular-dependencies/main.yaml vendored
View File

@ -10,6 +10,8 @@
- org/project1
- org/project2
- org/project3
- org/project5
- org/project6
github:
untrusted-projects:
- gh/project
@ -21,5 +23,9 @@
name: tenant-two
source:
gerrit:
config-projects:
- common-tenant-two
untrusted-projects:
- org/project4
- org/project5
- org/project6

49
tests/unit/test_circular_dependencies.py
View File

@ -2283,6 +2283,55 @@ class TestGerritCircularDependencies(ZuulTestCase):
dict(name="test-job", result="SUCCESS", changes="2,1 1,2"),
], ordered=False)
def test_deps_by_topic_multi_tenant(self):
A = self.fake_gerrit.addFakeChange('org/project5', "master", "A",
topic='test-topic')
B = self.fake_gerrit.addFakeChange('org/project6', "master", "B",
topic='test-topic')
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(len(A.patchsets[-1]["approvals"]), 1)
self.assertEqual(A.patchsets[-1]["approvals"][0]["type"], "Verified")
self.assertEqual(A.patchsets[-1]["approvals"][0]["value"], "1")
self.assertEqual(len(B.patchsets[-1]["approvals"]), 1)
self.assertEqual(B.patchsets[-1]["approvals"][0]["type"], "Verified")
self.assertEqual(B.patchsets[-1]["approvals"][0]["value"], "1")
# We're about to add approvals to changes without adding the
# triggering events to Zuul, so that we can be sure that it is
# enqueuing the changes based on dependencies, not because of
# triggering events. Since it will have the changes cached
# already (without approvals), we need to clear the cache
# first.
for connection in self.scheds.first.connections.connections.values():
connection.maintainCache([], max_age=0)
A.addApproval("Code-Review", 2)
B.addApproval("Code-Review", 2)
A.addApproval("Approved", 1)
self.fake_gerrit.addEvent(B.addApproval("Approved", 1))
self.waitUntilSettled()
self.assertEqual(A.reported, 4)
self.assertEqual(B.reported, 4)
self.assertEqual(A.data["status"], "MERGED")
self.assertEqual(B.data["status"], "MERGED")
self.assertHistory([
# Check
dict(name="project5-job-t1", result="SUCCESS", changes="1,1"),
dict(name="project6-job-t1", result="SUCCESS", changes="2,1"),
dict(name="project5-job-t2", result="SUCCESS", changes="2,1 1,1"),
dict(name="project6-job-t2", result="SUCCESS", changes="1,1 2,1"),
# Gate
dict(name="project5-job-t2", result="SUCCESS", changes="1,1 2,1"),
dict(name="project6-job-t2", result="SUCCESS", changes="1,1 2,1"),
], ordered=False)
class TestGithubCircularDependencies(ZuulTestCase):
config_file = "zuul-gerrit-github.conf"

6
zuul/driver/zuul/__init__.py
View File

@ -23,6 +23,7 @@ from zuul.driver.zuul.zuulmodel import ZuulTriggerEvent
from zuul.driver.zuul import zuulmodel
from zuul.driver.zuul import zuultrigger
from zuul.lib.logutil import get_annotated_logger
from zuul.model import Change
PARENT_CHANGE_ENQUEUED = 'parent-change-enqueued'
PROJECT_CHANGE_MERGED = 'project-change-merged'
@ -121,7 +122,7 @@ class ZuulDriver(Driver, TriggerInterface):
log = get_annotated_logger(self.log, event)
log.debug("Checking for changes needing %s:" % change)
if not hasattr(change, 'needed_by_changes'):
if not isinstance(change, Change):
log.debug(" %s does not support dependencies" % type(change))
return
@ -129,7 +130,8 @@ class ZuulDriver(Driver, TriggerInterface):
# numbers of github installations. This can be improved later
# with persistent storage of dependency information.
needed_by_changes = set(
pipeline.manager.resolveChangeReferences(change.needed_by_changes))
pipeline.manager.resolveChangeReferences(
change.getNeededByChanges()))
for source in self.sched.connections.getSources():
log.debug(" Checking source: %s",
source.connection.connection_name)

20
zuul/manager/__init__.py
View File

@ -228,9 +228,16 @@ class PipelineManager(metaclass=ABCMeta):
change = source.getChange(key)
if change is None:
self.log.error("Unable to resolve change from key %s", key)
if (isinstance(change, model.Change)
and change.commit_needs_changes is None):
self.updateCommitDependencies(change, None, event=None)
if isinstance(change, model.Change):
update_commit_dependencies = (
change.commit_needs_changes is None)
update_topic_dependencies = (
change.topic_needs_changes is None
and self.useDependenciesByTopic(change.project))
if (update_commit_dependencies
or update_topic_dependencies):
self.updateCommitDependencies(change, None, event=None)
self._change_cache[change.cache_key] = change
resolved_changes.append(change)
return resolved_changes
@ -246,8 +253,9 @@ class PipelineManager(metaclass=ABCMeta):
active_layout_uuids.add(item.layout_uuid)
if isinstance(item.change, model.Change):
referenced_change_keys.update(item.change.needs_changes)
referenced_change_keys.update(item.change.needed_by_changes)
referenced_change_keys.update(item.change.getNeedsChanges(
self.useDependenciesByTopic(item.change.project)))
referenced_change_keys.update(item.change.getNeededByChanges())
# Clean up unused layouts in the cache
unused_layouts = set(self._layout_cache.keys()) - active_layout_uuids
@ -574,7 +582,7 @@ class PipelineManager(metaclass=ABCMeta):
return True
cycle = []
if hasattr(change, "needs_changes"):
if isinstance(change, model.Change):
cycle = self.cycleForChange(change, dependency_graph, event)
if cycle and not self.canProcessCycle(change.project):
log.info("Dequeing change %s since at least one project "

20
zuul/manager/dependent.py
View File

@ -80,7 +80,7 @@ class DependentPipelineManager(SharedQueuePipelineManager):
history = history if history is not None else []
log.debug("Checking for changes needing %s:" % change)
if not hasattr(change, 'needed_by_changes'):
if not isinstance(change, model.Change):
log.debug(" %s does not support dependencies" % type(change))
return
@ -90,7 +90,7 @@ class DependentPipelineManager(SharedQueuePipelineManager):
sources = {p.source for p in projects}
needed_by_changes = self.resolveChangeReferences(
change.needed_by_changes)
change.getNeededByChanges())
log.debug(" Previously known following changes: %s",
needed_by_changes)
seen = set(needed_by_changes)
@ -184,10 +184,11 @@ class DependentPipelineManager(SharedQueuePipelineManager):
# Return true if okay to proceed enqueing this change,
# false if the change should not be enqueued.
log.debug("Checking for changes needed by %s:" % change)
if not hasattr(change, 'needs_changes'):
if not isinstance(change, model.Change):
log.debug(" %s does not support dependencies", type(change))
return False, []
if not change.needs_changes:
if not change.getNeedsChanges(
self.useDependenciesByTopic(change.project)):
log.debug(" No changes needed")
return False, []
changes_needed = []
@ -195,7 +196,8 @@ class DependentPipelineManager(SharedQueuePipelineManager):
# Ignore supplied change_queue
with self.getChangeQueue(change, event) as change_queue:
for needed_change in self.resolveChangeReferences(
change.needs_changes):
change.getNeedsChanges(
self.useDependenciesByTopic(change.project))):
log.debug(" Change %s needs change %s:" % (
change, needed_change))
if needed_change.is_merged:
@ -245,13 +247,15 @@ class DependentPipelineManager(SharedQueuePipelineManager):
return abort, changes_needed
def getFailingDependentItems(self, item, nnfi):
if not hasattr(item.change, 'needs_changes'):
if not isinstance(item.change, model.Change):
return None
if not item.change.needs_changes:
if not item.change.getNeedsChanges(
self.useDependenciesByTopic(item.change.project)):
return None
failing_items = set()
for needed_change in self.resolveChangeReferences(
item.change.needs_changes):
item.change.getNeedsChanges(
self.useDependenciesByTopic(item.change.project))):
needed_item = self.getItemForChange(needed_change)
if not needed_item:
continue

8
zuul/manager/independent.py
View File

@ -82,16 +82,18 @@ class IndependentPipelineManager(PipelineManager):
log.debug("Checking for changes needed by %s:" % change)
# Return true if okay to proceed enqueing this change,
# false if the change should not be enqueued.
if not hasattr(change, 'needs_changes'):
if not isinstance(change, model.Change):
log.debug(" %s does not support dependencies" % type(change))
return False, []
if not change.needs_changes:
if not change.getNeedsChanges(
self.useDependenciesByTopic(change.project)):
log.debug(" No changes needed")
return False, []
changes_needed = []
abort = False
for needed_change in self.resolveChangeReferences(
change.needs_changes):
change.getNeedsChanges(
self.useDependenciesByTopic(change.project))):
log.debug(" Change %s needs change %s:" % (
change, needed_change))
if needed_change.is_merged:

14
zuul/model.py
View File

@ -6162,18 +6162,17 @@ class Change(Branch):
return True
return False
@property
def needs_changes(self):
def getNeedsChanges(self, include_topic_needs=False):
commit_needs_changes = self.commit_needs_changes or []
topic_needs_changes = self.topic_needs_changes or []
topic_needs_changes = (
include_topic_needs and self.topic_needs_changes) or []
r = OrderedDict()
for x in (self.git_needs_changes + self.compat_needs_changes +
commit_needs_changes + topic_needs_changes):
r[x] = None
return tuple(r.keys())
@property
def needed_by_changes(self):
def getNeededByChanges(self):
r = OrderedDict()
for x in (self.git_needed_by_changes + self.compat_needed_by_changes):
r[x] = None
@ -6197,8 +6196,9 @@ class Change(Branch):
so far. Will be updated with additional changes by this method.
"""
related.add(self.cache_stat.key)
for reference in itertools.chain(self.needs_changes,
self.needed_by_changes):
for reference in itertools.chain(
self.getNeedsChanges(include_topic_needs=True),
self.getNeededByChanges()):
key = ChangeKey.fromReference(reference)
if key not in related:
source = sched.connections.getSource(key.connection_name)

4
zuul/reporter/__init__.py
View File

@ -261,7 +261,9 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
def _formatItemReportOtherBundleItems(self, item):
related_changes = item.pipeline.manager.resolveChangeReferences(
item.change.needs_changes)
item.change.getNeedsChanges(
item.pipeline.manager.useDependenciesByTopic(
item.change.project)))
return "Related changes:\n{}\n".format("\n".join(
f' - {c.url}' for c in related_changes if c is not item.change))