Merge "Specify gearman server listen address with separate option"
diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst
index b767210..4e04b04 100644
--- a/doc/source/zuul.rst
+++ b/doc/source/zuul.rst
@@ -466,21 +466,13 @@
A deprecated alternate spelling of *comment*. Only one of *comment* or
*comment_filter* should be used.
- *require-any-approval*
+ *require-approval*
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 follows the
same syntax as the :ref:`"approval" pipeline requirement below
<pipeline-require-approval>`.
- *require-all-approvals*
- This takes a list of approvals in the same format as
- *require-any-approval* but requires all approvals match the rules.
-
- **require-approval** (depreciated)
- A deprecated alternate spelling of *require-any-approval*. This will
- be joined with *require-any-approval* if both are present.
-
**timer**
This trigger will run based on a cron-style time specification.
It will enqueue an event into its pipeline for every project
@@ -527,7 +519,7 @@
.. _pipeline-require-approval:
- **any-approval**
+ **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
@@ -561,24 +553,6 @@
be a single value or a list: ``verified: [1, 2]`` would match
either a +1 or +2 vote.
- You can also match negative conditions by starting with an
- exclamation mark (!). This requires the value to be a string.
- Example: ``verified: '![-1, -2]'``
-
- This takes a list of approvals in the same format as above. It
- requires that any approval on a change can meet the specified
- criteria.
-
- **all-approvals**
- This takes a list of approvals in the same format as *any-approval* but
- requires all approvals match the rules. For example, you can stop any
- new changes from queueing when there is a negative vote by requiring
- all approves to not have a -1.
-
- **approval** (depreciated)
- A deprecated alternate spelling of *any-approval*. This will be
- joined with *any-approval* if both are present.
-
**open**
A boolean value (``true`` or ``false``) that indicates whether the change
must be open or closed in order to be enqueued.
diff --git a/tests/fixtures/layout-requirement-all.yaml b/tests/fixtures/layout-requirement-all.yaml
deleted file mode 100644
index 968739d..0000000
--- a/tests/fixtures/layout-requirement-all.yaml
+++ /dev/null
@@ -1,41 +0,0 @@
-pipelines:
- - name: pipeline
- manager: IndependentPipelineManager
- require:
- all-approvals:
- - username: jenkins
- verified: [1, 2]
- - verified: "![-1, -2]"
- trigger:
- gerrit:
- - event: comment-added
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
- - name: trigger
- manager: IndependentPipelineManager
- trigger:
- gerrit:
- - event: comment-added
- require-all-approvals:
- - username: jenkins
- verified: [1, 2]
- - verified: "![-1, -2]"
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
-projects:
- - name: org/project1
- pipeline:
- - project1-pipeline
- - name: org/project2
- trigger:
- - project2-trigger
diff --git a/tests/fixtures/layout-requirement-any.yaml b/tests/fixtures/layout-requirement-any.yaml
deleted file mode 100644
index 6275d8d..0000000
--- a/tests/fixtures/layout-requirement-any.yaml
+++ /dev/null
@@ -1,43 +0,0 @@
-pipelines:
- - name: pipeline
- manager: IndependentPipelineManager
- require:
- any-approval:
- - username: jenkins
- verified: [1, 2]
- - username: core-reviewer
- code-review: "![-1, -2]"
- trigger:
- gerrit:
- - event: comment-added
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
- - name: trigger
- manager: IndependentPipelineManager
- trigger:
- gerrit:
- - event: comment-added
- require-any-approval:
- - username: jenkins
- verified: [1, 2]
- - username: core-reviewer
- code-review: "![-1, -2]"
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
-projects:
- - name: org/project1
- pipeline:
- - project1-pipeline
- - name: org/project2
- trigger:
- - project2-trigger
diff --git a/tests/fixtures/layout-requirement-email.yaml b/tests/fixtures/layout-requirement-email.yaml
index dadcd6c..4bfb733 100644
--- a/tests/fixtures/layout-requirement-email.yaml
+++ b/tests/fixtures/layout-requirement-email.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- any-approval:
+ approval:
- email: jenkins@example.com
trigger:
gerrit:
@@ -19,7 +19,7 @@
trigger:
gerrit:
- event: comment-added
- require-any-approval:
+ require-approval:
- email: jenkins@example.com
success:
gerrit:
diff --git a/tests/fixtures/layout-requirement-negative-username.yaml b/tests/fixtures/layout-requirement-negative-username.yaml
deleted file mode 100644
index f542b86..0000000
--- a/tests/fixtures/layout-requirement-negative-username.yaml
+++ /dev/null
@@ -1,37 +0,0 @@
-pipelines:
- - name: pipeline
- manager: IndependentPipelineManager
- require:
- all-approvals:
- - username: '!jenkins'
- trigger:
- gerrit:
- - event: comment-added
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
- - name: trigger
- manager: IndependentPipelineManager
- trigger:
- gerrit:
- - event: comment-added
- require-all-approvals:
- - username: '!jenkins'
- success:
- gerrit:
- verified: 1
- failure:
- gerrit:
- verified: -1
-
-projects:
- - name: org/project1
- pipeline:
- - project1-pipeline
- - name: org/project2
- trigger:
- - project2-trigger
\ No newline at end of file
diff --git a/tests/fixtures/layout-requirement-newer-than.yaml b/tests/fixtures/layout-requirement-newer-than.yaml
index f723c79..b6beb35 100644
--- a/tests/fixtures/layout-requirement-newer-than.yaml
+++ b/tests/fixtures/layout-requirement-newer-than.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- any-approval:
+ approval:
- username: jenkins
newer-than: 48h
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-any-approval:
+ require-approval:
- username: jenkins
newer-than: 48h
success:
diff --git a/tests/fixtures/layout-requirement-older-than.yaml b/tests/fixtures/layout-requirement-older-than.yaml
index 0e011cc..2edf9df 100644
--- a/tests/fixtures/layout-requirement-older-than.yaml
+++ b/tests/fixtures/layout-requirement-older-than.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- any-approval:
+ approval:
- username: jenkins
older-than: 48h
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-any-approval:
+ require-approval:
- username: jenkins
older-than: 48h
success:
diff --git a/tests/fixtures/layout-requirement-username.yaml b/tests/fixtures/layout-requirement-username.yaml
index 8520179..7a549f0 100644
--- a/tests/fixtures/layout-requirement-username.yaml
+++ b/tests/fixtures/layout-requirement-username.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- any-approval:
+ approval:
- username: jenkins
trigger:
gerrit:
@@ -19,7 +19,7 @@
trigger:
gerrit:
- event: comment-added
- require-any-approval:
+ require-approval:
- username: jenkins
success:
gerrit:
diff --git a/tests/fixtures/layout-requirement-vote.yaml b/tests/fixtures/layout-requirement-vote.yaml
index 6736e98..7ccadff 100644
--- a/tests/fixtures/layout-requirement-vote.yaml
+++ b/tests/fixtures/layout-requirement-vote.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- any-approval:
+ approval:
- username: jenkins
verified: 1
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-any-approval:
+ require-approval:
- username: jenkins
verified: 1
success:
diff --git a/tests/fixtures/layout-requirement-vote1.yaml b/tests/fixtures/layout-requirement-vote1.yaml
index 6736e98..7ccadff 100644
--- a/tests/fixtures/layout-requirement-vote1.yaml
+++ b/tests/fixtures/layout-requirement-vote1.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- any-approval:
+ approval:
- username: jenkins
verified: 1
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-any-approval:
+ require-approval:
- username: jenkins
verified: 1
success:
diff --git a/tests/fixtures/layout-requirement-vote2.yaml b/tests/fixtures/layout-requirement-vote2.yaml
index a6cd6a3..33d84d1 100644
--- a/tests/fixtures/layout-requirement-vote2.yaml
+++ b/tests/fixtures/layout-requirement-vote2.yaml
@@ -2,7 +2,7 @@
- name: pipeline
manager: IndependentPipelineManager
require:
- any-approval:
+ approval:
- username: jenkins
verified: [1, 2]
trigger:
@@ -20,7 +20,7 @@
trigger:
gerrit:
- event: comment-added
- require-any-approval:
+ require-approval:
- username: jenkins
verified: [1, 2]
success:
diff --git a/tests/test_requirements.py b/tests/test_requirements.py
index 52e3973..4316925 100644
--- a/tests/test_requirements.py
+++ b/tests/test_requirements.py
@@ -323,131 +323,3 @@
self.fake_gerrit.addEvent(B.addApproval('CRVW', 2))
self.waitUntilSettled()
self.assertEqual(len(self.history), 1)
-
- def test_pipeline_require_negative_username(self):
- "Test negative pipeline requirement: no comment from jenkins"
- return self._test_require_negative_username('org/project1',
- 'project1-pipeline')
-
- def test_trigger_require_negative_username(self):
- "Test negative trigger requirement: no comment from jenkins"
- return self._test_require_negative_username('org/project2',
- 'project2-trigger')
-
- def _test_require_negative_username(self, project, job):
- "Test negative username's match"
- # Should only trigger if Jenkins hasn't voted.
- self.config.set(
- 'zuul', 'layout_config',
- 'tests/fixtures/layout-requirement-negative-username.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
-
- # add in a change with no comments
- A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 0)
-
- # add in a comment that will trigger
- self.fake_gerrit.addEvent(A.addApproval('CRVW', 1,
- username='reviewer'))
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
- self.assertEqual(self.history[0].name, job)
-
- # add in a comment from jenkins user which shouldn't trigger
- self.fake_gerrit.addEvent(A.addApproval('VRFY', 1, username='jenkins'))
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
-
- # Check future reviews also won't trigger as a 'jenkins' user has
- # commented previously
- self.fake_gerrit.addEvent(A.addApproval('CRVW', 1,
- username='reviewer'))
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
-
- def test_pipeline_require_any(self):
- "Test pipeline requirement: any requirement passes"
- return self._test_require_any('org/project1', 'project1-pipeline')
-
- def test_trigger_require_any(self):
- "Test trigger requirement: any requirement passes"
- return self._test_require_any('org/project2', 'project2-trigger')
-
- def _test_require_any(self, project, job):
- "Test any of the given requirements are matched"
- self.config.set(
- 'zuul', 'layout_config',
- 'tests/fixtures/layout-requirement-any.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
-
- A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
- # A comment event that we will keep submitting to trigger
- comment = A.addApproval('CRVW', 1, username='nobody')
- self.fake_gerrit.addEvent(comment)
- self.waitUntilSettled()
- # No approval from Jenkins so should not be enqueued
- self.assertEqual(len(self.history), 0)
-
- # A +1 from jenkins should allow it to be enqueued
- A.addApproval('VRFY', 1, username='jenkins')
- self.fake_gerrit.addEvent(comment)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
- self.assertEqual(self.history[0].name, job)
-
- # A non-negative from a non-core should not queue
- B = self.fake_gerrit.addFakeChange(project, 'master', 'B')
- # A comment event that we will keep submitting to trigger
- comment = B.addApproval('CRVW', 1, username='nobody')
- self.fake_gerrit.addEvent(comment)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
-
- # A non-negative from a core member should queue
- B.addApproval('CRVW', 2, username='core-reviewer')
- self.fake_gerrit.addEvent(comment)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 2)
- self.assertEqual(self.history[1].name, job)
-
- def test_pipeline_require_all(self):
- "Test pipeline requirement: all requirements pass"
- return self._test_require_all('org/project1', 'project1-pipeline')
-
- def test_trigger_require_all(self):
- "Test trigger requirement: all requirements pass"
- return self._test_require_all('org/project2', 'project2-trigger')
-
- def _test_require_all(self, project, job):
- "Test all of the given requirements are matched"
- self.config.set(
- 'zuul', 'layout_config',
- 'tests/fixtures/layout-requirement-all.yaml')
- self.sched.reconfigure(self.config)
- self.registerJobs()
-
- A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 0)
-
- # A +2 from 'nobody' only satisfies the non-negative requirement,
- # not the requirement to be from 'jenkins'
- comment = A.addApproval('VRFY', 1, username='nobody')
- self.fake_gerrit.addEvent(comment)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 0)
-
- B = self.fake_gerrit.addFakeChange(project, 'master', 'A')
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 0)
-
- # A +2 from Jenkins satisfies both the user condition and the
- # non-negative condition
- comment = B.addApproval('VRFY', 2, username='jenkins')
- self.fake_gerrit.addEvent(comment)
- self.waitUntilSettled()
- self.assertEqual(len(self.history), 1)
- self.assertEqual(self.history[0].name, job)
diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py
index 6c82748..8347f7a 100755
--- a/tests/test_scheduler.py
+++ b/tests/test_scheduler.py
@@ -2280,7 +2280,6 @@
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
A.addPatchset(['conflict'])
A.addApproval('CRVW', 2)
- self.fake_gerrit.addEvent(A.addApproval('APRV', 1))
# This change will be in merge conflict. During the
# reconfiguration, we will add a job. We want to make sure
@@ -2288,6 +2287,8 @@
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
B.addPatchset(['conflict'])
B.addApproval('CRVW', 2)
+
+ self.fake_gerrit.addEvent(A.addApproval('APRV', 1))
self.fake_gerrit.addEvent(B.addApproval('APRV', 1))
self.waitUntilSettled()
diff --git a/zuul/cmd/cloner.py b/zuul/cmd/cloner.py
index 5bbe3a0..187b6b0 100755
--- a/zuul/cmd/cloner.py
+++ b/zuul/cmd/cloner.py
@@ -88,17 +88,14 @@
)
args = parser.parse_args()
+ # Validate ZUUL_* arguments. If ref is provided then URL is required.
+ zuul_args = [zuul_opt for zuul_opt, val in vars(args).items()
+ if zuul_opt.startswith('zuul') and val is not None]
+ if 'zuul_ref' in zuul_args and 'zuul_url' not in zuul_args:
+ parser.error("Specifying a Zuul ref requires a Zuul url. "
+ "Define Zuul arguments either via environment "
+ "variables or using options above.")
- # Validate ZUUL_* arguments. If any ZUUL_* argument is set they
- # must all be set, otherwise fallback to defaults.
- zuul_missing = [zuul_opt for zuul_opt, val in vars(args).items()
- if zuul_opt.startswith('zuul') and val is None]
- if (len(zuul_missing) > 0 and
- len(zuul_missing) < len(ZUUL_ENV_SUFFIXES)):
- parser.error(("Some Zuul parameters are not set:\n\t%s\n"
- "Define them either via environment variables or "
- "using options above." %
- "\n\t".join(sorted(zuul_missing))))
self.args = args
def setup_logging(self, color=False, verbose=False):
diff --git a/zuul/cmd/server.py b/zuul/cmd/server.py
index d7ac57d..2d99a1f 100755
--- a/zuul/cmd/server.py
+++ b/zuul/cmd/server.py
@@ -62,7 +62,10 @@
signal.signal(signal.SIGHUP, signal.SIG_IGN)
self.read_config()
self.setup_logging('zuul', 'log_config')
- self.sched.reconfigure(self.config)
+ try:
+ self.sched.reconfigure(self.config)
+ except Exception:
+ self.log.exception("Reconfiguration failed:")
signal.signal(signal.SIGHUP, self.reconfigure_handler)
def exit_handler(self, signum, frame):
diff --git a/zuul/launcher/gearman.py b/zuul/launcher/gearman.py
index 57ac5ca..828e2a9 100644
--- a/zuul/launcher/gearman.py
+++ b/zuul/launcher/gearman.py
@@ -342,8 +342,7 @@
build.parameters = params
if job.name == 'noop':
- build.result = 'SUCCESS'
- self.sched.onBuildCompleted(build)
+ self.sched.onBuildCompleted(build, 'SUCCESS')
return build
gearman_job = gear.Job(name, json.dumps(params),
@@ -431,8 +430,7 @@
build.retry = True
self.log.info("Build %s complete, result %s" %
(job, result))
- build.result = result
- self.sched.onBuildCompleted(build)
+ self.sched.onBuildCompleted(build, result)
# The test suite expects the build to be removed from the
# internal dict after it's added to the report queue.
del self.builds[job.unique]
diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py
index 1569fa9..88d10e2 100644
--- a/zuul/layoutvalidator.py
+++ b/zuul/layoutvalidator.py
@@ -62,8 +62,6 @@
'branch': toList(str),
'ref': toList(str),
'approval': toList(variable_dict),
- 'require-all-approvals': toList(require_approval),
- 'require-any-approval': toList(require_approval),
'require-approval': toList(require_approval),
}
@@ -87,9 +85,7 @@
},
}
- require = {'all-approvals': toList(require_approval),
- 'any-approval': toList(require_approval),
- 'approval': toList(require_approval),
+ require = {'approval': toList(require_approval),
'open': bool,
'current-patchset': bool,
'status': toList(str)}
diff --git a/zuul/lib/cloner.py b/zuul/lib/cloner.py
index d697648..0ac7f0f 100644
--- a/zuul/lib/cloner.py
+++ b/zuul/lib/cloner.py
@@ -138,8 +138,11 @@
if project in self.project_branches:
indicated_branch = self.project_branches[project]
- override_zuul_ref = re.sub(self.zuul_branch, indicated_branch,
- self.zuul_ref)
+ if indicated_branch:
+ override_zuul_ref = re.sub(self.zuul_branch, indicated_branch,
+ self.zuul_ref)
+ else:
+ override_zuul_ref = None
if indicated_branch and repo.hasBranch(indicated_branch):
self.log.info("upstream repo has branch %s", indicated_branch)
@@ -150,14 +153,18 @@
# FIXME should be origin HEAD branch which might not be 'master'
fallback_branch = 'master'
- fallback_zuul_ref = re.sub(self.zuul_branch, fallback_branch,
- self.zuul_ref)
+ if self.zuul_branch:
+ fallback_zuul_ref = re.sub(self.zuul_branch, fallback_branch,
+ self.zuul_ref)
+ else:
+ fallback_zuul_ref = None
# If we have a non empty zuul_ref to use, use it. Otherwise we fall
# back to checking out the branch.
if ((override_zuul_ref and
self.fetchFromZuul(repo, project, override_zuul_ref)) or
- (fallback_zuul_ref != override_zuul_ref and
+ (fallback_zuul_ref and
+ fallback_zuul_ref != override_zuul_ref and
self.fetchFromZuul(repo, project, fallback_zuul_ref))):
# Work around a bug in GitPython which can not parse FETCH_HEAD
gitcmd = git.Git(dest)
@@ -169,9 +176,9 @@
# Checkout branch
self.log.info("Falling back to branch %s", fallback_branch)
try:
- repo.checkout('remotes/origin/%s' % fallback_branch)
+ commit = repo.checkout('remotes/origin/%s' % fallback_branch)
except (ValueError, GitCommandError):
self.log.exception("Fallback branch not found: %s",
fallback_branch)
- self.log.info("Prepared %s repo with branch %s",
- project, fallback_branch)
+ self.log.info("Prepared %s repo with branch %s at commit %s",
+ project, fallback_branch, commit)
diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py
index f571ad0..c84d042 100644
--- a/zuul/merger/merger.py
+++ b/zuul/merger/merger.py
@@ -86,9 +86,9 @@
return repo
def reset(self):
- repo = self.createRepoObject()
self.log.debug("Resetting repository %s" % self.local_path)
self.update()
+ repo = self.createRepoObject()
origin = repo.remotes.origin
for ref in origin.refs:
if ref.remote_head == 'HEAD':
@@ -130,6 +130,7 @@
self.log.debug("Checking out %s" % ref)
repo.head.reference = ref
reset_repo_to_head(repo)
+ return repo.head.commit
def cherryPick(self, ref):
repo = self.createRepoObject()
diff --git a/zuul/model.py b/zuul/model.py
index 6648774..4b907c3 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -12,10 +12,8 @@
# License for the specific language governing permissions and limitations
# under the License.
-import ast
import copy
import re
-import six
import time
from uuid import uuid4
import extras
@@ -1027,127 +1025,68 @@
class BaseFilter(object):
- def __init__(self, required_any_approval=[], required_all_approvals=[]):
- self._required_any_approval = copy.deepcopy(required_any_approval)
- self.required_any_approval = self._tidy_approvals(
- required_any_approval)
- self._required_all_approvals = copy.deepcopy(required_all_approvals)
- self.required_all_approvals = self._tidy_approvals(
- required_all_approvals)
+ def __init__(self, required_approvals=[]):
+ self._required_approvals = copy.deepcopy(required_approvals)
+ self.required_approvals = required_approvals
- def _tidy_approvals(self, approvals):
- for a in approvals:
+ for a in self.required_approvals:
for k, v in a.items():
if k == 'username':
pass
elif k in ['email', 'email-filter']:
- a['email'] = v
+ a['email'] = re.compile(v)
elif k == 'newer-than':
- a[k] = v
+ a[k] = time_to_seconds(v)
elif k == 'older-than':
- a[k] = v
+ a[k] = time_to_seconds(v)
+ else:
+ if not isinstance(v, list):
+ a[k] = [v]
if 'email-filter' in a:
del a['email-filter']
- return approvals
-
- def _match_approval_required_approval(self, rapproval, approval):
- # Check if the required approval and approval match
- if 'description' not in approval:
- return False
- now = time.time()
- found_approval = True
- by = approval.get('by', {})
- for k, v in rapproval.items():
- negative_match = False
- item_match = True
- if isinstance(v, six.string_types) and v[0] == '!':
- v = v[1:].strip()
- item_match = False
- negative_match = True
-
- if k == 'username':
- if (by.get('username', '') != v):
- item_match = negative_match
- elif k == 'email':
- v = re.compile(v)
- if (not v.search(by.get('email', ''))):
- item_match = negative_match
- elif k == 'newer-than':
- t = now - time_to_seconds(v)
- if (approval['grantedOn'] < t):
- item_match = negative_match
- elif k == 'older-than':
- t = now - time_to_seconds(v)
- if (approval['grantedOn'] >= t):
- item_match = negative_match
- else:
- if isinstance(v, six.string_types):
- v = ast.literal_eval(v)
- if not isinstance(v, list):
- v = [v]
- if (normalizeCategory(approval['description']) != k or
- int(approval['value']) not in v):
- item_match = negative_match
- if not item_match:
- found_approval = False
- return found_approval
def matchesRequiredApprovals(self, change):
- if (self.required_any_approval and not change.approvals
- or self.required_all_approvals and not change.approvals):
- # A change with no approvals can not match
- return False
-
- # TODO(jhesketh): If we wanted to optimise this slightly we could
- # analyse both the ANY and ALL filters by looping over the approvals
- # on the change and keeping track of what we have checked rather than
- # needing to loop on the change approvals twice
- return (self.matchesRequiredAnyApproval(change) and
- self.matchesRequiredAllApprovals(change))
-
- def matchesRequiredAnyApproval(self, change):
- # Check if any approvals match the any requirements
- if not self.required_any_approval:
- # No approval required, so we must match
- return True
-
- for rapproval in self.required_any_approval:
+ now = time.time()
+ for rapproval in self.required_approvals:
matches_approval = False
for approval in change.approvals:
- matches_approval = self._match_approval_required_approval(
- rapproval, approval)
- if matches_approval:
- # We have a matching approval so this requirement is
- # fulfilled
- return True
- return False
-
- def matchesRequiredAllApprovals(self, change):
- # Check that /all/ of the approvals match the requirements
- if not self.required_all_approvals:
- # No approvals required, so we must match
- return True
-
- for rapproval in self.required_all_approvals:
- for approval in change.approvals:
- matches_approval = self._match_approval_required_approval(
- rapproval, approval)
- if not matches_approval:
- # We have an approval that doesn't match so this
- # requirement can't be fulfilled
- return False
- # We must have matched everything
+ 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':
+ 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']) not in v):
+ found_approval = False
+ if found_approval:
+ matches_approval = True
+ break
+ if not matches_approval:
+ return False
return True
class EventFilter(BaseFilter):
def __init__(self, trigger, types=[], branches=[], refs=[],
event_approvals={}, comments=[], emails=[], usernames=[],
- timespecs=[], required_any_approval=[],
- required_all_approvals=[], pipelines=[]):
+ timespecs=[], required_approvals=[], pipelines=[]):
super(EventFilter, self).__init__(
- required_any_approval=required_any_approval,
- required_all_approvals=required_all_approvals)
+ required_approvals=required_approvals)
self.trigger = trigger
self._types = types
self._branches = branches
@@ -1180,12 +1119,9 @@
if self.event_approvals:
ret += ' event_approvals: %s' % ', '.join(
['%s:%s' % a for a in self.event_approvals.items()])
- if self.required_any_approval:
- ret += ' required_any_approval: %s' % ', '.join(
- ['%s' % a for a in self._required_any_approval])
- if self.required_all_approvals:
- ret += ' required_all_approvals: %s' % ', '.join(
- ['%s' % a for a in self._required_all_approvals])
+ if self.required_approvals:
+ ret += ' required_approvals: %s' % ', '.join(
+ ['%s' % a for a in self._required_approvals])
if self._comments:
ret += ' comments: %s' % ', '.join(self._comments)
if self._emails:
@@ -1274,6 +1210,10 @@
if not matches_approval:
return False
+ if self.required_approvals and not change.approvals:
+ # A change with no approvals can not match
+ return False
+
# required approvals are ANDed
if not self.matchesRequiredApprovals(change):
return False
@@ -1291,11 +1231,9 @@
class ChangeishFilter(BaseFilter):
def __init__(self, open=None, current_patchset=None,
- statuses=[], required_any_approval=[],
- required_all_approvals=[]):
+ statuses=[], required_approvals=[]):
super(ChangeishFilter, self).__init__(
- required_any_approval=required_any_approval,
- required_all_approvals=required_all_approvals)
+ required_approvals=required_approvals)
self.open = open
self.current_patchset = current_patchset
self.statuses = statuses
@@ -1309,12 +1247,8 @@
ret += ' current-patchset: %s' % self.current_patchset
if self.statuses:
ret += ' statuses: %s' % ', '.join(self.statuses)
- if self.required_any_approval:
- ret += (' required_any_approval: %s' %
- str(self.required_any_approval))
- if self.required_all_approvals:
- ret += (' required_all_approvals: %s' %
- str(self.required_all_approvals))
+ if self.required_approvals:
+ ret += ' required_approvals: %s' % str(self.required_approvals)
ret += '>'
return ret
@@ -1332,6 +1266,10 @@
if change.status not in self.statuses:
return False
+ if self.required_approvals and not change.approvals:
+ # A change with no approvals can not match
+ return False
+
# required approvals are ANDed
if not self.matchesRequiredApprovals(change):
return False
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index a3d9978..e39e43e 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -327,10 +327,7 @@
open=require.get('open'),
current_patchset=require.get('current-patchset'),
statuses=toList(require.get('status')),
- required_any_approval=(toList(require.get('any-approval'))
- + toList(require.get('approval'))),
- required_all_approvals=toList(require.get('all-approvals'))
- )
+ required_approvals=toList(require.get('approval')))
manager.changeish_filters.append(f)
# TODO: move this into triggers (may require pluggable
@@ -360,13 +357,9 @@
comments=comments,
emails=emails,
usernames=usernames,
- required_any_approval=(
- toList(trigger.get('require-any-approval'))
- + toList(trigger.get('require-approval'))
- ),
- required_all_approvals=toList(
- trigger.get('require-all-approvals')
- ),
+ required_approvals=toList(
+ trigger.get('require-approval')
+ )
)
manager.event_filters.append(f)
if 'timer' in conf_pipeline['trigger']:
@@ -381,13 +374,9 @@
trigger=self.triggers['zuul'],
types=toList(trigger['event']),
pipelines=toList(trigger.get('pipeline')),
- required_any_approval=(
- toList(trigger.get('require-any-approval'))
- + toList(trigger.get('require-approval'))
- ),
- required_all_approvals=toList(
- trigger.get('require-all-approvals')
- ),
+ required_approvals=toList(
+ trigger.get('require-approval')
+ )
)
manager.event_filters.append(f)
@@ -550,9 +539,15 @@
self.wake_event.set()
self.log.debug("Done adding start event for build: %s" % build)
- def onBuildCompleted(self, build):
- self.log.debug("Adding complete event for build: %s" % build)
+ def onBuildCompleted(self, build, result):
+ self.log.debug("Adding complete event for build: %s result: %s" % (
+ build, result))
build.end_time = time.time()
+ # Note, as soon as the result is set, other threads may act
+ # upon this, even though the event hasn't been fully
+ # processed. Ensure that any other data from the event (eg,
+ # timing) is recorded before setting the result.
+ build.result = result
try:
if statsd and build.pipeline:
jobname = build.job.name.replace('.', '_')