Merge "Fix race in gerrit+github test" into feature/zuulv3
diff --git a/tests/base.py b/tests/base.py
index 0aac6bd..2b0194e 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -592,6 +592,21 @@
def getPullRequestClosedEvent(self):
return self._getPullRequestEvent('closed')
+ def getPushEvent(self, old_sha, ref='refs/heads/master'):
+ name = 'push'
+ data = {
+ 'ref': ref,
+ 'before': old_sha,
+ 'after': self.head_sha,
+ 'repository': {
+ 'full_name': self.project
+ },
+ 'sender': {
+ 'login': 'ghuser'
+ }
+ }
+ return (name, data)
+
def addComment(self, message):
self.comments.append(message)
self._updateTimeStamp()
diff --git a/tests/fixtures/config/push-reqs/git/common-config/playbooks/job1.yaml b/tests/fixtures/config/push-reqs/git/common-config/playbooks/job1.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/push-reqs/git/common-config/playbooks/job1.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/push-reqs/git/common-config/zuul.yaml b/tests/fixtures/config/push-reqs/git/common-config/zuul.yaml
new file mode 100644
index 0000000..6569966
--- /dev/null
+++ b/tests/fixtures/config/push-reqs/git/common-config/zuul.yaml
@@ -0,0 +1,119 @@
+- pipeline:
+ name: current
+ manager: independent
+ require:
+ github:
+ current-patchset: true
+ gerrit:
+ current-patchset: true
+ trigger:
+ github:
+ - event: push
+ gerrit:
+ - event: ref-updated
+
+- pipeline:
+ name: open
+ manager: independent
+ require:
+ github:
+ open: true
+ gerrit:
+ open: true
+ trigger:
+ github:
+ - event: push
+ gerrit:
+ - event: ref-updated
+
+- pipeline:
+ name: review
+ manager: independent
+ require:
+ github:
+ review:
+ - type: approval
+ gerrit:
+ approval:
+ - email: herp@derp.invalid
+ trigger:
+ github:
+ - event: push
+ gerrit:
+ - event: ref-updated
+
+- pipeline:
+ name: status
+ manager: independent
+ require:
+ github:
+ status: 'zuul:check:success'
+ trigger:
+ github:
+ - event: push
+
+- pipeline:
+ name: pushhub
+ manager: independent
+ require:
+ gerrit:
+ open: true
+ trigger:
+ github:
+ - event: push
+ gerrit:
+ - event: ref-updated
+
+- pipeline:
+ name: pushgerrit
+ manager: independent
+ require:
+ github:
+ open: true
+ trigger:
+ github:
+ - event: push
+ gerrit:
+ - event: ref-updated
+
+- job:
+ name: job1
+
+- project:
+ name: org/project1
+ current:
+ jobs:
+ - job1
+ open:
+ jobs:
+ - job1
+ review:
+ jobs:
+ - job1
+ status:
+ jobs:
+ - job1
+ pushhub:
+ jobs:
+ - job1
+ pushgerrit:
+ jobs:
+ - job1
+
+- project:
+ name: org/project2
+ current:
+ jobs:
+ - job1
+ open:
+ jobs:
+ - job1
+ review:
+ jobs:
+ - job1
+ pushhub:
+ jobs:
+ - job1
+ pushgerrit:
+ jobs:
+ - job1
diff --git a/tests/fixtures/config/push-reqs/git/org_project1/README b/tests/fixtures/config/push-reqs/git/org_project1/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/push-reqs/git/org_project1/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/push-reqs/git/org_project2/README b/tests/fixtures/config/push-reqs/git/org_project2/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/push-reqs/git/org_project2/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/push-reqs/main.yaml b/tests/fixtures/config/push-reqs/main.yaml
new file mode 100644
index 0000000..d9f1a42
--- /dev/null
+++ b/tests/fixtures/config/push-reqs/main.yaml
@@ -0,0 +1,11 @@
+- tenant:
+ name: tenant-one
+ source:
+ github:
+ config-projects:
+ - common-config
+ untrusted-projects:
+ - org/project1
+ gerrit:
+ untrusted-projects:
+ - org/project2
diff --git a/tests/fixtures/zuul-push-reqs.conf b/tests/fixtures/zuul-push-reqs.conf
new file mode 100644
index 0000000..661ac79
--- /dev/null
+++ b/tests/fixtures/zuul-push-reqs.conf
@@ -0,0 +1,23 @@
+[gearman]
+server=127.0.0.1
+
+[zuul]
+job_name_in_report=true
+status_url=http://zuul.example.com/status
+
+[merger]
+git_dir=/tmp/zuul-test/git
+git_user_email=zuul@example.com
+git_user_name=zuul
+zuul_url=http://zuul.example.com/p
+
+[executor]
+git_dir=/tmp/zuul-test/executor-git
+
+[connection github]
+driver=github
+
+[connection gerrit]
+driver=gerrit
+server=review.example.com
+user=jenkins
diff --git a/tests/unit/test_push_reqs.py b/tests/unit/test_push_reqs.py
new file mode 100644
index 0000000..657d9b8
--- /dev/null
+++ b/tests/unit/test_push_reqs.py
@@ -0,0 +1,53 @@
+# Copyright (c) 2017 IBM Corp.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from tests.base import ZuulTestCase
+
+
+class TestPushRequirements(ZuulTestCase):
+ config_file = 'zuul-push-reqs.conf'
+ tenant_config_file = 'config/push-reqs/main.yaml'
+
+ def setup_config(self):
+ super(TestPushRequirements, self).setup_config()
+
+ def test_push_requirements(self):
+ self.executor_server.hold_jobs_in_build = True
+
+ # Create a github change, add a change and emit a push event
+ A = self.fake_github.openFakePullRequest('org/project1', 'master', 'A')
+ old_sha = A.head_sha
+ self.fake_github.emitEvent(A.getPushEvent(old_sha))
+
+ self.waitUntilSettled()
+
+ # All but one pipeline should be skipped
+ self.assertEqual(1, len(self.builds))
+ self.assertEqual('pushhub', self.builds[0].pipeline)
+ self.assertEqual('org/project1', self.builds[0].project)
+
+ # Make a gerrit change, and emit a ref-updated event
+ B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
+ self.fake_gerrit.addEvent(B.getRefUpdatedEvent())
+
+ self.waitUntilSettled()
+
+ # All but one pipeline should be skipped, increasing builds by 1
+ self.assertEqual(2, len(self.builds))
+ self.assertEqual('pushgerrit', self.builds[1].pipeline)
+ self.assertEqual('org/project2', self.builds[1].project)
+
+ self.executor_server.hold_jobs_in_build = False
+ self.executor_server.release()
+ self.waitUntilSettled()
diff --git a/zuul/driver/gerrit/gerritmodel.py b/zuul/driver/gerrit/gerritmodel.py
index 009a723..818d260 100644
--- a/zuul/driver/gerrit/gerritmodel.py
+++ b/zuul/driver/gerrit/gerritmodel.py
@@ -115,6 +115,10 @@
return True
def matchesApprovals(self, change):
+ if self.required_approvals or self.reject_approvals:
+ if not hasattr(change, 'number'):
+ # Not a change, no reviews
+ return False
if (self.required_approvals and not change.approvals
or self.reject_approvals and not change.approvals):
# A change with no approvals can not match
@@ -291,10 +295,10 @@
class GerritRefFilter(RefFilter, GerritApprovalFilter):
- def __init__(self, open=None, current_patchset=None,
+ def __init__(self, connection_name, open=None, current_patchset=None,
statuses=[], required_approvals=[],
reject_approvals=[]):
- RefFilter.__init__(self)
+ RefFilter.__init__(self, connection_name)
GerritApprovalFilter.__init__(self,
required_approvals=required_approvals,
@@ -307,6 +311,7 @@
def __repr__(self):
ret = '<GerritRefFilter'
+ ret += ' connection_name: %s' % self.connection_name
if self.open is not None:
ret += ' open: %s' % self.open
if self.current_patchset is not None:
@@ -325,11 +330,21 @@
def matches(self, change):
if self.open is not None:
- if self.open != change.open:
+ # if a "change" has no number, it's not a change, but a push
+ # and cannot possibly pass this test.
+ if hasattr(change, 'number'):
+ if self.open != change.open:
+ return False
+ else:
return False
if self.current_patchset is not None:
- if self.current_patchset != change.is_current_patchset:
+ # if a "change" has no number, it's not a change, but a push
+ # and cannot possibly pass this test.
+ if hasattr(change, 'number'):
+ if self.current_patchset != change.is_current_patchset:
+ return False
+ else:
return False
if self.statuses:
diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py
index 6cb0c39..4571cc1 100644
--- a/zuul/driver/gerrit/gerritsource.py
+++ b/zuul/driver/gerrit/gerritsource.py
@@ -65,6 +65,7 @@
def getRequireFilters(self, config):
f = GerritRefFilter(
+ connection_name=self.connection.connection_name,
open=config.get('open'),
current_patchset=config.get('current-patchset'),
statuses=to_list(config.get('status')),
@@ -74,6 +75,7 @@
def getRejectFilters(self, config):
f = GerritRefFilter(
+ connection_name=self.connection.connection_name,
reject_approvals=to_list(config.get('approval')),
)
return [f]
diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py
index 3e25115..9516097 100644
--- a/zuul/driver/github/githubmodel.py
+++ b/zuul/driver/github/githubmodel.py
@@ -109,9 +109,13 @@
return True
def matchesReviews(self, change):
- if self.required_reviews and not change.reviews:
- # No reviews means no matching
- return False
+ if self.required_reviews:
+ if not hasattr(change, 'number'):
+ # not a PR, no reviews
+ return False
+ if not change.reviews:
+ # No reviews means no matching
+ return False
return self.matchesRequiredReviews(change)
@@ -133,6 +137,9 @@
# statuses and the filter statuses are a null intersection, there
# are no matches and we return false
if self.required_statuses:
+ if not hasattr(change, 'number'):
+ # not a PR, no status
+ return False
if set(change.status).isdisjoint(set(self.required_statuses)):
return False
return True
@@ -263,9 +270,9 @@
class GithubRefFilter(RefFilter, GithubCommonFilter):
- def __init__(self, statuses=[], required_reviews=[], open=None,
- current_patchset=None):
- RefFilter.__init__(self)
+ def __init__(self, connection_name, statuses=[], required_reviews=[],
+ open=None, current_patchset=None):
+ RefFilter.__init__(self, connection_name)
GithubCommonFilter.__init__(self, required_reviews=required_reviews,
required_statuses=statuses)
@@ -276,6 +283,7 @@
def __repr__(self):
ret = '<GithubRefFilter'
+ ret += ' connection_name: %s' % self.connection_name
if self.statuses:
ret += ' statuses: %s' % ', '.join(self.statuses)
if self.required_reviews:
@@ -295,11 +303,21 @@
return False
if self.open is not None:
- if self.open != change.open:
+ # if a "change" has no number, it's not a change, but a push
+ # and cannot possibly pass this test.
+ if hasattr(change, 'number'):
+ if self.open != change.open:
+ return False
+ else:
return False
if self.current_patchset is not None:
- if self.current_patchset != change.is_current_patchset:
+ # if a "change" has no number, it's not a change, but a push
+ # and cannot possibly pass this test.
+ if hasattr(change, 'number'):
+ if self.current_patchset != change.is_current_patchset:
+ return False
+ else:
return False
# required reviews are ANDed
diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py
index 58ca2b9..1350b10 100644
--- a/zuul/driver/github/githubsource.py
+++ b/zuul/driver/github/githubsource.py
@@ -96,6 +96,7 @@
def getRequireFilters(self, config):
f = GithubRefFilter(
+ connection_name=self.connection.connection_name,
statuses=to_list(config.get('status')),
required_reviews=to_list(config.get('review')),
open=config.get('open'),
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 0f8cd0f..2302412 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -304,6 +304,8 @@
def _copy_ansible_files(python_module, target_dir):
library_path = os.path.dirname(os.path.abspath(python_module.__file__))
for fn in os.listdir(library_path):
+ if fn == "__pycache__":
+ continue
full_path = os.path.join(library_path, fn)
if os.path.isdir(full_path):
shutil.copytree(full_path, os.path.join(target_dir, fn))
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index 7649944..93522f0 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -282,6 +282,10 @@
if not ignore_requirements:
for f in self.changeish_filters:
+ if f.connection_name != change.project.connection_name:
+ self.log.debug("Filter %s skipped for change %s due "
+ "to mismatched connections" % (f, change))
+ continue
if not f.matches(change):
self.log.debug("Change %s does not match pipeline "
"requirement %s" % (change, f))
diff --git a/zuul/model.py b/zuul/model.py
index 30bcb9b..ee1fede 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1924,8 +1924,9 @@
class RefFilter(BaseFilter):
"""Allows a Manager to only enqueue Changes that meet certain criteria."""
- def __init__(self):
+ def __init__(self, connection_name):
super(RefFilter, self).__init__()
+ self.connection_name = connection_name
def matches(self, change):
return True