Make file matchers overridable
Files and irrelevant-files don't behave in the way people intuitively expect. If a job or template has an irrelevant-files matcher, it's not possible to alter its set of irrelevant-files in a project-pipeline variant. This changes them to be treated as overwriteable attributes and evaluated after branch-matching variants are combined. * Files and irrelevant-files are overwritten, so the last value encountered when combining all the matching variants (looking only at branches) wins. * It's possible to both reduce and expand the scope of jobs, but the user may need to manually copy values from a parent or other variant in order to do so. * It will no longer be possible to alter a job attribute by adding a variant with only a files matcher -- in all cases files and irrelevant-files are used solely to determine whether the job is run, not to determine whether to apply a variant. See http://lists.openstack.org/pipermail/openstack-dev/2018-May/130074.html and http://lists.zuul-ci.org/pipermail/zuul-discuss/2018-May/000397.html for further context. Change-Id: I7b619df10c0896e696e065cb093d4e11911d3ce1
This commit is contained in:
parent
9022b1e562
commit
b48cdf46de
@ -967,24 +967,21 @@ Here is an example of two job definitions:
|
||||
it will remain set for all child jobs and variants (it can not be
|
||||
set to ``false``).
|
||||
|
||||
.. _matchers:
|
||||
|
||||
The following job attributes are considered "matchers". They are
|
||||
not inherited in the usual manner, instead, these attributes are
|
||||
used to determine whether a specific variant is used when
|
||||
running a job.
|
||||
|
||||
.. attr:: branches
|
||||
|
||||
A regular expression (or list of regular expressions) which
|
||||
describe on what branches a job should run (or in the case of
|
||||
variants: to alter the behavior of a job for a certain branch).
|
||||
variants, to alter the behavior of a job for a certain branch).
|
||||
|
||||
This attribute is not inherited in the usual manner. Instead,
|
||||
it is used to determine whether each variant on which it appears
|
||||
will be used when running the job.
|
||||
|
||||
If there is no job definition for a given job which matches the
|
||||
branch of an item, then that job is not run for the item.
|
||||
Otherwise, all of the job variants which match that branch (and
|
||||
any other selection criteria) are used when freezing the job.
|
||||
However, if :attr:`job.override-checkout` or
|
||||
Otherwise, all of the job variants which match that branch are
|
||||
used when freezing the job. However, if
|
||||
:attr:`job.override-checkout` or
|
||||
:attr:`job.required-projects.override-checkout` are set for a
|
||||
project, Zuul will attempt to use the job variants which match
|
||||
the values supplied in ``override-checkout`` for jobs defined in
|
||||
@ -1053,18 +1050,20 @@ Here is an example of two job definitions:
|
||||
|
||||
.. attr:: files
|
||||
|
||||
This matcher indicates that the job should only run on changes
|
||||
where the specified files are modified. This is a regular
|
||||
expression or list of regular expressions.
|
||||
This indicates that the job should only run on changes where the
|
||||
specified files are modified. Unlike **branches**, this value
|
||||
is subject to inheritance and overriding, so only the final
|
||||
value is used to determine if the job should run. This is a
|
||||
regular expression or list of regular expressions.
|
||||
|
||||
.. attr:: irrelevant-files
|
||||
|
||||
This matcher is a negative complement of **files**. It
|
||||
indicates that the job should run unless *all* of the files
|
||||
changed match this list. In other words, if the regular
|
||||
expression ``docs/.*`` is supplied, then this job will not run
|
||||
if the only files changed are in the docs directory. A regular
|
||||
expression or list of regular expressions.
|
||||
This is a negative complement of **files**. It indicates that
|
||||
the job should run unless *all* of the files changed match this
|
||||
list. In other words, if the regular expression ``docs/.*`` is
|
||||
supplied, then this job will not run if the only files changed
|
||||
are in the docs directory. A regular expression or list of
|
||||
regular expressions.
|
||||
|
||||
.. _project:
|
||||
|
||||
|
@ -0,0 +1,8 @@
|
||||
features:
|
||||
- |
|
||||
Files (and irrelevant-files) matchers are now overridable. Zuul
|
||||
now uses only branch matchers to collect job variants. Once those
|
||||
variants are collected, they are combined, and the files and
|
||||
irrelevant-files attributes are inherited and overridden as any
|
||||
other job attribute. The final values are used to determine
|
||||
whether the job should ultimately run.
|
63
tests/fixtures/layouts/file-matchers.yaml
vendored
Normal file
63
tests/fixtures/layouts/file-matchers.yaml
vendored
Normal file
@ -0,0 +1,63 @@
|
||||
- pipeline:
|
||||
name: check
|
||||
manager: independent
|
||||
trigger:
|
||||
gerrit:
|
||||
- event: patchset-created
|
||||
success:
|
||||
gerrit:
|
||||
Verified: 1
|
||||
failure:
|
||||
gerrit:
|
||||
Verified: -1
|
||||
|
||||
- job:
|
||||
name: base
|
||||
parent: null
|
||||
run: playbooks/base.yaml
|
||||
|
||||
- job:
|
||||
name: parent-override-job
|
||||
files: parent1.txt
|
||||
|
||||
- job:
|
||||
name: parent-override-job
|
||||
files: parent2.txt
|
||||
|
||||
- job:
|
||||
name: child-job
|
||||
parent: parent-override-job
|
||||
|
||||
- job:
|
||||
name: child-override-job
|
||||
parent: parent-override-job
|
||||
files: child.txt
|
||||
|
||||
- job:
|
||||
name: project-override-job
|
||||
parent: parent-override-job
|
||||
|
||||
- job:
|
||||
name: irr-job
|
||||
files:
|
||||
- tests/.*
|
||||
irrelevant-files:
|
||||
- README
|
||||
|
||||
- job:
|
||||
name: irr-override-job
|
||||
parent: irr-job
|
||||
irrelevant-files:
|
||||
- README
|
||||
- tests/docs/.*
|
||||
|
||||
- project:
|
||||
name: org/project
|
||||
check:
|
||||
jobs:
|
||||
- child-job # parent2
|
||||
- child-override-job # child
|
||||
- project-override-job: # project
|
||||
files: project.txt
|
||||
- irr-job # tests/docs/foo tests/foo
|
||||
- irr-override-job # tests/foo
|
@ -82,22 +82,22 @@ class TestJob(BaseTestCase):
|
||||
def test_change_matches_returns_false_for_matched_skip_if(self):
|
||||
change = model.Change('project')
|
||||
change.files = ['/COMMIT_MSG', 'docs/foo']
|
||||
self.assertFalse(self.job.changeMatches(change))
|
||||
self.assertFalse(self.job.changeMatchesFiles(change))
|
||||
|
||||
def test_change_matches_returns_false_for_single_matched_skip_if(self):
|
||||
change = model.Change('project')
|
||||
change.files = ['docs/foo']
|
||||
self.assertFalse(self.job.changeMatches(change))
|
||||
self.assertFalse(self.job.changeMatchesFiles(change))
|
||||
|
||||
def test_change_matches_returns_true_for_unmatched_skip_if(self):
|
||||
change = model.Change('project')
|
||||
change.files = ['/COMMIT_MSG', 'foo']
|
||||
self.assertTrue(self.job.changeMatches(change))
|
||||
self.assertTrue(self.job.changeMatchesFiles(change))
|
||||
|
||||
def test_change_matches_returns_true_for_single_unmatched_skip_if(self):
|
||||
change = model.Change('project')
|
||||
change.files = ['foo']
|
||||
self.assertTrue(self.job.changeMatches(change))
|
||||
self.assertTrue(self.job.changeMatchesFiles(change))
|
||||
|
||||
def test_job_sets_defaults_for_boolean_attributes(self):
|
||||
self.assertIsNotNone(self.job.voting)
|
||||
@ -203,9 +203,9 @@ class TestJob(BaseTestCase):
|
||||
item = queue.enqueueChange(change)
|
||||
item.layout = self.layout
|
||||
|
||||
self.assertTrue(base.changeMatches(change))
|
||||
self.assertTrue(python27.changeMatches(change))
|
||||
self.assertFalse(python27diablo.changeMatches(change))
|
||||
self.assertTrue(base.changeMatchesBranch(change))
|
||||
self.assertTrue(python27.changeMatchesBranch(change))
|
||||
self.assertFalse(python27diablo.changeMatchesBranch(change))
|
||||
|
||||
item.freezeJobGraph()
|
||||
self.assertEqual(len(item.getJobs()), 1)
|
||||
@ -217,9 +217,9 @@ class TestJob(BaseTestCase):
|
||||
item = queue.enqueueChange(change)
|
||||
item.layout = self.layout
|
||||
|
||||
self.assertTrue(base.changeMatches(change))
|
||||
self.assertTrue(python27.changeMatches(change))
|
||||
self.assertTrue(python27diablo.changeMatches(change))
|
||||
self.assertTrue(base.changeMatchesBranch(change))
|
||||
self.assertTrue(python27.changeMatchesBranch(change))
|
||||
self.assertTrue(python27diablo.changeMatchesBranch(change))
|
||||
|
||||
item.freezeJobGraph()
|
||||
self.assertEqual(len(item.getJobs()), 1)
|
||||
@ -269,8 +269,8 @@ class TestJob(BaseTestCase):
|
||||
item = queue.enqueueChange(change)
|
||||
item.layout = self.layout
|
||||
|
||||
self.assertTrue(base.changeMatches(change))
|
||||
self.assertFalse(python27.changeMatches(change))
|
||||
self.assertTrue(base.changeMatchesFiles(change))
|
||||
self.assertFalse(python27.changeMatchesFiles(change))
|
||||
|
||||
item.freezeJobGraph()
|
||||
self.assertEqual([], item.getJobs())
|
||||
|
@ -4893,10 +4893,61 @@ For CI problems and help debugging, contact ci@example.org"""
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertHistory([
|
||||
dict(name='child-job', result='SUCCESS', changes='2,1'),
|
||||
dict(name='child-job', result='SUCCESS', changes='3,1'),
|
||||
dict(name='child-job', result='SUCCESS', changes='4,1'),
|
||||
])
|
||||
], ordered=False)
|
||||
|
||||
@simple_layout('layouts/file-matchers.yaml')
|
||||
def test_file_matchers(self):
|
||||
"Test several file matchers"
|
||||
files = {'parent1.txt': ''}
|
||||
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
|
||||
files=files)
|
||||
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
files = {'parent2.txt': ''}
|
||||
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
|
||||
files=files)
|
||||
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
files = {'child.txt': ''}
|
||||
C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C',
|
||||
files=files)
|
||||
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
files = {'project.txt': ''}
|
||||
D = self.fake_gerrit.addFakeChange('org/project', 'master', 'D',
|
||||
files=files)
|
||||
self.fake_gerrit.addEvent(D.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
files = {'tests/foo': ''}
|
||||
E = self.fake_gerrit.addFakeChange('org/project', 'master', 'E',
|
||||
files=files)
|
||||
self.fake_gerrit.addEvent(E.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
files = {'tests/docs/foo': ''}
|
||||
F = self.fake_gerrit.addFakeChange('org/project', 'master', 'F',
|
||||
files=files)
|
||||
self.fake_gerrit.addEvent(F.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertHistory([
|
||||
dict(name='child-job', result='SUCCESS', changes='2,1'),
|
||||
|
||||
dict(name='child-override-job', result='SUCCESS', changes='3,1'),
|
||||
|
||||
dict(name='project-override-job', result='SUCCESS', changes='4,1'),
|
||||
|
||||
dict(name='irr-job', result='SUCCESS', changes='5,1'),
|
||||
dict(name='irr-override-job', result='SUCCESS', changes='5,1'),
|
||||
|
||||
dict(name='irr-job', result='SUCCESS', changes='6,1'),
|
||||
], ordered=False)
|
||||
|
||||
def test_trusted_project_dep_on_non_live_untrusted_project(self):
|
||||
# Test we get a layout for trusted projects when they depend on
|
||||
|
@ -910,8 +910,6 @@ class Job(ConfigObject):
|
||||
success_message=None,
|
||||
failure_url=None,
|
||||
success_url=None,
|
||||
# Matchers. These are separate so they can be individually
|
||||
# overidden.
|
||||
branch_matcher=None,
|
||||
file_matcher=None,
|
||||
irrelevant_file_matcher=None, # skip-if
|
||||
@ -1270,7 +1268,7 @@ class Job(ConfigObject):
|
||||
|
||||
self.inheritance_path = self.inheritance_path + (repr(other),)
|
||||
|
||||
def changeMatches(self, change, override_branch=None):
|
||||
def changeMatchesBranch(self, change, override_branch=None):
|
||||
if override_branch is None:
|
||||
branch_change = change
|
||||
else:
|
||||
@ -1283,7 +1281,9 @@ class Job(ConfigObject):
|
||||
if self.branch_matcher and not self.branch_matcher.matches(
|
||||
branch_change):
|
||||
return False
|
||||
return True
|
||||
|
||||
def changeMatchesFiles(self, change):
|
||||
if self.file_matcher and not self.file_matcher.matches(change):
|
||||
return False
|
||||
|
||||
@ -3031,8 +3031,9 @@ class Layout(object):
|
||||
branches = self.tenant.getProjectBranches(project)
|
||||
if override_branch not in branches:
|
||||
override_branch = None
|
||||
if not variant.changeMatches(change,
|
||||
override_branch=override_branch):
|
||||
if not variant.changeMatchesBranch(
|
||||
change,
|
||||
override_branch=override_branch):
|
||||
self.log.debug("Variant %s did not match %s", repr(variant),
|
||||
change)
|
||||
item.debug("Variant {variant} did not match".format(
|
||||
@ -3114,7 +3115,7 @@ class Layout(object):
|
||||
# jobs which match.
|
||||
override_checkouts = {}
|
||||
for variant in job_list.jobs[jobname]:
|
||||
if variant.changeMatches(change):
|
||||
if variant.changeMatchesBranch(change):
|
||||
self._updateOverrideCheckouts(override_checkouts, variant)
|
||||
try:
|
||||
variants = self.collectJobs(
|
||||
@ -3141,7 +3142,7 @@ class Layout(object):
|
||||
# variants
|
||||
matched = False
|
||||
for variant in job_list.jobs[jobname]:
|
||||
if variant.changeMatches(change):
|
||||
if variant.changeMatchesBranch(change):
|
||||
frozen_job.applyVariant(variant, item.layout)
|
||||
matched = True
|
||||
self.log.debug("Pipeline variant %s matched %s",
|
||||
@ -3159,6 +3160,12 @@ class Layout(object):
|
||||
item.debug("No matching pipeline variants for {jobname}".
|
||||
format(jobname=jobname), indent=2)
|
||||
continue
|
||||
if not frozen_job.changeMatchesFiles(change):
|
||||
self.log.debug("Job %s did not match files in %s",
|
||||
repr(frozen_job), change)
|
||||
item.debug("Job {jobname} did not match files".
|
||||
format(jobname=jobname), indent=2)
|
||||
continue
|
||||
if frozen_job.abstract:
|
||||
raise Exception("Job %s is abstract and may not be "
|
||||
"directly run" %
|
||||
|
Loading…
Reference in New Issue
Block a user