Combine branch templates and pipeline branch matchers
In a situation where a projec-template is defined on multiple branches
and then used by a project on multiple branches, the behavior was not
as one might expect.
Currently, assuming no explicit branch matchers are in play, all the
jobs listed in the project-template will get implied branch matchers
attached. So the resulting template looks like
- job @ master
- job @ stable/newton
- job @ stable/ocata
Then when that template is applied to the project, since the jobs within
already have branch matchers, the implied branch matcher for the project
pipeline definition is not applied. When the template is added to the
project on the ocata branch, all 3 of those jobs are added to the project,
and when it is added on the newton branch, all 3 jobs are added to the
project again. That's invisible to the user, until they attempt to remove
the template from one of the branches. Because the other branches still
add the template, which contains all the jobs, they still run.
To correct this, either replace the branch matcher obtained from the job
in the project template (if it is a simple match of a single branch), or
combine it using a boolean "and" (if it is something more complex) with a
branch matcher for the project definition. When the above template is added
to a project on the newton branch, only the following job will be added to
the project-pipeline:
- job @ stable/newton
When the template is added on the ocata branch, likewise only the ocata job
will be added.
If, instead, the project template had an explicit branch matcher, the
resulting template might be:
- job @ ^(?!stable/diablo).*$
After adding that to the project on the newton branch, the resulting project
pipeline would be:
- job @ { ^(?!stable/diablo).*$ AND stable/newton }
Ensuring that since the template was only added to the project on newton,
its jobs only run on newton changes.
Change-Id: I1969d588bc47b8ab5a54a885a68f98178b16b9d5
diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst
index fa874a9..3ea20ab 100644
--- a/doc/source/user/config.rst
+++ b/doc/source/user/config.rst
@@ -658,8 +658,17 @@
* In the case of a job variant defined within a
:ref:`project-template`, if no branch specifier appears, the
- implied branch specifier for the :ref:`project` definition which
- uses the project-template will be used.
+ implied branch containing the project-template definition is
+ used as an implied branch specifier. This means that
+ definitions of the same project-template on different branches
+ may run different jobs.
+
+ When that project-template is used by a :ref:`project`
+ definition within a :term:`untrusted-project`, the branch
+ containing that project definition is combined with the branch
+ specifier of the project-template. This means it is possible
+ for a project to use a template on one branch, but not on
+ another.
This allows for the very simple and expected workflow where if a
project defines a job on the ``master`` branch with no branch
diff --git a/tests/fixtures/config/branch-templates/git/project-config/zuul.yaml b/tests/fixtures/config/branch-templates/git/project-config/zuul.yaml
new file mode 100644
index 0000000..ce08877
--- /dev/null
+++ b/tests/fixtures/config/branch-templates/git/project-config/zuul.yaml
@@ -0,0 +1,26 @@
+- pipeline:
+ name: check
+ manager: independent
+ trigger:
+ gerrit:
+ - event: patchset-created
+ success:
+ gerrit:
+ Verified: 1
+ failure:
+ gerrit:
+ Verified: -1
+
+- job:
+ name: base
+ parent: null
+
+- project:
+ name: project-config
+ check:
+ jobs: []
+
+- project:
+ name: puppet-integration
+ check:
+ jobs: []
diff --git a/tests/fixtures/config/branch-templates/git/puppet-integration/.zuul.yaml b/tests/fixtures/config/branch-templates/git/puppet-integration/.zuul.yaml
new file mode 100644
index 0000000..dfea632
--- /dev/null
+++ b/tests/fixtures/config/branch-templates/git/puppet-integration/.zuul.yaml
@@ -0,0 +1,25 @@
+- job:
+ name: puppet-unit-base
+ run: playbooks/run-unit-tests.yaml
+
+- job:
+ name: puppet-unit-3.8
+ parent: puppet-unit-base
+ branches: ^(stable/(newton|ocata)).*$
+ vars:
+ puppet_gem_version: 3.8
+
+- job:
+ name: puppet-something
+ run: playbooks/run-unit-tests.yaml
+
+- project-template:
+ name: puppet-unit
+ check:
+ jobs:
+ - puppet-unit-3.8
+
+- project:
+ name: puppet-integration
+ templates:
+ - puppet-unit
diff --git a/tests/fixtures/config/branch-templates/git/puppet-integration/README b/tests/fixtures/config/branch-templates/git/puppet-integration/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/branch-templates/git/puppet-integration/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/branch-templates/git/puppet-integration/playbooks/run-unit-tests.yaml b/tests/fixtures/config/branch-templates/git/puppet-integration/playbooks/run-unit-tests.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/branch-templates/git/puppet-integration/playbooks/run-unit-tests.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/branch-templates/git/puppet-tripleo/.zuul.yaml b/tests/fixtures/config/branch-templates/git/puppet-tripleo/.zuul.yaml
new file mode 100644
index 0000000..4be8146
--- /dev/null
+++ b/tests/fixtures/config/branch-templates/git/puppet-tripleo/.zuul.yaml
@@ -0,0 +1,4 @@
+- project:
+ name: puppet-tripleo
+ templates:
+ - puppet-unit
diff --git a/tests/fixtures/config/branch-templates/git/puppet-tripleo/README b/tests/fixtures/config/branch-templates/git/puppet-tripleo/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/branch-templates/git/puppet-tripleo/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/branch-templates/main.yaml b/tests/fixtures/config/branch-templates/main.yaml
new file mode 100644
index 0000000..f7677a3
--- /dev/null
+++ b/tests/fixtures/config/branch-templates/main.yaml
@@ -0,0 +1,9 @@
+- tenant:
+ name: tenant-one
+ source:
+ gerrit:
+ config-projects:
+ - project-config
+ untrusted-projects:
+ - puppet-integration
+ - puppet-tripleo
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index c04604d..e2da808 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -157,6 +157,115 @@
self.assertIn('Unable to modify final job', A.messages[0])
+class TestBranchTemplates(ZuulTestCase):
+ tenant_config_file = 'config/branch-templates/main.yaml'
+
+ def test_template_removal_from_branch(self):
+ # Test that a template can be removed from one branch but not
+ # another.
+ # This creates a new branch with a copy of the config in master
+ self.create_branch('puppet-integration', 'stable/newton')
+ self.create_branch('puppet-integration', 'stable/ocata')
+ self.create_branch('puppet-tripleo', 'stable/newton')
+ self.create_branch('puppet-tripleo', 'stable/ocata')
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-integration', 'stable/newton'))
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-integration', 'stable/ocata'))
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-tripleo', 'stable/newton'))
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-tripleo', 'stable/ocata'))
+ self.waitUntilSettled()
+
+ in_repo_conf = textwrap.dedent(
+ """
+ - project:
+ name: puppet-tripleo
+ check:
+ jobs:
+ - puppet-something
+ """)
+
+ file_dict = {'.zuul.yaml': in_repo_conf}
+ A = self.fake_gerrit.addFakeChange('puppet-tripleo', 'stable/newton',
+ 'A', files=file_dict)
+ self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+ self.assertHistory([
+ dict(name='puppet-something', result='SUCCESS', changes='1,1')])
+
+ def test_template_change_on_branch(self):
+ # Test that the contents of a template can be changed on one
+ # branch without affecting another.
+
+ # This creates a new branch with a copy of the config in master
+ self.create_branch('puppet-integration', 'stable/newton')
+ self.create_branch('puppet-integration', 'stable/ocata')
+ self.create_branch('puppet-tripleo', 'stable/newton')
+ self.create_branch('puppet-tripleo', 'stable/ocata')
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-integration', 'stable/newton'))
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-integration', 'stable/ocata'))
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-tripleo', 'stable/newton'))
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-tripleo', 'stable/ocata'))
+ self.waitUntilSettled()
+
+ in_repo_conf = textwrap.dedent("""
+ - job:
+ name: puppet-unit-base
+ run: playbooks/run-unit-tests.yaml
+
+ - job:
+ name: puppet-unit-3.8
+ parent: puppet-unit-base
+ branches: ^(stable/(newton|ocata)).*$
+ vars:
+ puppet_gem_version: 3.8
+
+ - job:
+ name: puppet-something
+ run: playbooks/run-unit-tests.yaml
+
+ - project-template:
+ name: puppet-unit
+ check:
+ jobs:
+ - puppet-something
+
+ - project:
+ name: puppet-integration
+ templates:
+ - puppet-unit
+ """)
+
+ file_dict = {'.zuul.yaml': in_repo_conf}
+ A = self.fake_gerrit.addFakeChange('puppet-integration',
+ 'stable/newton',
+ 'A', files=file_dict)
+ B = self.fake_gerrit.addFakeChange('puppet-tripleo',
+ 'stable/newton',
+ 'B')
+ B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
+ B.subject, A.data['id'])
+ self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+ self.assertHistory([
+ dict(name='puppet-something', result='SUCCESS',
+ changes='1,1 2,1')])
+
+
class TestBranchVariants(ZuulTestCase):
tenant_config_file = 'config/branch-variants/main.yaml'
diff --git a/zuul/model.py b/zuul/model.py
index b027c53..5d8d271 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -948,6 +948,28 @@
matchers.append(change_matcher.BranchMatcher(branch))
self.branch_matcher = change_matcher.MatchAny(matchers)
+ def getSimpleBranchMatcher(self):
+ # If the job has a simple branch matcher, return it; otherwise None.
+ if not self.branch_matcher:
+ return None
+ m = self.branch_matcher
+ if not isinstance(m, change_matcher.AbstractMatcherCollection):
+ return None
+ if len(m.matchers) != 1:
+ return None
+ m = m.matchers[0]
+ if not isinstance(m, change_matcher.BranchMatcher):
+ return None
+ return m._regex
+
+ def addBranchMatcher(self, branch):
+ # Add a branch matcher that combines as a boolean *and* with
+ # existing branch matchers, if any.
+ matchers = [change_matcher.BranchMatcher(branch)]
+ if self.branch_matcher:
+ matchers.append(self.branch_matcher)
+ self.branch_matcher = change_matcher.MatchAll(matchers)
+
def updateVariables(self, other_vars):
v = copy.deepcopy(self.variables)
Job._deepUpdate(v, other_vars)
@@ -1097,9 +1119,26 @@
for jobname, jobs in other.jobs.items():
joblist = self.jobs.setdefault(jobname, [])
for job in jobs:
- if not job.branch_matcher and implied_branch:
- job = job.copy()
- job.setBranchMatcher([implied_branch])
+ 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 and simple_branch != implied_branch:
+ # Job is for a different branch, 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.
if job not in joblist:
joblist.append(job)