Consider queue settings for topic dependencies
Most of a change's attributes are tenant-independent. This however is different for topic dependencies, which should only be considered in tenants where the dependencies-by-topic feature is enabled. This is mainly a problem when a project is part of multiple tenants as the dependencies-by-topic setting might be different for each tenant. To fix this we will only return the topic dependencies for a change in tenants where the feature has been activated. Since the `needs_changes` property is now a method called `getNeedsChanges()`, we also changed `needed_by_changes` to `getNeededByChanges()` so they match. Change-Id: I343306db0abbe2fbf98ddb3f81b6d509eaf4a2bf
This commit is contained in:
parent
5443a0f51b
commit
cb3cc6bd4b
@ -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
1
tests/fixtures/config/circular-dependencies/git/common-tenant-two/playbooks/run.yaml
vendored
Normal file
@ -0,0 +1 @@
|
||||
---
|
96
tests/fixtures/config/circular-dependencies/git/common-tenant-two/zuul.yaml
vendored
Normal file
96
tests/fixtures/config/circular-dependencies/git/common-tenant-two/zuul.yaml
vendored
Normal 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
1
tests/fixtures/config/circular-dependencies/git/org_project5/README
vendored
Normal file
@ -0,0 +1 @@
|
||||
test
|
1
tests/fixtures/config/circular-dependencies/git/org_project6/README
vendored
Normal file
1
tests/fixtures/config/circular-dependencies/git/org_project6/README
vendored
Normal file
@ -0,0 +1 @@
|
||||
test
|
@ -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
|
||||
|
@ -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"
|
||||
|
@ -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)
|
||||
|
@ -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 "
|
||||
|
@ -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
|
||||
|
@ -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:
|
||||
|
@ -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)
|
||||
|
@ -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))
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user