Browse Source

Fix implied branch matchers and tags

Adding an implied branch matcher to jobs on in-repo project defs
works great when an item *has* a branch.  But some items, such as
tags, don't.  With recent changes, it is now impossible for a
project to add a job in-repo that runs in a tag pipeline.

To correct this, we need to drop some of the optimizations which
assumed we could match the implied branch against existing branch
matchers, and instead, when adding a job in-repo, simply add a new
kind of branch matcher, an ImpliedBranchMatcher, that is evaluated
in a boolean 'and' with any existing branch matchers.

The ImpliedBranchMatcher only fails if the item has a branch, and
the branch doesn't match.  If the item doesn't have a branch, it
always succeeds.

This means that when a project adds a job to a tag pipeline in-repo,
it will most likely only have the ImpliedBranchMatcher, which will
simply succeed.

It also means that the multiple project configurations present in
the project's multiple branches can all add jobs to tag pipelines,
and so to remove such a job, changes may need to be made to all
branches of a project.  However, there's not much that can be done
about that at the moment.

Change-Id: Id51ddfce7ef0a6d5e3273da784e407ac72a669db
changes/71/524771/1
James E. Blair 4 years ago
parent
commit
1edfd97943
  1. 23
      tests/base.py
  2. 9
      tests/fixtures/config/branch-tag/git/org_project/.zuul.yaml
  3. 1
      tests/fixtures/config/branch-tag/git/org_project/README
  4. 2
      tests/fixtures/config/branch-tag/git/org_project/playbooks/test-job.yaml
  5. 21
      tests/fixtures/config/branch-tag/git/project-config/zuul.yaml
  6. 8
      tests/fixtures/config/branch-tag/main.yaml
  7. 12
      tests/unit/test_v3.py
  8. 14
      zuul/change_matcher.py
  9. 32
      zuul/manager/__init__.py
  10. 26
      zuul/model.py

23
tests/base.py

@ -486,6 +486,29 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
self.changes[self.change_number] = c
return c
def addFakeTag(self, project, branch, tag):
path = os.path.join(self.upstream_root, project)
repo = git.Repo(path)
commit = repo.heads[branch].commit
newrev = commit.hexsha
ref = 'refs/tags/' + tag
git.Tag.create(repo, tag, commit)
event = {
"type": "ref-updated",
"submitter": {
"name": "User Name",
},
"refUpdate": {
"oldRev": 40 * '0',
"newRev": newrev,
"refName": ref,
"project": project,
}
}
return event
def getFakeBranchCreatedEvent(self, project, branch):
path = os.path.join(self.upstream_root, project)
repo = git.Repo(path)

9
tests/fixtures/config/branch-tag/git/org_project/.zuul.yaml

@ -0,0 +1,9 @@
- job:
name: test-job
run: playbooks/test-job.yaml
- project:
name: org/project
tag:
jobs:
- test-job

1
tests/fixtures/config/branch-tag/git/org_project/README

@ -0,0 +1 @@
test

2
tests/fixtures/config/branch-tag/git/org_project/playbooks/test-job.yaml

@ -0,0 +1,2 @@
- hosts: all
tasks: []

21
tests/fixtures/config/branch-tag/git/project-config/zuul.yaml

@ -0,0 +1,21 @@
- pipeline:
name: tag
manager: independent
trigger:
gerrit:
- event: ref-updated
ref: ^refs/tags/.*$
- job:
name: base
parent: null
- project:
name: project-config
tag:
jobs: []
- project:
name: org/project
tag:
jobs: []

8
tests/fixtures/config/branch-tag/main.yaml

@ -0,0 +1,8 @@
- tenant:
name: tenant-one
source:
gerrit:
config-projects:
- project-config
untrusted-projects:
- org/project

12
tests/unit/test_v3.py

@ -157,6 +157,18 @@ class TestFinal(ZuulTestCase):
self.assertIn('Unable to modify final job', A.messages[0])
class TestBranchTag(ZuulTestCase):
tenant_config_file = 'config/branch-tag/main.yaml'
def test_negative_branch_match(self):
# Test that a negative branch matcher works with implied branches.
event = self.fake_gerrit.addFakeTag('org/project', 'master', 'foo')
self.fake_gerrit.addEvent(event)
self.waitUntilSettled()
self.assertHistory([
dict(name='test-job', result='SUCCESS', ref='refs/tags/foo')])
class TestBranchNegative(ZuulTestCase):
tenant_config_file = 'config/branch-negative/main.yaml'

