Match tag items against containing branches
To try to approach a more intuitive behavior for jobs which apply to tags but are defined in-repo (or even for centrally defined jobs which should behave differently on tags from different branches), look up which branches contain the commit referenced by a tag and use that list in branch matchers. If a tag item is enqueued, we look up the branches which contain the commit referenced by the tag. If any of those branches match a branch matcher, the matcher is considered to have matched. This means that if a release job is defined on multiple branches, the branch variant from each branch the tagged commit is on will be used. A typical case is for a tagged commit to appear in exactly one branch. In that case, the most intuitive behavior (the version of the job defined on that branch) occurs. A less typical but perfectly reasonable case is that there are two identical branches (ie, stable has just branched from master but not diverged). In this case, if an identical commit is merged to both branches, then both variants of a release job will run. However, it's likely that these variants are identical anyway, so the result is apparently the same as the previous case. However if the variants are defined centrally, then they may differ while the branch contents are the same, causing unexpected behavior when both variants are applied. If two branches have diverged, it will not be possible for the same commit to be added to both branches, so in that case, only one of the variants will apply. However, tags can be created retroactively, so that even if a branch has diverged, if a commit in the history of both branches is tagged, then both variants will apply, possibly producing unexpected behavior. Considering that the current behavior is to apply all variants of jobs on tags all the time, the partial reduction of scope in the most typical circumstances is probably a useful change. Change-Id: I5734ed8aeab90c1754e27dc792d39690f16ac70c Co-Authored-By: Tobias Henkel <tobias.henkel@bmw.de>
This commit is contained in:
parent
1536b1b674
commit
04ac8287b6
|
@ -782,6 +782,10 @@ Here is an example of two job definitions:
|
|||
those projects. This can be used to run a job defined in one
|
||||
project on another project without a matching branch.
|
||||
|
||||
If a tag item is enqueued, we look up the branches which contain
|
||||
the commit referenced by the tag. If any of those branches match a
|
||||
branch matcher, the matcher is considered to have matched.
|
||||
|
||||
This example illustrates a job called *run-tests* which uses a
|
||||
nodeset based on the current release of an operating system to
|
||||
perform its tests, except when testing changes to the stable/2.0
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
features:
|
||||
- |
|
||||
Zuul now matches tag items against their containing branches. This makes
|
||||
it simpler to have release jobs in-tree. See :attr:`job.branches` for
|
||||
details.
|
|
@ -431,13 +431,67 @@ class TestBranchTag(ZuulTestCase):
|
|||
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.
|
||||
# test-job does run in this case because it is defined in a
|
||||
# branched repo with implied branch matchers, and the tagged
|
||||
# commit is in both branches.
|
||||
self.assertHistory([
|
||||
dict(name='central-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_divergent_multi_branch(self):
|
||||
# Test that tag jobs from divergent branches run different job
|
||||
# variants.
|
||||
self.create_branch('org/project', 'stable/pike')
|
||||
self.fake_gerrit.addEvent(
|
||||
self.fake_gerrit.getFakeBranchCreatedEvent(
|
||||
'org/project', 'stable/pike'))
|
||||
self.waitUntilSettled()
|
||||
|
||||
# Add a new job to master
|
||||
in_repo_conf = textwrap.dedent(
|
||||
"""
|
||||
- job:
|
||||
name: test2-job
|
||||
run: playbooks/test-job.yaml
|
||||
|
||||
- project:
|
||||
name: org/project
|
||||
tag:
|
||||
jobs:
|
||||
- central-job
|
||||
- test2-job
|
||||
""")
|
||||
|
||||
file_dict = {'.zuul.yaml': in_repo_conf}
|
||||
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
|
||||
files=file_dict)
|
||||
A.setMerged()
|
||||
self.fake_gerrit.addEvent(A.getChangeMergedEvent())
|
||||
self.waitUntilSettled()
|
||||
|
||||
event = self.fake_gerrit.addFakeTag(
|
||||
'org/project', 'stable/pike', 'foo')
|
||||
self.fake_gerrit.addEvent(event)
|
||||
self.waitUntilSettled()
|
||||
# test-job runs because we tagged stable/pike, but test2-job does
|
||||
# not, it only applied to master.
|
||||
self.assertHistory([
|
||||
dict(name='central-job', result='SUCCESS', ref='refs/tags/foo'),
|
||||
dict(name='test-job', result='SUCCESS', ref='refs/tags/foo')],
|
||||
ordered=False)
|
||||
|
||||
event = self.fake_gerrit.addFakeTag('org/project', 'master', 'bar')
|
||||
self.fake_gerrit.addEvent(event)
|
||||
self.waitUntilSettled()
|
||||
# test2-job runs because we tagged master, but test-job does
|
||||
# not, it only applied to stable/pike.
|
||||
self.assertHistory([
|
||||
dict(name='central-job', result='SUCCESS', ref='refs/tags/foo'),
|
||||
dict(name='test-job', result='SUCCESS', ref='refs/tags/foo'),
|
||||
dict(name='central-job', result='SUCCESS', ref='refs/tags/bar'),
|
||||
dict(name='test2-job', result='SUCCESS', ref='refs/tags/bar')],
|
||||
ordered=False)
|
||||
|
||||
|
||||
class TestBranchNegative(ZuulTestCase):
|
||||
|
|
|
@ -58,29 +58,29 @@ class ProjectMatcher(AbstractChangeMatcher):
|
|||
|
||||
|
||||
class BranchMatcher(AbstractChangeMatcher):
|
||||
fullmatch = False
|
||||
|
||||
def matches(self, change):
|
||||
if hasattr(change, 'branch'):
|
||||
# an implied branch matcher must do a fullmatch to work correctly
|
||||
if self.fullmatch:
|
||||
if self.regex.fullmatch(change.branch):
|
||||
return True
|
||||
else:
|
||||
if self.regex.match(change.branch):
|
||||
return True
|
||||
return False
|
||||
if self.regex.match(change.ref):
|
||||
return True
|
||||
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.fullmatch(change.branch):
|
||||
if hasattr(change, 'containing_branches'):
|
||||
for branch in change.containing_branches:
|
||||
if self.regex.fullmatch(branch):
|
||||
return True
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
class ImpliedBranchMatcher(BranchMatcher):
|
||||
fullmatch = True
|
||||
|
||||
|
||||
class FileMatcher(AbstractChangeMatcher):
|
||||
|
|
|
@ -1027,6 +1027,7 @@ class PipelineManager(object):
|
|||
def onMergeCompleted(self, event):
|
||||
build_set = event.build_set
|
||||
item = build_set.item
|
||||
item.change.containing_branches = event.item_in_branches
|
||||
build_set.merge_state = build_set.COMPLETE
|
||||
build_set.repo_state = event.repo_state
|
||||
if event.merged:
|
||||
|
|
|
@ -186,9 +186,11 @@ class MergeClient(object):
|
|||
commit = data.get('commit')
|
||||
files = data.get('files', {})
|
||||
repo_state = data.get('repo_state', {})
|
||||
item_in_branches = data.get('item_in_branches', [])
|
||||
job.files = files
|
||||
log.info("Merge %s complete, merged: %s, updated: %s, "
|
||||
"commit: %s", job, merged, job.updated, commit)
|
||||
"commit: %s, branches: %s", job, merged, job.updated, commit,
|
||||
item_in_branches)
|
||||
job.setComplete()
|
||||
if job.build_set:
|
||||
if job.name == 'merger:fileschanges':
|
||||
|
@ -196,8 +198,7 @@ class MergeClient(object):
|
|||
else:
|
||||
self.sched.onMergeCompleted(job.build_set,
|
||||
merged, job.updated, commit, files,
|
||||
repo_state)
|
||||
|
||||
repo_state, item_in_branches)
|
||||
# The test suite expects the job to be removed from the
|
||||
# internal account after the wake flag is set.
|
||||
self.jobs.remove(job)
|
||||
|
|
|
@ -610,6 +610,20 @@ class Repo(object):
|
|||
return int(m.group(1))
|
||||
return None
|
||||
|
||||
def contains(self, hexsha, zuul_event_id=None):
|
||||
repo = self.createRepoObject(zuul_event_id)
|
||||
log = get_annotated_logger(self.log, zuul_event_id)
|
||||
try:
|
||||
branches = repo.git.branch(contains=hexsha, color='never')
|
||||
except git.GitCommandError as e:
|
||||
if e.status == 129:
|
||||
log.debug("Found commit %s in no branches", hexsha)
|
||||
return []
|
||||
branches = [x.replace('*', '').strip() for x in branches.split('\n')]
|
||||
branches = [x for x in branches if x != '(no branch)']
|
||||
log.debug("Found commit %s in branches: %s", hexsha, branches)
|
||||
return branches
|
||||
|
||||
|
||||
class Merger(object):
|
||||
def __init__(self, working_root, connections, email, username,
|
||||
|
@ -910,6 +924,8 @@ class Merger(object):
|
|||
seen = set()
|
||||
recent = {}
|
||||
repo_state = {}
|
||||
# A list of branch names the last item appears in.
|
||||
item_in_branches = []
|
||||
for item in items:
|
||||
# If we're in the executor context we have the repo_locks object
|
||||
# and perform per repo locking.
|
||||
|
@ -926,7 +942,7 @@ class Merger(object):
|
|||
repo.reset()
|
||||
except Exception:
|
||||
self.log.exception("Unable to reset repo %s" % repo)
|
||||
return (False, {})
|
||||
return (False, {}, [])
|
||||
|
||||
self._saveRepoState(item['connection'], item['project'],
|
||||
repo, repo_state, recent, branches)
|
||||
|
@ -937,7 +953,10 @@ class Merger(object):
|
|||
self._alterRepoState(
|
||||
item['connection'], item['project'], repo_state,
|
||||
item['ref'], item['newrev'])
|
||||
return (True, repo_state)
|
||||
item = items[-1]
|
||||
repo = self.getRepo(item['connection'], item['project'])
|
||||
item_in_branches = repo.contains(item['newrev'])
|
||||
return (True, repo_state, item_in_branches)
|
||||
|
||||
def getFiles(self, connection_name, project_name, branch, files, dirs=[]):
|
||||
repo = self.getRepo(connection_name, project_name)
|
||||
|
|
|
@ -161,11 +161,13 @@ class BaseMergeServer(metaclass=ABCMeta):
|
|||
self.log.debug("Got refstate job: %s" % job.unique)
|
||||
args = json.loads(job.arguments)
|
||||
zuul_event_id = args.get('zuul_event_id')
|
||||
success, repo_state = self.merger.getRepoState(
|
||||
success, repo_state, item_in_branches = \
|
||||
self.merger.getRepoState(
|
||||
args['items'], branches=args.get('branches'),
|
||||
repo_locks=self.repo_locks)
|
||||
result = dict(updated=success,
|
||||
repo_state=repo_state)
|
||||
repo_state=repo_state,
|
||||
item_in_branches=item_in_branches)
|
||||
result['zuul_event_id'] = zuul_event_id
|
||||
job.sendWorkComplete(json.dumps(result))
|
||||
|
||||
|
|
|
@ -3136,6 +3136,7 @@ class Tag(Ref):
|
|||
def __init__(self, project):
|
||||
super(Tag, self).__init__(project)
|
||||
self.tag = None
|
||||
self.containing_branches = []
|
||||
|
||||
|
||||
class Change(Branch):
|
||||
|
|
|
@ -221,16 +221,19 @@ class MergeCompletedEvent(ResultEvent):
|
|||
:arg bool updated: Whether the repo was updated (changes without refs).
|
||||
:arg str commit: The SHA of the merged commit (changes with refs).
|
||||
:arg dict repo_state: The starting repo state before the merge.
|
||||
:arg list item_in_branches: A list of branches in which the final
|
||||
commit in the merge list appears (changes without refs).
|
||||
"""
|
||||
|
||||
def __init__(self, build_set, merged, updated, commit,
|
||||
files, repo_state):
|
||||
files, repo_state, item_in_branches):
|
||||
self.build_set = build_set
|
||||
self.merged = merged
|
||||
self.updated = updated
|
||||
self.commit = commit
|
||||
self.files = files
|
||||
self.repo_state = repo_state
|
||||
self.item_in_branches = item_in_branches
|
||||
|
||||
|
||||
class FilesChangesCompletedEvent(ResultEvent):
|
||||
|
@ -519,9 +522,10 @@ class Scheduler(threading.Thread):
|
|||
self.wake_event.set()
|
||||
|
||||
def onMergeCompleted(self, build_set, merged, updated,
|
||||
commit, files, repo_state):
|
||||
event = MergeCompletedEvent(build_set, merged,
|
||||
updated, commit, files, repo_state)
|
||||
commit, files, repo_state, item_in_branches):
|
||||
event = MergeCompletedEvent(build_set, merged, updated,
|
||||
commit, files, repo_state,
|
||||
item_in_branches)
|
||||
self.result_event_queue.put(event)
|
||||
self.wake_event.set()
|
||||
|
||||
|
|
Loading…
Reference in New Issue