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
diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst
index ef6259c..21d3bae 100644
--- a/doc/source/zuul.rst
+++ b/doc/source/zuul.rst
@@ -476,6 +476,10 @@
A boolean value (``true`` or ``false``) that indicates whether the change
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**
A string value that corresponds with the status of the change
reported by the trigger. For example, when using the Gerrit
diff --git a/tests/fixtures/layout-current-patchset.yaml b/tests/fixtures/layout-current-patchset.yaml
new file mode 100644
index 0000000..dc8f768
--- /dev/null
+++ b/tests/fixtures/layout-current-patchset.yaml
@@ -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
diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py
index f66c2fe..f5070bb 100755
--- a/tests/test_scheduler.py
+++ b/tests/test_scheduler.py
@@ -132,6 +132,7 @@
self.upstream_root = upstream_root
self.addPatchset()
self.data['submitRecords'] = self.getSubmitRecords()
+ self.open = True
def add_fake_change_to_repo(self, msg, fn, large):
path = os.path.join(self.upstream_root, self.project)
@@ -221,6 +222,23 @@
"reason": ""}
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',
granted_on=None):
if not granted_on:
@@ -4063,3 +4081,35 @@
self.getJobFromHistory('experimental-project-test').result,
'SUCCESS')
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([])
diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py
index 3e0a0ab..9a448a3 100644
--- a/zuul/layoutvalidator.py
+++ b/zuul/layoutvalidator.py
@@ -71,6 +71,7 @@
require = {'approval': toList(require_approval),
'open': bool,
+ 'current-patchset': bool,
'status': toList(str)}
window = v.All(int, v.Range(min=0))
diff --git a/zuul/model.py b/zuul/model.py
index c5c5c7d..39d004d 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1158,8 +1158,10 @@
class ChangeishFilter(object):
- def __init__(self, open=None, statuses=[], approvals=[]):
+ def __init__(self, open=None, current_patchset=None,
+ statuses=[], approvals=[]):
self.open = open
+ self.current_patchset = current_patchset
self.statuses = statuses
self.approvals = approvals
@@ -1176,10 +1178,12 @@
if self.open is not None:
ret += ' open: %s' % self.open
+ if self.current_patchset is not None:
+ ret += ' current-patchset: %s' % self.current_patchset
if self.statuses:
ret += ' statuses: %s' % ', '.join(self.statuses)
if self.approvals:
- ret += ' approvals: %s' % ', '.join(str(self.approvals))
+ ret += ' approvals: %s' % str(self.approvals)
ret += '>'
return ret
@@ -1189,6 +1193,10 @@
if self.open != change.open:
return False
+ if self.current_patchset is not None:
+ if self.current_patchset != change.is_current_patchset:
+ return False
+
if self.statuses:
if change.status not in self.statuses:
return False
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 55b1624..a7160c7 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -276,9 +276,11 @@
if 'require' in conf_pipeline:
require = conf_pipeline['require']
- f = ChangeishFilter(open=require.get('open'),
- statuses=toList(require.get('status')),
- approvals=toList(require.get('approval')))
+ f = ChangeishFilter(
+ open=require.get('open'),
+ current_patchset=require.get('current-patchset'),
+ statuses=toList(require.get('status')),
+ approvals=toList(require.get('approval')))
manager.changeish_filters.append(f)
# TODO: move this into triggers (may require pluggable