zuul
/
zuul
Code Issues Proposed changes

Merge "Build layout of non-live items with config updates"

This commit is contained in:
Zuul 2019-07-15 15:33:00 +00:00 committed by Gerrit Code Review
parent f1bcb7ccb5 de1a8372a8
commit d2cfb81246
4 changed files with 151 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
tests/unit/test_gerrit.py
View File

@ -182,7 +182,8 @@ class TestGerritWeb(ZuulTestCase):
self.waitUntilSettled()
self.assertEqual(B.patchsets[0]['approvals'][0]['value'], "-1")
self.assertIn('Zuul encountered a syntax error',
self.assertIn('This change depends on a change '
'with an invalid configuration',
B.messages[0])
self.assertEqual(B.comments, [])

41
tests/unit/test_scheduler.py
View File

@ -5781,6 +5781,47 @@ class TestJobUpdateFileMatcher(ZuulTestCase):
dict(name='new-files', result='SUCCESS', changes='1,1'),
])
def test_patch_series(self):
"Test that we diff to the nearest layout in a patch series"
in_repo_conf = textwrap.dedent(
"""
- job:
name: new-files1
parent: existing-files
- project:
check:
jobs:
- new-files1
""")
file_dict = {'zuul.d/new1.yaml': in_repo_conf}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict)
in_repo_conf = textwrap.dedent(
"""
- job:
name: new-files2
parent: existing-files
- project:
check:
jobs:
- new-files2
""")
file_dict = {'zuul.d/new2.yaml': in_repo_conf}
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
files=file_dict)
B.setDependsOn(A, 1)
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertHistory([
dict(name='new-files2', result='SUCCESS', changes='1,1 2,1'),
])
def test_disable_match(self):
"Test matchers are not overridden if we say so"
in_repo_conf = textwrap.dedent(

85
tests/unit/test_v3.py
View File

@ -2178,8 +2178,8 @@ class TestInRepoConfig(ZuulTestCase):
self.assertEqual(C.reported, 1,
"C should report failure")
self.assertIn('Zuul encountered a syntax error while parsing '
'its configuration',
self.assertIn('This change depends on a change '
'with an invalid configuration.',
C.messages[0],
"C should have an error reported")
@ -2215,20 +2215,14 @@ class TestNonLiveMerges(ZuulTestCase):
config_file = 'zuul-connections-gerrit-and-github.conf'
tenant_config_file = 'config/in-repo/main.yaml'
def test_non_live_merges(self):
def test_non_live_merges_with_config_updates(self):
"""
This test checks that we don't do merges for non-live queue items.
The scenario tests three scenarios:
This test checks that we do merges for non-live queue items with
config updates.
* Simple dependency chain:
A -> B -> C
* Dependency chain that includes a merge conflict:
A -> B -> B_conflict (conflicts with A) -> C_conflict
* Dependency chain with broken config in the middls:
A_invalid (broken config) -> B_after_invalid (repairs broken config(
"""
in_repo_conf_a = textwrap.dedent(
@ -2302,55 +2296,18 @@ class TestNonLiveMerges(ZuulTestCase):
parent=B.patchsets[0]['ref'])
C.setDependsOn(B, 1)
B_conflict = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'B_conflict', files=file_dict_a)
B_conflict.setDependsOn(B, 1)
C_conflict = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'C_conflict')
C_conflict.setDependsOn(B_conflict, 1)
in_repo_conf_a_invalid = textwrap.dedent(
"""
- project:
name: org/project
check:
jobs:
- project-test1
""")
file_dict_a_invalid = {'.zuul.yaml': in_repo_conf_a_invalid,
'playbooks/project-test.yaml': in_repo_playbook}
A_invalid = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'A_invalid', files=file_dict_a_invalid)
B_after_invalid = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'B', files=file_dict_b,
parent=A_invalid.patchsets[0]['ref'])
B_after_invalid.setDependsOn(A_invalid, 1)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B_conflict.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(C_conflict.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(A_invalid.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B_after_invalid.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(A.reported, 1, "A should report")
self.assertEqual(B.reported, 1, "B should report")
self.assertEqual(C.reported, 1, "C should report")
self.assertEqual(B_conflict.reported, 1, "B_conflict should report")
self.assertEqual(C_conflict.reported, 1, "C_conflict should report")
self.assertEqual(B_after_invalid.reported, 1,
"B_after_invalid should report")
self.assertIn('Build succeeded', A.messages[0])
self.assertIn('Build succeeded', B.messages[0])
self.assertIn('Build succeeded', C.messages[0])
self.assertIn('Merge Failed', B_conflict.messages[0])
self.assertIn('Merge Failed', C_conflict.messages[0])
self.assertIn('Zuul encountered a syntax error', A_invalid.messages[0])
self.assertIn('Build succeeded', B_after_invalid.messages[0])
self.assertHistory([
# Change A
@ -2367,14 +2324,34 @@ class TestNonLiveMerges(ZuulTestCase):
changes='1,1 2,1 3,1'),
dict(name='project-test3', result='SUCCESS',
changes='1,1 2,1 3,1'),
# Change B_after_invalid
dict(name='project-test1', result='SUCCESS', changes='6,1 7,1'),
dict(name='project-test2', result='SUCCESS', changes='6,1 7,1'),
], ordered=False)
# We expect one merge call per change
self.assertEqual(len(self.merge_client.history['merger:merge']), 7)
# We expect one merge call per live change, plus one call for
# each non-live change with a config update (which is all of them).
self.assertEqual(len(self.merge_client.history['merger:merge']), 6)
def test_non_live_merges(self):
"""
This test checks that we don't do merges for non-live queue items.
* Simple dependency chain:
A -> B -> C
"""
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
B.setDependsOn(A, 1)
C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
C.setDependsOn(B, 1)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
# We expect one merge call per live change.
self.assertEqual(len(self.merge_client.history['merger:merge']), 3)
class TestJobContamination(AnsibleZuulTestCase):

131
zuul/manager/__init__.py
View File

@ -250,8 +250,7 @@ class PipelineManager(object):
item.job_graph = None
item.layout = None
if item.active:
if self.prepareItem(item):
self.prepareJobs(item)
self.prepareItem(item)
# Re-set build results in case any new jobs have been
# added to the tree.
@ -458,12 +457,9 @@ class PipelineManager(object):
return canceled
def _findRelevantErrors(self, item, layout):
parent_layout = None
parent = item.item_ahead
while not parent_layout and parent:
parent_layout = parent.layout
parent = parent.item_ahead
if not parent_layout:
if item.item_ahead:
parent_layout = item.item_ahead.layout
else:
parent_layout = item.pipeline.tenant.layout
relevant_errors = []
@ -604,24 +600,22 @@ class PipelineManager(object):
return False
def getLayout(self, item):
if not self._queueUpdatesConfig(item):
# No config updates in queue. Use existing pipeline layout
return item.queue.pipeline.tenant.layout
elif (not item.change.updatesConfig(item.pipeline.tenant) and
item.item_ahead and item.item_ahead.live):
# Current change does not update layout, use its parent if parent
# has a layout.
return item.item_ahead.layout
# Else this item or a non live parent updates the config,
if item.item_ahead:
fallback_layout = item.item_ahead.layout
else:
fallback_layout = item.pipeline.tenant.layout
if not item.change.updatesConfig(item.pipeline.tenant):
# Current change does not update layout, use its parent.
return fallback_layout
# Else this item updates the config,
# ask the merger for the result.
build_set = item.current_build_set
if build_set.merge_state == build_set.PENDING:
if build_set.merge_state != build_set.COMPLETE:
return None
if build_set.merge_state == build_set.COMPLETE:
if build_set.unable_to_merge:
return None
self.log.debug("Preparing dynamic layout for: %s" % item.change)
return self._loadDynamicLayout(item)
if build_set.unable_to_merge:
return fallback_layout
self.log.debug("Preparing dynamic layout for: %s" % item.change)
return self._loadDynamicLayout(item)
def scheduleMerge(self, item, files=None, dirs=None):
log = item.annotateLogger(self.log)
@ -681,47 +675,76 @@ class PipelineManager(object):
return False
def prepareItem(self, item):
# This runs on every iteration of _processOneItem
# Returns True if the item is ready, false otherwise
ready = True
build_set = item.current_build_set
tenant = item.pipeline.tenant
# We always need to set the configuration of the item if it
# isn't already set.
tpc = tenant.project_configs.get(item.change.project.canonical_name)
if not build_set.ref:
build_set.setConfiguration()
# Next, if a change ahead has a broken config, then so does
# this one. Record that and don't do anything else.
if (item.item_ahead and item.item_ahead.current_build_set and
item.item_ahead.current_build_set.config_errors):
msg = "This change depends on a change "\
"with an invalid configuration.\n"
item.setConfigError(msg)
return False
# The next section starts between 0 and 2 remote merger
# operations in parallel as needed.
ready = True
# If the project is in this tenant, fetch missing files so we
# know if it updates the config.
if tpc:
if build_set.files_state == build_set.NEW:
ready = self.scheduleFilesChanges(item)
if build_set.files_state == build_set.PENDING:
ready = False
# If this change alters config or is live, schedule merge and
# build a layout.
if build_set.merge_state == build_set.NEW:
tenant = item.pipeline.tenant
tpc = tenant.project_configs[item.change.project.canonical_name]
ready = self.scheduleMerge(
item,
files=(['zuul.yaml', '.zuul.yaml'] +
list(tpc.extra_config_files)),
dirs=(['zuul.d', '.zuul.d'] +
list(tpc.extra_config_dirs)))
if build_set.files_state == build_set.NEW:
ready = self.scheduleFilesChanges(item)
if build_set.files_state == build_set.PENDING:
ready = False
if item.live or item.change.updatesConfig(tenant):
ready = self.scheduleMerge(
item,
files=(['zuul.yaml', '.zuul.yaml'] +
list(tpc.extra_config_files)),
dirs=(['zuul.d', '.zuul.d'] +
list(tpc.extra_config_dirs)))
if build_set.merge_state == build_set.PENDING:
ready = False
if build_set.unable_to_merge:
ready = False
if build_set.config_errors:
ready = False
return ready
def prepareJobs(self, item):
log = item.annotateLogger(self.log)
# This only runs once the item is in the pipeline's action window
# Returns True if the item is ready, false otherwise
if not item.live:
# Short circuit as non live items don't need layouts.
# We also don't need to take further ready actions in
# _processOneItem() so we return false.
# If a merger op is outstanding, we're not ready.
if not ready:
return False
elif not item.layout:
# With the merges done, we have the info needed to get a
# layout. This may return the pipeline layout, a layout from
# a change ahead, a newly generated layout for this change, or
# None if there was an error that makes the layout unusable.
# In the last case, it will have set the config_errors on this
# item, which may be picked up by the next itme.
if not item.layout:
item.layout = self.getLayout(item)
if not item.layout:
return False
# If the change can not be merged or has config errors, don't
# run jobs.
if build_set.unable_to_merge:
return False
if build_set.config_errors:
return False
# We don't need to build a job graph for a non-live item, we
# just need the layout.
if not item.live:
return False
# At this point we have a layout for the item, and it's live,
# so freeze the job graph.
log = item.annotateLogger(self.log)
if not item.job_graph:
try:
log.debug("Freezing job graph for %s" % (item,))
@ -784,8 +807,8 @@ class PipelineManager(object):
change_queue.moveItem(item, nnfi)
changed = True
self.cancelJobs(item)
if actionable and item.live:
ready = self.prepareItem(item) and self.prepareJobs(item)
if actionable:
ready = self.prepareItem(item)
# Starting jobs reporting should only be done once if there are
# jobs to run for this item.
if ready and len(self.pipeline.start_actions) > 0 \