Add multi-branch support for project-templates
Currently, to avoid confusion, we simply ignore project-template
definitions after the first. However, a more intuitive approach
would be, if the template appears on multiple branches, to have
it reflect those branches.
To that end, add implied branch matchers to templates in the same
way that we do to jobs themselves. This should provide the same
intuitive behavior where a template defined in zuul-jobs will apply
to all branches, but one defined in a multi-branch repository will
add jobs with implied branch matchers.
When a template is defined more than once, combine them (in the same
way that multiple project definitions are defined) so the resulting
template contains all the jobs (likely with implied branch matchers).
Allow a template to be defined multiple times (e.g., branches) within
the same project, but do not allow it to be redefined in another
project.
Because the multiple project definitions in different branches may
end up applying the same template (which will have all of the
per-branch jobs) multiple times, detect duplicate job definitions
when applying templates and filter them out.
The test test_dynamic_template was originally written to verify that
a project could not redefine a template defined in another project.
Clarify that, and update it to support the newly reported error in
that condition.
Change-Id: I6613885a8c7ebf400e85041e0d68b2eb5ceb033f
diff --git a/tests/fixtures/config/central-jobs/git/central-jobs/README b/tests/fixtures/config/central-jobs/git/central-jobs/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/central-jobs/git/central-jobs/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/central-jobs/git/central-jobs/playbooks/central-job.yaml b/tests/fixtures/config/central-jobs/git/central-jobs/playbooks/central-job.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/central-jobs/git/central-jobs/playbooks/central-job.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/central-jobs/git/central-jobs/zuul.yaml b/tests/fixtures/config/central-jobs/git/central-jobs/zuul.yaml
new file mode 100644
index 0000000..2bf782e
--- /dev/null
+++ b/tests/fixtures/config/central-jobs/git/central-jobs/zuul.yaml
@@ -0,0 +1,9 @@
+- job:
+ name: central-job
+ run: playbooks/central-job.yaml
+
+- project-template:
+ name: central-jobs
+ check:
+ jobs:
+ - central-job
diff --git a/tests/fixtures/config/central-jobs/git/common-config/zuul.yaml b/tests/fixtures/config/central-jobs/git/common-config/zuul.yaml
new file mode 100644
index 0000000..c31af45
--- /dev/null
+++ b/tests/fixtures/config/central-jobs/git/common-config/zuul.yaml
@@ -0,0 +1,52 @@
+- pipeline:
+ name: check
+ manager: independent
+ trigger:
+ gerrit:
+ - event: patchset-created
+ success:
+ gerrit:
+ Verified: 1
+ failure:
+ gerrit:
+ Verified: -1
+
+- pipeline:
+ name: gate
+ manager: dependent
+ trigger:
+ gerrit:
+ - event: comment-added
+ approval:
+ - Approved: 1
+ success:
+ gerrit:
+ Verified: 2
+ submit: true
+ failure:
+ gerrit:
+ Verified: -2
+ start:
+ gerrit:
+ Verified: 0
+ precedence: high
+
+- job:
+ name: base
+ parent: null
+
+- project:
+ name: common-config
+ check:
+ jobs: []
+ gate:
+ jobs:
+ - noop
+
+- project:
+ name: org/project
+ check:
+ jobs: []
+ gate:
+ jobs:
+ - noop
diff --git a/tests/fixtures/config/central-jobs/git/org_project/README b/tests/fixtures/config/central-jobs/git/org_project/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/central-jobs/git/org_project/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/central-jobs/main.yaml b/tests/fixtures/config/central-jobs/main.yaml
new file mode 100644
index 0000000..08f4d5d
--- /dev/null
+++ b/tests/fixtures/config/central-jobs/main.yaml
@@ -0,0 +1,9 @@
+- tenant:
+ name: tenant-one
+ source:
+ gerrit:
+ config-projects:
+ - common-config
+ untrusted-projects:
+ - central-jobs
+ - org/project
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 80e6ccc..c04604d 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -251,6 +251,94 @@
self.waitUntilSettled()
+class TestCentralJobs(ZuulTestCase):
+ tenant_config_file = 'config/central-jobs/main.yaml'
+
+ def setUp(self):
+ super(TestCentralJobs, self).setUp()
+ self.create_branch('org/project', 'stable')
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'org/project', 'stable'))
+ self.waitUntilSettled()
+
+ def _updateConfig(self, config, branch):
+ file_dict = {'.zuul.yaml': config}
+ C = self.fake_gerrit.addFakeChange('org/project', branch, 'C',
+ files=file_dict)
+ C.addApproval('Code-Review', 2)
+ self.fake_gerrit.addEvent(C.addApproval('Approved', 1))
+ self.waitUntilSettled()
+ self.fake_gerrit.addEvent(C.getChangeMergedEvent())
+ self.waitUntilSettled()
+
+ def _test_central_job_on_branch(self, branch, other_branch):
+ # Test that a job defined on a branchless repo only runs on
+ # the branch applied
+ config = textwrap.dedent(
+ """
+ - project:
+ name: org/project
+ check:
+ jobs:
+ - central-job
+ """)
+ self._updateConfig(config, branch)
+
+ A = self.fake_gerrit.addFakeChange('org/project', branch, 'A')
+ self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ self.assertHistory([
+ dict(name='central-job', result='SUCCESS', changes='2,1')])
+
+ # No jobs should run for this change.
+ B = self.fake_gerrit.addFakeChange('org/project', other_branch, 'B')
+ self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ self.assertHistory([
+ dict(name='central-job', result='SUCCESS', changes='2,1')])
+
+ def test_central_job_on_stable(self):
+ self._test_central_job_on_branch('master', 'stable')
+
+ def test_central_job_on_master(self):
+ self._test_central_job_on_branch('stable', 'master')
+
+ def _test_central_template_on_branch(self, branch, other_branch):
+ # Test that a project-template defined on a branchless repo
+ # only runs on the branch applied
+ config = textwrap.dedent(
+ """
+ - project:
+ name: org/project
+ templates: ['central-jobs']
+ """)
+ self._updateConfig(config, branch)
+
+ A = self.fake_gerrit.addFakeChange('org/project', branch, 'A')
+ self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ self.assertHistory([
+ dict(name='central-job', result='SUCCESS', changes='2,1')])
+
+ # No jobs should run for this change.
+ B = self.fake_gerrit.addFakeChange('org/project', other_branch, 'B')
+ self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ self.assertHistory([
+ dict(name='central-job', result='SUCCESS', changes='2,1')])
+
+ def test_central_template_on_stable(self):
+ self._test_central_template_on_branch('master', 'stable')
+
+ def test_central_template_on_master(self):
+ self._test_central_template_on_branch('stable', 'master')
+
+
class TestInRepoConfig(ZuulTestCase):
# A temporary class to hold new tests while others are disabled
@@ -357,6 +445,8 @@
dict(name='project-test2', result='SUCCESS', changes='2,1')])
def test_dynamic_template(self):
+ # Tests that a project can't update a template in another
+ # project.
in_repo_conf = textwrap.dedent(
"""
- job:
@@ -378,8 +468,12 @@
files=file_dict)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
- self.assertHistory([
- dict(name='template-job', result='SUCCESS', changes='1,1')])
+
+ self.assertEqual(A.patchsets[0]['approvals'][0]['value'], "-1")
+ self.assertIn('Project template common-config-template '
+ 'is already defined',
+ A.messages[0],
+ "A should have failed the check pipeline")
def test_dynamic_config_non_existing_job(self):
"""Test that requesting a non existent job fails"""
diff --git a/zuul/configloader.py b/zuul/configloader.py
index ec2c1e0..01a87fd 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -477,12 +477,7 @@
]
@staticmethod
- def _getImpliedBranches(tenant, job, project_pipeline):
- # If this is a project pipeline, don't create implied branch
- # matchers -- that's handled in ProjectParser.
- if project_pipeline:
- return None
-
+ def _getImpliedBranches(tenant, job):
# If the user has set a pragma directive for this, use the
# value (if unset, the value is None).
if job.source_context.implied_branch_matchers is True:
@@ -673,8 +668,7 @@
branches = None
if ('branches' not in conf):
- branches = JobParser._getImpliedBranches(
- tenant, job, project_pipeline)
+ branches = JobParser._getImpliedBranches(tenant, job)
if (not branches) and ('branches' in conf):
branches = as_list(conf['branches'])
if branches:
@@ -745,8 +739,8 @@
if validate:
with configuration_exceptions('project-template', conf):
self.schema(conf)
- project_template = model.ProjectConfig(conf['name'])
source_context = conf['_source_context']
+ project_template = model.ProjectConfig(conf['name'], source_context)
start_mark = conf['_start_mark']
for pipeline in self.layout.pipelines.values():
conf_pipeline = conf.get(pipeline.name)
@@ -1562,8 +1556,9 @@
classes = TenantParser._getLoadClasses(tenant, config_template)
if 'project-template' not in classes:
continue
- layout.addProjectTemplate(project_template_parser.fromYaml(
- config_template))
+ with configuration_exceptions('project-template', config_template):
+ layout.addProjectTemplate(project_template_parser.fromYaml(
+ config_template))
project_parser = ProjectParser(tenant, layout, project_template_parser)
for config_projects in data.projects.values():
diff --git a/zuul/model.py b/zuul/model.py
index 3f85087..68df7fc 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1094,7 +1094,8 @@
if not job.branch_matcher and implied_branch:
job = job.copy()
job.setBranchMatcher([implied_branch])
- joblist.append(job)
+ if job not in joblist:
+ joblist.append(job)
class JobGraph(object):
@@ -2209,8 +2210,11 @@
class ProjectConfig(object):
# Represents a project cofiguration
- def __init__(self, name):
+ def __init__(self, name, source_context=None):
self.name = name
+ # If this is a template, it will have a source_context, but
+ # not if it is a project definition.
+ self.source_context = source_context
self.merge_mode = None
# The default branch for the project (usually master).
self.default_branch = None
@@ -2479,12 +2483,18 @@
self.pipelines[pipeline.name] = pipeline
def addProjectTemplate(self, project_template):
- if project_template.name in self.project_templates:
- # TODO(jeblair): issue a warning to the logs on loading
- # the config, and an error when this hits in a proposed
- # change.
- return
- self.project_templates[project_template.name] = project_template
+ template = self.project_templates.get(project_template.name)
+ if template:
+ if (project_template.source_context.project !=
+ template.source_context.project):
+ raise Exception("Project template %s is already defined" %
+ (project_template.name,))
+ for pipeline in project_template.pipelines:
+ template.pipelines[pipeline].job_list.\
+ inheritFrom(project_template.pipelines[pipeline].job_list,
+ None)
+ else:
+ self.project_templates[project_template.name] = project_template
def addProjectConfig(self, project_config):
self.project_configs[project_config.name] = project_config