Add pipeline requirements
Add a generalized pipeline requirements option that allows the
user to specify that a change must meet certain pre-requisites
before being enqueued into the pipeline. This is intended to
replace the "requires-approval" event filter on triggers (which
due to the automatic enqueuing of dependencies may not always
behave as the user intends). It also adds the ability to
specify that a change must be either open or closed or have one
of a number of specified statuses in order to be enqueued.
Change-Id: I7a55aa33fa8e1dcb405796261085e31138d37653
Co-Authored-By: Jeremy Stanley <fungi@yuggoth.org>
diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst
index fd26cc0..ef6259c 100644
--- a/doc/source/zuul.rst
+++ b/doc/source/zuul.rst
@@ -418,34 +418,13 @@
containing 'retrigger' somewhere in the comment text are added to a
change.
- *require-approval*
+ *require-approval* (deprecated)
This may be used for any event. It requires that a certain kind
of approval be present for the current patchset of the change (the
- approval could be added by the event in question). It takes
- several sub-parameters, all of which are optional and are combined
- together so that there must be an approval matching all specified
- requirements.
-
- *username*
- If present, an approval from this username is required.
-
- *email-filter*
- If present, an approval with this email address is required. It
- is treated as a regular expression as above.
-
- *older-than*
- If present, the approval must be older than this amount of time
- to match. Provide a time interval as a number with a suffix of
- "w" (weeks), "d" (days), "h" (hours), "m" (minutes), "s"
- (seconds). Example ``48h`` or ``2d``.
-
- *newer-than*
- If present, the approval must be newer than this amount of time
- to match. Same format as "older-than".
-
- Any other field is interpreted as a review category and value
- pair. For example ``verified: 1`` would require that the approval
- be for a +1 vote in the "Verified" column.
+ approval could be added by the event in question). It follows the
+ same syntax as the "approval" pipeline requirement below. This
+ form should be considered deprecated and the pipeline requirement
+ used instead.
**timer**
This trigger will run based on a cron-style time specification.
@@ -458,6 +437,49 @@
supported, not the symbolic names. Example: ``0 0 * * *`` runs
at midnight.
+**require**
+ If this section is present, it established pre-requisites for any
+ kind of item entering the Pipeline. Regardless of how the item is
+ to be enqueued (via any trigger or automatic dependency resolution),
+ the conditions specified here must be met or the item will not be
+ enqueued.
+
+ **approval**
+ This requires that a certain kind of approval be present for the
+ current patchset of the change (the approval could be added by the
+ event in question). It takes several sub-parameters, all of which
+ are optional and are combined together so that there must be an
+ approval matching all specified requirements.
+
+ *username*
+ If present, an approval from this username is required.
+
+ *email-filter*
+ If present, an approval with this email address is required. It
+ is treated as a regular expression as above.
+
+ *older-than*
+ If present, the approval must be older than this amount of time
+ to match. Provide a time interval as a number with a suffix of
+ "w" (weeks), "d" (days), "h" (hours), "m" (minutes), "s"
+ (seconds). Example ``48h`` or ``2d``.
+
+ *newer-than*
+ If present, the approval must be newer than this amount of time
+ to match. Same format as "older-than".
+
+ Any other field is interpreted as a review category and value
+ pair. For example ``verified: 1`` would require that the approval
+ be for a +1 vote in the "Verified" column.
+
+ **open**
+ A boolean value (``true`` or ``false``) that indicates whether the change
+ must be open or closed 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
+ trigger, status values such as ``NEW`` or ``MERGED`` may be useful.
**dequeue-on-new-patchset**
Normally, if a new patchset is uploaded to a change that is in a
diff --git a/tests/fixtures/layout-pipeline-requirements.yaml b/tests/fixtures/layout-pipeline-requirements.yaml
new file mode 100644
index 0000000..1826d88
--- /dev/null
+++ b/tests/fixtures/layout-pipeline-requirements.yaml
@@ -0,0 +1,59 @@
+includes:
+ - python-file: custom_functions.py
+
+pipelines:
+ - name: check
+ manager: IndependentPipelineManager
+ require:
+ approval:
+ - email-filter: jenkins@example.com
+ older-than: 48h
+ open: True
+ trigger:
+ gerrit:
+ - event: patchset-created
+ - event: comment-added
+ success:
+ gerrit:
+ verified: 1
+ failure:
+ gerrit:
+ verified: -1
+
+ - name: gate
+ manager: DependentPipelineManager
+ failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures
+ require:
+ status:
+ - NEW
+ approval:
+ - verified: 1
+ username: jenkins
+ newer-than: 48h
+ trigger:
+ gerrit:
+ - event: comment-added
+ approval:
+ - approved: 1
+ - event: comment-added
+ approval:
+ - verified: 1
+ success:
+ gerrit:
+ verified: 2
+ submit: true
+ failure:
+ gerrit:
+ verified: -2
+ start:
+ gerrit:
+ verified: 0
+ precedence: high
+
+projects:
+ - name: org/project
+ merge-mode: cherry-pick
+ check:
+ - project-check
+ gate:
+ - project-gate
diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py
index 563090b..f66c2fe 100755
--- a/tests/test_scheduler.py
+++ b/tests/test_scheduler.py
@@ -118,7 +118,7 @@
'id': 'I' + random_sha1(),
'lastUpdated': time.time(),
'number': str(number),
- 'open': True,
+ 'open': status == 'NEW',
'owner': {'email': 'user@example.com',
'name': 'User Name',
'username': 'username'},
@@ -345,10 +345,11 @@
self.change_number = 0
self.changes = {}
- def addFakeChange(self, project, branch, subject):
+ def addFakeChange(self, project, branch, subject, status='NEW'):
self.change_number += 1
c = FakeChange(self, self.change_number, project, branch, subject,
- upstream_root=self.upstream_root)
+ upstream_root=self.upstream_root,
+ status=status)
self.changes[self.change_number] = c
return c
@@ -1813,6 +1814,31 @@
self.assertTrue(trigger.canMerge(a, mgr.getSubmitAllowNeeds()))
trigger.maintainCache([])
+ def test_pipeline_requirements_closed_change(self):
+ "Test that pipeline requirements for closed changes are effective"
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-pipeline-requirements.yaml')
+ self.sched.reconfigure(self.config)
+
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ status='MERGED')
+ self.fake_gerrit.addEvent(A.addApproval('CRVW', 2))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+ self.assertEqual(len(self.builds), 0)
+
+ B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
+ status='MERGED')
+ B.addApproval('CRVW', 2)
+ B.addApproval('VRFY', 1)
+ self.fake_gerrit.addEvent(B.addApproval('APRV', 1))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+ self.assertEqual(len(self.builds), 0)
+
+ for pipeline in self.sched.layout.pipelines.values():
+ pipeline.trigger.maintainCache([])
+
def test_build_configuration(self):
"Test that zuul merges the right commits for testing"
@@ -2775,13 +2801,23 @@
self.assertEqual(D.data['status'], 'MERGED')
self.assertEqual(D.reported, 2)
+ def test_pipeline_requirements_approval_check_and_gate(self):
+ "Test pipeline requirements triggers both check and gate"
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-pipeline-requirements.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+ self._test_required_approval_check_and_gate()
+
def test_required_approval_check_and_gate(self):
"Test required-approval triggers both check and gate"
self.config.set('zuul', 'layout_config',
'tests/fixtures/layout-require-approval.yaml')
self.sched.reconfigure(self.config)
self.registerJobs()
+ self._test_required_approval_check_and_gate()
+ def _test_required_approval_check_and_gate(self):
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
A.addApproval('CRVW', 2)
# Add a too-old +1
@@ -2802,12 +2838,27 @@
self.assertEqual(len(self.history), 2)
self.assertEqual(self.history[1].name, 'project-gate')
+ def test_pipeline_requirements_approval_newer(self):
+ "Test pipeline requirements newer trigger parameter"
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-pipeline-requirements.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+ self._test_required_approval_newer()
+
def test_required_approval_newer(self):
"Test required-approval newer trigger parameter"
self.config.set('zuul', 'layout_config',
'tests/fixtures/layout-require-approval.yaml')
self.sched.reconfigure(self.config)
self.registerJobs()
+ self._test_required_approval_newer()
+
+ def _test_required_approval_newer(self):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-require-approval.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
A.addApproval('CRVW', 2)
@@ -2831,12 +2882,27 @@
self.assertEqual(len(self.history), 2)
self.assertEqual(self.history[1].name, 'project-gate')
+ def test_pipeline_requirements_approval_older(self):
+ "Test pipeline requirements older trigger parameter"
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-pipeline-requirements.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
+ self._test_required_approval_older()
+
def test_required_approval_older(self):
"Test required-approval older trigger parameter"
self.config.set('zuul', 'layout_config',
'tests/fixtures/layout-require-approval.yaml')
self.sched.reconfigure(self.config)
self.registerJobs()
+ self._test_required_approval_older()
+
+ def _test_required_approval_older(self):
+ self.config.set('zuul', 'layout_config',
+ 'tests/fixtures/layout-require-approval.yaml')
+ self.sched.reconfigure(self.config)
+ self.registerJobs()
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
crvw = A.addApproval('CRVW', 2)
diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py
index acc6879..3e0a0ab 100644
--- a/zuul/layoutvalidator.py
+++ b/zuul/layoutvalidator.py
@@ -68,6 +68,11 @@
'subject': str,
},
}
+
+ require = {'approval': toList(require_approval),
+ 'open': bool,
+ 'status': toList(str)}
+
window = v.All(int, v.Range(min=0))
window_floor = v.All(int, v.Range(min=1))
window_type = v.Any('linear', 'exponential')
@@ -77,6 +82,7 @@
v.Required('manager'): manager,
'precedence': precedence,
'description': str,
+ 'require': require,
'success-message': str,
'failure-message': str,
'merge-failure-message': str,
diff --git a/zuul/model.py b/zuul/model.py
index 8e2b2e5..c5c5c7d 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -57,6 +57,11 @@
raise Exception("Unable to parse time value: %s" % s)
+def normalizeCategory(name):
+ name = name.lower()
+ return re.sub(' ', '-', name)
+
+
class Pipeline(object):
"""A top-level pipeline such as check, gate, post, etc."""
def __init__(self, name):
@@ -844,6 +849,8 @@
self.is_merged = False
self.failed_to_merge = False
self.approvals = []
+ self.open = None
+ self.status = None
def _id(self):
return '%s,%s' % (self.number, self.patchset)
@@ -1036,10 +1043,6 @@
return ret
def matches(self, event, change):
- def normalizeCategory(name):
- name = name.lower()
- return re.sub(' ', '-', name)
-
# event types are ORed
matches_type = False
for etype in self.types:
@@ -1154,6 +1157,82 @@
return True
+class ChangeishFilter(object):
+ def __init__(self, open=None, statuses=[], approvals=[]):
+ self.open = open
+ self.statuses = statuses
+ self.approvals = approvals
+
+ for a in self.approvals:
+ if 'older-than' in a:
+ a['older-than'] = time_to_seconds(a['older-than'])
+ if 'newer-than' in a:
+ a['newer-than'] = time_to_seconds(a['newer-than'])
+ if 'email-filter' in a:
+ a['email-filter'] = re.compile(a['email-filter'])
+
+ def __repr__(self):
+ ret = '<ChangeishFilter'
+
+ if self.open is not None:
+ ret += ' open: %s' % self.open
+ if self.statuses:
+ ret += ' statuses: %s' % ', '.join(self.statuses)
+ if self.approvals:
+ ret += ' approvals: %s' % ', '.join(str(self.approvals))
+ ret += '>'
+
+ return ret
+
+ def matches(self, change):
+ if self.open is not None:
+ if self.open != change.open:
+ return False
+
+ if self.statuses:
+ if change.status not in self.statuses:
+ return False
+
+ if self.approvals and not change.approvals:
+ # A change with no approvals can not match
+ return False
+
+ now = time.time()
+ for rapproval in self.approvals:
+ matches_approval = False
+ for approval in change.approvals:
+ if 'description' not in approval:
+ continue
+ found_approval = True
+ by = approval.get('by', {})
+ for k, v in rapproval.items():
+ if k == 'username':
+ if (by.get('username', '') != v):
+ found_approval = False
+ elif k == 'email-filter':
+ if (not v.search(by.get('email', ''))):
+ found_approval = False
+ elif k == 'newer-than':
+ t = now - v
+ if (approval['grantedOn'] < t):
+ found_approval = False
+ elif k == 'older-than':
+ t = now - v
+ if (approval['grantedOn'] >= t):
+ found_approval = False
+ else:
+ if (normalizeCategory(approval['description']) != k or
+ int(approval['value']) != v):
+ found_approval = False
+ if found_approval:
+ matches_approval = True
+ break
+ if not matches_approval:
+ return False
+
+ return True
+
+
class Layout(object):
def __init__(self):
self.projects = {}
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 427ca4f..55b1624 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -29,7 +29,8 @@
import layoutvalidator
import model
-from model import ActionReporter, Pipeline, Project, ChangeQueue, EventFilter
+from model import ActionReporter, Pipeline, Project, ChangeQueue
+from model import EventFilter, ChangeishFilter
from zuul import version as zuul_version
statsd = extras.try_import('statsd.statsd')
@@ -273,6 +274,13 @@
pipeline.setManager(manager)
layout.pipelines[conf_pipeline['name']] = pipeline
+ 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')))
+ manager.changeish_filters.append(f)
+
# TODO: move this into triggers (may require pluggable
# configuration)
if 'gerrit' in conf_pipeline['trigger']:
@@ -903,6 +911,7 @@
self.sched = sched
self.pipeline = pipeline
self.event_filters = []
+ self.changeish_filters = []
if self.sched.config and self.sched.config.has_option(
'zuul', 'report_times'):
self.report_times = self.sched.config.getboolean(
@@ -915,6 +924,9 @@
def _postConfig(self, layout):
self.log.info("Configured Pipeline Manager %s" % self.pipeline.name)
+ self.log.info(" Requirements:")
+ for f in self.changeish_filters:
+ self.log.info(" %s" % f)
self.log.info(" Events:")
for e in self.event_filters:
self.log.info(" %s" % e)
@@ -1084,6 +1096,12 @@
change)
return False
+ for f in self.changeish_filters:
+ if not f.matches(change):
+ self.log.debug("Change %s does not match pipeline "
+ "requirements" % change)
+ return False
+
if not self.enqueueChangesAhead(change, quiet):
self.log.debug("Failed to enqueue changes ahead of %s" % change)
return False
diff --git a/zuul/trigger/gerrit.py b/zuul/trigger/gerrit.py
index dc66f97..a6eedb1 100644
--- a/zuul/trigger/gerrit.py
+++ b/zuul/trigger/gerrit.py
@@ -363,6 +363,8 @@
change.needed_by_changes.append(dep)
change.approvals = data['currentPatchSet'].get('approvals', [])
+ change.open = data['open']
+ change.status = data['status']
return change