diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index be148194f2..a69de58667 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -1034,27 +1034,6 @@ Here is an example of two job definitions: branch, the branch containing the job definition is used as an implied branch specifier. - * In the case of a job variant defined within a :ref:`project`, - if the project definition is in a :term:`config-project`, no - implied branch specifier is used. If it appears in an - :term:`untrusted-project`, with no branch specifier, the - branch containing the project definition is used as an implied - branch specifier. - - * In the case of a job variant defined within a - :ref:`project-template`, if no branch specifier appears, the - 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 specifier, and then creates a new branch based on ``master``, @@ -1098,8 +1077,13 @@ participates in a pipeline. Multiple project definitions may appear for the same project (for example, in a central :term:`config projects ` as well as in a repo's own ``.zuul.yaml``). In this case, all of the project -definitions are combined (the jobs listed in all of the definitions -will be run). +definitions for the relevant branch are combined (the jobs listed in +all of the matching definitions will be run). If a project definition +appears in a :term:`config-project`, it will apply to all branches of +the project. If it appears in a branch of an +:term:`untrusted-project` it will only apply to changes on that +branch. In the case of an item which does not have a branch (for +example, a tag), all of the project definitions will be combined. Consider the following project definition:: @@ -1266,7 +1250,14 @@ A Project Template uses the same syntax as a :ref:`project` definition, however, in the case of a template, the :attr:`project.name` attribute does not refer to the name of a project, but rather names the template so that it can be referenced in -a `Project` definition. +a :ref:`project` definition. + +Because Project Templates may be used outside of the projects they are +defined, they honor the implied branches :ref:`pragma` (unlike +Projects). The same heuristics described in :attr:`job.branches` that +determine what implied branches a :ref:`job` will receive apply to +Project Templates (with the exception that it is not possible to +explicity set a branch matcher on a Project Template). .. _secret: @@ -1333,7 +1324,7 @@ branch will not immediately produce a configuration error. .. attr:: name :required: - The name of the secret, used in a :ref:`Job` definition to + The name of the secret, used in a :ref:`job` definition to request the secret. .. attr:: data @@ -1507,9 +1498,9 @@ pragma directives may not be set and then unset within the same file. This is a boolean, which, if set, may be used to enable (``True``) or disable (``False``) the addition of implied branch - matchers to job definitions. Normally Zuul decides whether to - add these based on heuristics described in :attr:`job.branches`. - This attribute overrides that behavior. + matchers to job and project-template definitions. Normally Zuul + decides whether to add these based on heuristics described in + :attr:`job.branches`. This attribute overrides that behavior. This can be useful if a project has multiple branches, yet the jobs defined in the master branch should apply to all branches. @@ -1521,7 +1512,8 @@ pragma directives may not be set and then unset within the same file. This is a list of regular expressions, just as :attr:`job.branches`, which may be used to supply the value of - the implied branch matcher for all jobs in a file. + the implied branch matcher for all jobs and project-templates in + a file. This may be useful if two projects share jobs but have dissimilar branch names. If, for example, two projects have diff --git a/releasenotes/notes/template-branch-matchers-2259b585d239b6fe.yaml b/releasenotes/notes/template-branch-matchers-2259b585d239b6fe.yaml new file mode 100644 index 0000000000..d18d9aeb38 --- /dev/null +++ b/releasenotes/notes/template-branch-matchers-2259b585d239b6fe.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Project Templates are now branch-aware and behave more like + project stanzas. If a template is defined on a branch, it will + only apply to changes to that branch. diff --git a/tests/fixtures/config/branch-tag/git/org_project/.zuul.yaml b/tests/fixtures/config/branch-tag/git/org_project/.zuul.yaml index acbba6cb53..e6515a74ad 100644 --- a/tests/fixtures/config/branch-tag/git/org_project/.zuul.yaml +++ b/tests/fixtures/config/branch-tag/git/org_project/.zuul.yaml @@ -6,4 +6,5 @@ name: org/project tag: jobs: + - central-job - test-job diff --git a/tests/fixtures/config/branch-tag/git/project-config/playbooks/central-job.yaml b/tests/fixtures/config/branch-tag/git/project-config/playbooks/central-job.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/branch-tag/git/project-config/playbooks/central-job.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/branch-tag/git/project-config/zuul.yaml b/tests/fixtures/config/branch-tag/git/project-config/zuul.yaml index 0ae639641a..63a7da66b4 100644 --- a/tests/fixtures/config/branch-tag/git/project-config/zuul.yaml +++ b/tests/fixtures/config/branch-tag/git/project-config/zuul.yaml @@ -10,6 +10,10 @@ name: base parent: null +- job: + name: central-job + run: playbooks/central-job.yaml + - project: name: project-config tag: diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 1c376af3e4..15f1ab0e32 100755 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -5227,7 +5227,7 @@ class TestSchedulerTemplatedProject(ZuulTestCase): def test_unimplied_branch_matchers(self): # This tests that there are no implied branch matchers added - # by project templates. + # to project templates in unbranched projects. self.create_branch('org/layered-project', 'stable') A = self.fake_gerrit.addFakeChange( diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 3963b41e77..dd71508a21 100755 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -402,13 +402,36 @@ class TestBranchDeletion(ZuulTestCase): 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. + def test_no_branch_match(self): + # Test that tag jobs run with no explicit branch matchers 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')]) + dict(name='central-job', result='SUCCESS', ref='refs/tags/foo'), + dict(name='test-job', result='SUCCESS', ref='refs/tags/foo')], + ordered=False) + + def test_no_branch_match_multi_branch(self): + # Test that tag jobs run with no explicit branch matchers in a + # multi-branch project (where jobs generally get implied + # branch matchers) + self.create_branch('org/project', 'stable/pike') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable/pike')) + self.waitUntilSettled() + + event = self.fake_gerrit.addFakeTag('org/project', 'master', 'foo') + self.fake_gerrit.addEvent(event) + self.waitUntilSettled() + # test-job does not run in this case because it is defined in + # a branched repo with implied branch matchers. A release job + # defined in a multi-branch repo would need at least one + # top-level variant with no branch matcher in order to match a + # tag. + self.assertHistory([ + dict(name='central-job', result='SUCCESS', ref='refs/tags/foo')]) class TestBranchNegative(ZuulTestCase): diff --git a/zuul/configloader.py b/zuul/configloader.py index 8c7f28baf0..be39e6334e 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -577,32 +577,6 @@ class JobParser(object): self.log = logging.getLogger("zuul.JobParser") self.pcontext = pcontext - def _getImpliedBranches(self, 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: - if job.source_context.implied_branches is not None: - return job.source_context.implied_branches - return [job.source_context.branch] - elif job.source_context.implied_branch_matchers is False: - return None - - # If this is a trusted project, don't create implied branch - # matchers. - if job.source_context.trusted: - return None - - # If this project only has one branch, don't create implied - # branch matchers. This way central job repos can work. - branches = self.pcontext.tenant.getProjectBranches( - job.source_context.project) - if len(branches) == 1: - return None - - if job.source_context.implied_branches is not None: - return job.source_context.implied_branches - return [job.source_context.branch] - def fromYaml(self, conf, project_pipeline=False, name=None, validate=True): if validate: @@ -782,10 +756,10 @@ class JobParser(object): job.allowed_projects = frozenset(allowed) branches = None - if ('branches' not in conf): - branches = self._getImpliedBranches(job) - if (not branches) and ('branches' in conf): + if 'branches' in conf: branches = as_list(conf['branches']) + elif not project_pipeline: + branches = self.pcontext.getImpliedBranches(job.source_context) if branches: job.setBranchMatcher(branches) if 'files' in conf: @@ -868,6 +842,13 @@ class ProjectTemplateParser(object): self.parseJobList( conf_pipeline.get('jobs', []), source_context, start_mark, project_pipeline.job_list) + + # If this project definition is in a place where it + # should get implied branch matchers, set it. + branches = self.pcontext.getImpliedBranches(source_context) + if branches: + project_template.setImpliedBranchMatchers(branches) + if freeze: project_template.freeze() return project_template @@ -957,11 +938,15 @@ class ProjectParser(object): project_config.name = project.canonical_name - if not conf['_source_context'].trusted: - # If this project definition is in a place where it - # should get implied branch matchers, set it. - project_config.addImpliedBranchMatcher( - conf['_source_context'].branch) + # Pragmas can cause templates to end up with implied + # branch matchers for arbitrary branches, but project + # stanzas should not. They should either have the current + # branch or no branch matcher. + if conf['_source_context'].trusted: + project_config.setImpliedBranchMatchers([]) + else: + project_config.setImpliedBranchMatchers( + [conf['_source_context'].branch]) # Add templates for name in conf.get('templates', []): @@ -1184,6 +1169,32 @@ class ParseContext(object): self.project_template_parser = ProjectTemplateParser(self) self.project_parser = ProjectParser(self) + def getImpliedBranches(self, source_context): + # If the user has set a pragma directive for this, use the + # value (if unset, the value is None). + if source_context.implied_branch_matchers is True: + if source_context.implied_branches is not None: + return source_context.implied_branches + return [source_context.branch] + elif source_context.implied_branch_matchers is False: + return None + + # If this is a trusted project, don't create implied branch + # matchers. + if source_context.trusted: + return None + + # If this project only has one branch, don't create implied + # branch matchers. This way central job repos can work. + branches = self.tenant.getProjectBranches( + source_context.project) + if len(branches) == 1: + return None + + if source_context.implied_branches is not None: + return source_context.implied_branches + return [source_context.branch] + class TenantParser(object): def __init__(self, connections, scheduler, merger): diff --git a/zuul/model.py b/zuul/model.py index 0b207b1f8b..1cffcb8c1d 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1753,6 +1753,8 @@ class QueueItem(object): # Conditionally set self.ppc so that the debug method can # consult it as we resolve the jobs. self.project_pipeline_config = ppc + for msg in ppc.debug_messages: + self.debug(msg) job_graph = self.layout.createJobGraph(self, ppc) for job in job_graph.getJobs(): # Ensure that each jobs's dependencies are fully @@ -2537,6 +2539,10 @@ class ProjectPipelineConfig(ConfigObject): self.job_list = JobList() self.queue_name = None self.debug = False + self.debug_messages = [] + + def addDebug(self, msg): + self.debug_messages.append(msg) def update(self, other): if not isinstance(other, ProjectPipelineConfig): @@ -2564,6 +2570,10 @@ class ProjectConfig(ConfigObject): self.merge_mode = None self.default_branch = None + def __repr__(self): + return '' % ( + self.name, self.source_context, self.branch_matcher) + def copy(self): r = self.__class__(self.name) r.source_context = self.source_context @@ -2575,8 +2585,16 @@ class ProjectConfig(ConfigObject): r.default_branch = self.default_branch return r - def addImpliedBranchMatcher(self, branch): - self.branch_matcher = change_matcher.ImpliedBranchMatcher(branch) + def setImpliedBranchMatchers(self, branches): + if len(branches) == 0: + self.branch_matcher = None + elif len(branches) > 1: + matchers = [change_matcher.ImpliedBranchMatcher(branch) + for branch in branches] + self.branch_matcher = change_matcher.MatchAny(matchers) + else: + self.branch_matcher = change_matcher.ImpliedBranchMatcher( + branches[0]) def changeMatches(self, change): if self.branch_matcher and not self.branch_matcher.matches(change): @@ -3037,12 +3055,28 @@ class Layout(object): project_in_pipeline = False for pc in self.getProjectConfigs(item.change.project.canonical_name): if not pc.changeMatches(item.change): + msg = "Project %s did not match" % (pc,) + ppc.addDebug(msg) + self.log.debug("%s item %s" % (msg, item)) continue + msg = "Project %s matched" % (pc,) + ppc.addDebug(msg) + self.log.debug("%s item %s" % (msg, item)) for template_name in pc.templates: templates = self.getProjectTemplates(template_name) for template in templates: template_ppc = template.pipelines.get(item.pipeline.name) if template_ppc: + if not template.changeMatches(item.change): + msg = "Project template %s did not match" % ( + template,) + ppc.addDebug(msg) + self.log.debug("%s item %s" % (msg, item)) + continue + msg = "Project template %s matched" % ( + template,) + ppc.addDebug(msg) + self.log.debug("%s item %s" % (msg, item)) project_in_pipeline = True ppc.update(template_ppc) project_ppc = pc.pipelines.get(item.pipeline.name)