Add pipeline requirement for current-patchset.
Zuul can get itself into test loops when it receives comments for old patchsets because it attempts to vote on old patchsets which is not possible so the vote based requeuing stuff falls over and loops. Add an option to ignore old patchsets. Co-Authored-By: James E. Blair <jeblair@openstack.org> Change-Id: Ie3cd82fb9535e27b549a2483ac4fa9736930b044
This commit is contained in:
parent
11041d2130
commit
a9702ada76
|
@ -476,6 +476,10 @@ explanation of each of the parameters::
|
||||||
A boolean value (``true`` or ``false``) that indicates whether the change
|
A boolean value (``true`` or ``false``) that indicates whether the change
|
||||||
must be open or closed in order to be enqueued.
|
must be open or closed in order to be enqueued.
|
||||||
|
|
||||||
|
**current-patchset**
|
||||||
|
A boolean value (``true`` or ``false``) that indicates whether the change
|
||||||
|
must be the current patchset in order to be enqueued.
|
||||||
|
|
||||||
**status**
|
**status**
|
||||||
A string value that corresponds with the status of the change
|
A string value that corresponds with the status of the change
|
||||||
reported by the trigger. For example, when using the Gerrit
|
reported by the trigger. For example, when using the Gerrit
|
||||||
|
|
|
@ -0,0 +1,24 @@
|
||||||
|
includes:
|
||||||
|
- python-file: custom_functions.py
|
||||||
|
|
||||||
|
pipelines:
|
||||||
|
- name: check
|
||||||
|
manager: IndependentPipelineManager
|
||||||
|
require:
|
||||||
|
current-patchset: True
|
||||||
|
trigger:
|
||||||
|
gerrit:
|
||||||
|
- event: patchset-created
|
||||||
|
- event: comment-added
|
||||||
|
success:
|
||||||
|
gerrit:
|
||||||
|
verified: 1
|
||||||
|
failure:
|
||||||
|
gerrit:
|
||||||
|
verified: -1
|
||||||
|
|
||||||
|
projects:
|
||||||
|
- name: org/project
|
||||||
|
merge-mode: cherry-pick
|
||||||
|
check:
|
||||||
|
- project-check
|
|
@ -132,6 +132,7 @@ class FakeChange(object):
|
||||||
self.upstream_root = upstream_root
|
self.upstream_root = upstream_root
|
||||||
self.addPatchset()
|
self.addPatchset()
|
||||||
self.data['submitRecords'] = self.getSubmitRecords()
|
self.data['submitRecords'] = self.getSubmitRecords()
|
||||||
|
self.open = True
|
||||||
|
|
||||||
def add_fake_change_to_repo(self, msg, fn, large):
|
def add_fake_change_to_repo(self, msg, fn, large):
|
||||||
path = os.path.join(self.upstream_root, self.project)
|
path = os.path.join(self.upstream_root, self.project)
|
||||||
|
@ -221,6 +222,23 @@ class FakeChange(object):
|
||||||
"reason": ""}
|
"reason": ""}
|
||||||
return event
|
return event
|
||||||
|
|
||||||
|
def getChangeCommentEvent(self, patchset):
|
||||||
|
event = {"type": "comment-added",
|
||||||
|
"change": {"project": self.project,
|
||||||
|
"branch": self.branch,
|
||||||
|
"id": "I5459869c07352a31bfb1e7a8cac379cabfcb25af",
|
||||||
|
"number": str(self.number),
|
||||||
|
"subject": self.subject,
|
||||||
|
"owner": {"name": "User Name"},
|
||||||
|
"url": "https://hostname/3"},
|
||||||
|
"patchSet": self.patchsets[patchset - 1],
|
||||||
|
"author": {"name": "User Name"},
|
||||||
|
"approvals": [{"type": "Code-Review",
|
||||||
|
"description": "Code-Review",
|
||||||
|
"value": "0"}],
|
||||||
|
"comment": "This is a comment"}
|
||||||
|
return event
|
||||||
|
|
||||||
def addApproval(self, category, value, username='jenkins',
|
def addApproval(self, category, value, username='jenkins',
|
||||||
granted_on=None):
|
granted_on=None):
|
||||||
if not granted_on:
|
if not granted_on:
|
||||||
|
@ -4063,3 +4081,35 @@ For CI problems and help debugging, contact ci@example.org"""
|
||||||
self.getJobFromHistory('experimental-project-test').result,
|
self.getJobFromHistory('experimental-project-test').result,
|
||||||
'SUCCESS')
|
'SUCCESS')
|
||||||
self.assertEqual(A.reported, 1)
|
self.assertEqual(A.reported, 1)
|
||||||
|
|
||||||
|
def test_old_patchset_doesnt_trigger(self):
|
||||||
|
"Test that jobs never run against old patchsets"
|
||||||
|
self.config.set('zuul', 'layout_config',
|
||||||
|
'tests/fixtures/layout-current-patchset.yaml')
|
||||||
|
self.sched.reconfigure(self.config)
|
||||||
|
self.registerJobs()
|
||||||
|
# Create two patchsets and let their tests settle out. Then
|
||||||
|
# comment on first patchset and check that no additional
|
||||||
|
# jobs are run.
|
||||||
|
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
|
||||||
|
# Added because the layout file really wants an approval but this
|
||||||
|
# doesn't match anyways.
|
||||||
|
self.fake_gerrit.addEvent(A.addApproval('CRVW', 1))
|
||||||
|
self.waitUntilSettled()
|
||||||
|
A.addPatchset()
|
||||||
|
self.fake_gerrit.addEvent(A.addApproval('CRVW', 1))
|
||||||
|
self.waitUntilSettled()
|
||||||
|
|
||||||
|
old_history_count = len(self.history)
|
||||||
|
self.assertEqual(old_history_count, 2) # one job for each ps
|
||||||
|
self.fake_gerrit.addEvent(A.getChangeCommentEvent(1))
|
||||||
|
self.waitUntilSettled()
|
||||||
|
|
||||||
|
# Assert no new jobs ran after event for old patchset.
|
||||||
|
self.assertEqual(len(self.history), old_history_count)
|
||||||
|
|
||||||
|
# The last thing we did was add an event for a change then do
|
||||||
|
# nothing with a pipeline, so it will be in the cache;
|
||||||
|
# clean it up so it does not fail the test.
|
||||||
|
for pipeline in self.sched.layout.pipelines.values():
|
||||||
|
pipeline.trigger.maintainCache([])
|
||||||
|
|
|
@ -71,6 +71,7 @@ class LayoutSchema(object):
|
||||||
|
|
||||||
require = {'approval': toList(require_approval),
|
require = {'approval': toList(require_approval),
|
||||||
'open': bool,
|
'open': bool,
|
||||||
|
'current-patchset': bool,
|
||||||
'status': toList(str)}
|
'status': toList(str)}
|
||||||
|
|
||||||
window = v.All(int, v.Range(min=0))
|
window = v.All(int, v.Range(min=0))
|
||||||
|
|
|
@ -1158,8 +1158,10 @@ class EventFilter(object):
|
||||||
|
|
||||||
|
|
||||||
class ChangeishFilter(object):
|
class ChangeishFilter(object):
|
||||||
def __init__(self, open=None, statuses=[], approvals=[]):
|
def __init__(self, open=None, current_patchset=None,
|
||||||
|
statuses=[], approvals=[]):
|
||||||
self.open = open
|
self.open = open
|
||||||
|
self.current_patchset = current_patchset
|
||||||
self.statuses = statuses
|
self.statuses = statuses
|
||||||
self.approvals = approvals
|
self.approvals = approvals
|
||||||
|
|
||||||
|
@ -1176,10 +1178,12 @@ class ChangeishFilter(object):
|
||||||
|
|
||||||
if self.open is not None:
|
if self.open is not None:
|
||||||
ret += ' open: %s' % self.open
|
ret += ' open: %s' % self.open
|
||||||
|
if self.current_patchset is not None:
|
||||||
|
ret += ' current-patchset: %s' % self.current_patchset
|
||||||
if self.statuses:
|
if self.statuses:
|
||||||
ret += ' statuses: %s' % ', '.join(self.statuses)
|
ret += ' statuses: %s' % ', '.join(self.statuses)
|
||||||
if self.approvals:
|
if self.approvals:
|
||||||
ret += ' approvals: %s' % ', '.join(str(self.approvals))
|
ret += ' approvals: %s' % str(self.approvals)
|
||||||
ret += '>'
|
ret += '>'
|
||||||
|
|
||||||
return ret
|
return ret
|
||||||
|
@ -1189,6 +1193,10 @@ class ChangeishFilter(object):
|
||||||
if self.open != change.open:
|
if self.open != change.open:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
if self.current_patchset is not None:
|
||||||
|
if self.current_patchset != change.is_current_patchset:
|
||||||
|
return False
|
||||||
|
|
||||||
if self.statuses:
|
if self.statuses:
|
||||||
if change.status not in self.statuses:
|
if change.status not in self.statuses:
|
||||||
return False
|
return False
|
||||||
|
|
|
@ -276,9 +276,11 @@ class Scheduler(threading.Thread):
|
||||||
|
|
||||||
if 'require' in conf_pipeline:
|
if 'require' in conf_pipeline:
|
||||||
require = conf_pipeline['require']
|
require = conf_pipeline['require']
|
||||||
f = ChangeishFilter(open=require.get('open'),
|
f = ChangeishFilter(
|
||||||
statuses=toList(require.get('status')),
|
open=require.get('open'),
|
||||||
approvals=toList(require.get('approval')))
|
current_patchset=require.get('current-patchset'),
|
||||||
|
statuses=toList(require.get('status')),
|
||||||
|
approvals=toList(require.get('approval')))
|
||||||
manager.changeish_filters.append(f)
|
manager.changeish_filters.append(f)
|
||||||
|
|
||||||
# TODO: move this into triggers (may require pluggable
|
# TODO: move this into triggers (may require pluggable
|
||||||
|
|
Loading…
Reference in New Issue