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>
This commit is contained in:
parent
5af998b406
commit
11041d2130
|
@ -418,34 +418,13 @@ explanation of each of the parameters::
|
|||
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 @@ explanation of each of the parameters::
|
|||
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
|
||||
|
|
|
@ -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
|
|
@ -118,7 +118,7 @@ class FakeChange(object):
|
|||
'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 @@ class FakeGerrit(object):
|
|||
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 @@ class TestScheduler(testtools.TestCase):
|
|||
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 @@ class TestScheduler(testtools.TestCase):
|
|||
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 @@ class TestScheduler(testtools.TestCase):
|
|||
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 @@ class TestScheduler(testtools.TestCase):
|
|||
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)
|
||||
|
|
|
@ -68,6 +68,11 @@ class LayoutSchema(object):
|
|||
'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 @@ class LayoutSchema(object):
|
|||
v.Required('manager'): manager,
|
||||
'precedence': precedence,
|
||||
'description': str,
|
||||
'require': require,
|
||||
'success-message': str,
|
||||
'failure-message': str,
|
||||
'merge-failure-message': str,
|
||||
|
|
|
@ -57,6 +57,11 @@ def time_to_seconds(s):
|
|||
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 @@ class Change(Changeish):
|
|||
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 @@ class EventFilter(object):
|
|||
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 @@ class EventFilter(object):
|
|||
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 = {}
|
||||
|
|
|
@ -29,7 +29,8 @@ import yaml
|
|||
|
||||
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 @@ class Scheduler(threading.Thread):
|
|||
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 @@ class BasePipelineManager(object):
|
|||
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 @@ class BasePipelineManager(object):
|
|||
|
||||
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 @@ class BasePipelineManager(object):
|
|||
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
|
||||
|
|
|
@ -363,6 +363,8 @@ class Gerrit(object):
|
|||
change.needed_by_changes.append(dep)
|
||||
|
||||
change.approvals = data['currentPatchSet'].get('approvals', [])
|
||||
change.open = data['open']
|
||||
change.status = data['status']
|
||||
|
||||
return change
|
||||
|
||||
|
|
Loading…
Reference in New Issue