From 71a47ff90c775a11b7080815c5cd880c2f903354 Mon Sep 17 00:00:00 2001 From: Jesse Keating Date: Tue, 6 Jun 2017 11:36:43 -0700 Subject: [PATCH] Extend in-repo config update support to github The code that would look for in-repo config changes was gerrit specific. Turn the gerrit specific bit into a generic that drivers can implement. Implement the generic in the github driver, which required accounting for files that are part of a push event (where code is landing in the repo we care about). Also remove an unnecessary fake pull request method of getPushEvent, as there was already one in the fake github class. Move the real updatesConfig function from the Change object to the Ref object (since we can get a change from a ref) and move the files attribute as well. Introduce a test for github to verify that the tenant is reconfigured. This required introducing a new Scheduler object attribute to track when each tenant is reconfigured. The existing reconfiguration time tracker was generic, and not updated via dyanmic updates. Change-Id: Ibf59f91fa3701c15d93d859920fe3070478fe457 Story: 2000774 Task: 4664 --- tests/base.py | 27 ++++++++++---------------- tests/unit/test_github_driver.py | 15 ++++++++++++++ tests/unit/test_push_reqs.py | 5 ++++- zuul/driver/gerrit/gerritconnection.py | 8 ++++++++ zuul/driver/github/githubconnection.py | 10 ++++++++++ zuul/model.py | 11 +++++------ zuul/scheduler.py | 4 +++- 7 files changed, 55 insertions(+), 25 deletions(-) diff --git a/tests/base.py b/tests/base.py index 65ded500e8..b78495deb5 100755 --- a/tests/base.py +++ b/tests/base.py @@ -592,21 +592,6 @@ class FakeGithubPullRequest(object): 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() @@ -896,7 +881,8 @@ class FakeGithubConnection(githubconnection.GithubConnection): self.pull_requests.append(pull_request) return pull_request - def getPushEvent(self, project, ref, old_rev=None, new_rev=None): + def getPushEvent(self, project, ref, old_rev=None, new_rev=None, + added_files=[], removed_files=[], modified_files=[]): if not old_rev: old_rev = '00000000000000000000000000000000' if not new_rev: @@ -908,7 +894,14 @@ class FakeGithubConnection(githubconnection.GithubConnection): 'after': new_rev, 'repository': { 'full_name': project - } + }, + 'commits': [ + { + 'added': added_files, + 'removed': removed_files, + 'modified': modified_files + } + ] } return (name, data) diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 227d6590f3..24740950b1 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -510,3 +510,18 @@ class TestGithubDriver(ZuulTestCase): self.assertNotIn('merge', A.labels) self.assertNotIn('merge', B.labels) self.assertNotIn('merge', C.labels) + + @simple_layout('layouts/basic-github.yaml', driver='github') + def test_push_event_reconfigure(self): + pevent = self.fake_github.getPushEvent(project='common-config', + ref='refs/heads/master', + modified_files=['zuul.yaml']) + + # record previous tenant reconfiguration time, which may not be set + old = self.sched.tenant_last_reconfigured.get('tenant-one', 0) + time.sleep(1) + self.fake_github.emitEvent(pevent) + self.waitUntilSettled() + new = self.sched.tenant_last_reconfigured.get('tenant-one', 0) + # New timestamp should be greater than the old timestamp + self.assertLess(old, new) diff --git a/tests/unit/test_push_reqs.py b/tests/unit/test_push_reqs.py index 657d9b8bf8..d3a1febe8c 100644 --- a/tests/unit/test_push_reqs.py +++ b/tests/unit/test_push_reqs.py @@ -28,7 +28,10 @@ class TestPushRequirements(ZuulTestCase): # 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)) + pevent = self.fake_github.getPushEvent(project='org/project1', + ref='refs/heads/master', + old_rev=old_sha) + self.fake_github.emitEvent(pevent) self.waitUntilSettled() diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index 06962e51d7..85a16fb6bf 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -76,6 +76,14 @@ class GerritEventConnector(threading.Thread): time.sleep(max((ts + self.delay) - now, 0.0)) event = GerritTriggerEvent() event.type = data.get('type') + # This catches when a change is merged, as it could potentially + # have merged layout info which will need to be read in. + # Ideally this would be done with a refupdate event so as to catch + # directly pushed things as well as full changes being merged. + # But we do not yet get files changed data for pure refupdate events. + # TODO(jlk): handle refupdated events instead of just changes + if event.type == 'change-merged': + event.branch_updated = True event.trigger_name = 'gerrit' change = data.get('change') event.project_hostname = self.connection.canonical_hostname diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 02c795ece1..d0557b4ea5 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -129,10 +129,12 @@ class GithubWebhookListener(): event.trigger_name = 'github' event.project_name = base_repo.get('full_name') event.type = 'push' + event.branch_updated = True event.ref = body.get('ref') event.oldrev = body.get('before') event.newrev = body.get('after') + event.commits = body.get('commits') ref_parts = event.ref.split('/') # ie, ['refs', 'heads', 'master'] @@ -490,6 +492,7 @@ class GithubConnection(BaseConnection): change.newrev = event.newrev change.url = self.getGitwebUrl(project, sha=event.newrev) change.source_event = event + change.files = self.getPushedFileNames(event) else: change = Ref(project) return change @@ -728,6 +731,13 @@ class GithubConnection(BaseConnection): pr = self.getPull(project, number) return pr.get('head').get('sha') == sha + def getPushedFileNames(self, event): + files = set() + for c in event.commits: + for f in c.get('added') + c.get('modified') + c.get('removed'): + files.add(f) + return list(files) + def _ghTimestampToDate(self, timestamp): return time.strptime(timestamp, '%Y-%m-%dT%H:%M:%SZ') diff --git a/zuul/model.py b/zuul/model.py index 12ddbda58d..4a3dba6d79 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1767,6 +1767,8 @@ class Ref(object): self.oldrev = None self.newrev = None + self.files = [] + def getBasePath(self): base_path = '' if hasattr(self, 'ref'): @@ -1809,6 +1811,8 @@ class Ref(object): return set() def updatesConfig(self): + if 'zuul.yaml' in self.files or '.zuul.yaml' in self.files: + return True return False def getSafeAttributes(self): @@ -1828,7 +1832,6 @@ class Change(Ref): self.patchset = None self.refspec = None - self.files = [] self.needs_changes = [] self.needed_by_changes = [] self.is_current_patchset = True @@ -1876,11 +1879,6 @@ class Change(Ref): related.update(c.getRelatedChanges()) return related - def updatesConfig(self): - if 'zuul.yaml' in self.files or '.zuul.yaml' in self.files: - return True - return False - def getSafeAttributes(self): return Attributes(project=self.project, number=self.number, @@ -1894,6 +1892,7 @@ class TriggerEvent(object): self.data = None # common self.type = None + self.branch_updated = False # For management events (eg: enqueue / promote) self.tenant_name = None self.project_hostname = None diff --git a/zuul/scheduler.py b/zuul/scheduler.py index 40d5eb7511..4dc2c97151 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -228,6 +228,7 @@ class Scheduler(threading.Thread): self.zuul_version = zuul_version.version_info.release_string() self.last_reconfigured = None + self.tenant_last_reconfigured = {} def stop(self): self._stopped = True @@ -595,6 +596,7 @@ class Scheduler(threading.Thread): trigger.postConfig(pipeline) for reporter in pipeline.actions: reporter.postConfig() + self.tenant_last_reconfigured[tenant.name] = int(time.time()) if self.statsd: try: for pipeline in tenant.layout.pipelines.values(): @@ -752,7 +754,7 @@ class Scheduler(threading.Thread): "source %s", e.change, project.source) continue - if (event.type == 'change-merged' and + if (event.branch_updated and hasattr(change, 'files') and change.updatesConfig()): # The change that just landed updates the config.