14
zuul/change_matcher.py

@ -69,6 +69,20 @@ class BranchMatcher(AbstractChangeMatcher):
return False
class ImpliedBranchMatcher(AbstractChangeMatcher):
"""
A branch matcher that only considers branch refs, and always
succeeds on other types (e.g., tags).
"""
def matches(self, change):
if hasattr(change, 'branch'):
if self.regex.match(change.branch):
return True
return False
return True
class FileMatcher(AbstractChangeMatcher):
def matches(self, change):

32
zuul/manager/__init__.py

@ -853,20 +853,22 @@ class PipelineManager(object):
if dt:
self.sched.statsd.timing(key + '.resident_time', dt)
self.sched.statsd.incr(key + '.total_changes')
hostname = (item.change.project.canonical_hostname.
replace('.', '_'))
projectname = (item.change.project.name.
replace('.', '_').replace('/', '.'))
projectname = projectname.replace('.', '_').replace('/', '.')
branchname = item.change.branch.replace('.', '_').replace('/', '.')
# stats.timers.zuul.tenant.<tenant>.pipeline.<pipeline>.
# project.<host>.<project>.<branch>.resident_time
# stats_counts.zuul.tenant.<tenant>.pipeline.<pipeline>.
# project.<host>.<project>.<branch>.total_changes
key += '.project.%s.%s.%s' % (hostname, projectname, branchname)
if dt:
self.sched.statsd.timing(key + '.resident_time', dt)
self.sched.statsd.incr(key + '.total_changes')
if hasattr(item.change, 'branch'):
hostname = (item.change.project.canonical_hostname.
replace('.', '_'))
projectname = (item.change.project.name.
replace('.', '_').replace('/', '.'))
projectname = projectname.replace('.', '_').replace('/', '.')
branchname = item.change.branch.replace('.', '_').replace(
'/', '.')
# stats.timers.zuul.tenant.<tenant>.pipeline.<pipeline>.
# project.<host>.<project>.<branch>.resident_time
# stats_counts.zuul.tenant.<tenant>.pipeline.<pipeline>.
# project.<host>.<project>.<branch>.total_changes
key += '.project.%s.%s.%s' % (hostname, projectname,
branchname)
if dt:
self.sched.statsd.timing(key + '.resident_time', dt)
self.sched.statsd.incr(key + '.total_changes')
except Exception:
self.log.exception("Exception reporting pipeline stats")

26
zuul/model.py

@ -963,10 +963,10 @@ class Job(object):
return None
return m
def addBranchMatcher(self, branch):
def addImpliedBranchMatcher(self, branch):
# Add a branch matcher that combines as a boolean *and* with
# existing branch matchers, if any.
matchers = [change_matcher.BranchMatcher(branch)]
matchers = [change_matcher.ImpliedBranchMatcher(branch)]
if self.branch_matcher:
matchers.append(self.branch_matcher)
self.branch_matcher = change_matcher.MatchAll(matchers)
@ -1121,26 +1121,8 @@ class JobList(object):
joblist = self.jobs.setdefault(jobname, [])
for job in jobs:
if implied_branch:
# If setting an implied branch and the current
# branch matcher is a simple match for a different
# branch, then simply do not add this job. If it
# is absent, set it to the implied branch.
# Otherwise, combine it with the implied branch to
# ensure that it still only affects this branch
# (whatever else it may do).
simple_branch = job.getSimpleBranchMatcher()
if simple_branch:
if not simple_branch.regex.match(implied_branch):
# This branch will never match, don't add it.
continue
if not simple_branch:
# The branch matcher could be complex, or
# missing. Add our implied matcher.
job = job.copy()
job.addBranchMatcher(implied_branch)
# Otherwise we have a simple branch matcher which
# is the same as our implied branch, the job can
# be added as-is.
job = job.copy()
job.addImpliedBranchMatcher(implied_branch)
if job not in joblist:
joblist.append(job)

Loading…
Cancel
Save