summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZuul <zuul@review.openstack.org>2018-12-29 14:49:32 +0000
committerGerrit Code Review <review@openstack.org>2018-12-29 14:49:32 +0000
commit5ae6d549a5d56e2813b5663751b6b2c883486d3c (patch)
tree123ee6974d1e5b7fb5289a6d52e51f10de29ab41
parentdcfeb3a42bc31d4aea15d3032fc162f97486891d (diff)
parent8bfc0cd409e5d1571ca391523b3895b8bfe884ed (diff)
Merge "Delay Github fileschanges workaround to pipeline processing"
-rw-r--r--tests/base.py1
-rw-r--r--tests/unit/test_github_driver.py36
-rw-r--r--zuul/driver/github/githubconnection.py28
-rw-r--r--zuul/manager/__init__.py37
-rw-r--r--zuul/merger/client.py16
-rw-r--r--zuul/model.py9
-rw-r--r--zuul/scheduler.py31
7 files changed, 126 insertions, 32 deletions
diff --git a/tests/base.py b/tests/base.py
index 97af80d..6cc89fc 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -1001,7 +1001,6 @@ class FakeGithubPullRequest(object):
1001 msg = self.subject + '-' + str(self.number_of_commits) 1001 msg = self.subject + '-' + str(self.number_of_commits)
1002 for fn, content in self.files.items(): 1002 for fn, content in self.files.items():
1003 fn = os.path.join(repo.working_dir, fn) 1003 fn = os.path.join(repo.working_dir, fn)
1004 f = open(fn, 'w')
1005 with open(fn, 'w') as f: 1004 with open(fn, 'w') as f:
1006 f.write(content) 1005 f.write(content)
1007 repo.index.add([fn]) 1006 repo.index.add([fn])
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index 28b7b4a..7b70486 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -119,6 +119,42 @@ class TestGithubDriver(ZuulTestCase):
119 self.waitUntilSettled() 119 self.waitUntilSettled()
120 self.assertEqual(1, len(self.history)) 120 self.assertEqual(1, len(self.history))
121 121
122 @simple_layout('layouts/files-github.yaml', driver='github')
123 def test_pull_changed_files_length_mismatch_reenqueue(self):
124 # Hold jobs so we can trigger a reconfiguration while the item is in
125 # the pipeline
126 self.executor_server.hold_jobs_in_build = True
127
128 files = {'{:03d}.txt'.format(n): 'test' for n in range(300)}
129 # File 301 which is not included in the list of files of the PR,
130 # since Github only returns max. 300 files in alphabetical order
131 files["foobar-requires"] = "test"
132 A = self.fake_github.openFakePullRequest(
133 'org/project', 'master', 'A', files=files)
134
135 self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
136 self.waitUntilSettled()
137
138 # Comment on the pull request to trigger updateChange
139 self.fake_github.emitEvent(A.getCommentAddedEvent('casual comment'))
140 self.waitUntilSettled()
141
142 # Trigger reconfig to enforce a reenqueue of the item
143 self.sched.reconfigure(self.config)
144 self.waitUntilSettled()
145
146 # Now we can release all jobs
147 self.executor_server.hold_jobs_in_build = True
148 self.executor_server.release()
149 self.waitUntilSettled()
150
151 # There must be exactly one successful job in the history. If there is
152 # an aborted job in the history the reenqueue failed.
153 self.assertHistory([
154 dict(name='project-test1', result='SUCCESS',
155 changes="%s,%s" % (A.number, A.head_sha)),
156 ])
157
122 @simple_layout('layouts/basic-github.yaml', driver='github') 158 @simple_layout('layouts/basic-github.yaml', driver='github')
123 def test_pull_github_files_error(self): 159 def test_pull_github_files_error(self):
124 A = self.fake_github.openFakePullRequest( 160 A = self.fake_github.openFakePullRequest(
diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py
index 95eb7f5..69085d1 100644
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -904,24 +904,17 @@ class GithubConnection(BaseConnection):
904 904
905 return changes 905 return changes
906 906
907 def getFilesChanges(self, project_name, head, base):
908 job = self.sched.merger.getFilesChanges(self.connection_name,
909 project_name,
910 head, base)
911 self.log.debug("Waiting for fileschanges job %s", job)
912 job.wait()
913 if not job.updated:
914 raise Exception("Fileschanges job {} failed".format(job))
915 self.log.debug("Fileschanges job %s got changes on files %s",
916 job, job.files)
917 return job.files
918
919 def _updateChange(self, change): 907 def _updateChange(self, change):
920 self.log.info("Updating %s" % (change,)) 908 self.log.info("Updating %s" % (change,))
921 change.pr = self.getPull(change.project.name, change.number) 909 change.pr = self.getPull(change.project.name, change.number)
922 change.ref = "refs/pull/%s/head" % change.number 910 change.ref = "refs/pull/%s/head" % change.number
923 change.branch = change.pr.get('base').get('ref') 911 change.branch = change.pr.get('base').get('ref')
924 change.files = change.pr.get('files') 912
913 # Don't overwrite the files list. The change object is bound to a
914 # specific revision and thus the changed files won't change. This is
915 # important if we got the files later because of the 300 files limit.
916 if not change.files:
917 change.files = change.pr.get('files')
925 # Github's pull requests files API only returns at max 918 # Github's pull requests files API only returns at max
926 # the first 300 changed files of a PR in alphabetical order. 919 # the first 300 changed files of a PR in alphabetical order.
927 # https://developer.github.com/v3/pulls/#list-pull-requests-files 920 # https://developer.github.com/v3/pulls/#list-pull-requests-files
@@ -929,10 +922,11 @@ class GithubConnection(BaseConnection):
929 self.log.warning("Got only %s files but PR has %s files.", 922 self.log.warning("Got only %s files but PR has %s files.",
930 len(change.files), 923 len(change.files),
931 change.pr.get('changed_files', 0)) 924 change.pr.get('changed_files', 0))
932 change.files = self.getFilesChanges( 925 # In this case explicitly set change.files to None to signalize
933 change.project.name, 926 # that we need to ask the mergers later in pipeline processing.
934 change.ref, 927 # We cannot query the files here using the mergers because this
935 change.branch) 928 # can slow down the github event queue considerably.
929 change.files = None
936 change.title = change.pr.get('title') 930 change.title = change.pr.get('title')
937 change.open = change.pr.get('state') == 'open' 931 change.open = change.pr.get('state') == 'open'
938 change.is_merged = change.pr.get('merged') 932 change.is_merged = change.pr.get('merged')
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index d29af84..62b42a5 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -578,8 +578,6 @@ class PipelineManager(object):
578 return self._loadDynamicLayout(item) 578 return self._loadDynamicLayout(item)
579 579
580 def scheduleMerge(self, item, files=None, dirs=None): 580 def scheduleMerge(self, item, files=None, dirs=None):
581 build_set = item.current_build_set
582
583 self.log.debug("Scheduling merge for item %s (files: %s, dirs: %s)" % 581 self.log.debug("Scheduling merge for item %s (files: %s, dirs: %s)" %
584 (item, files, dirs)) 582 (item, files, dirs))
585 build_set = item.current_build_set 583 build_set = item.current_build_set
@@ -594,23 +592,38 @@ class PipelineManager(object):
594 precedence=self.pipeline.precedence) 592 precedence=self.pipeline.precedence)
595 return False 593 return False
596 594
595 def scheduleFilesChanges(self, item):
596 self.log.debug("Scheduling fileschanged for item %s", item)
597 build_set = item.current_build_set
598 build_set.files_state = build_set.PENDING
599
600 self.sched.merger.getFilesChanges(
601 item.change.project.connection_name, item.change.project.name,
602 item.change.ref, item.change.branch, build_set=build_set)
603 return False
604
597 def prepareItem(self, item): 605 def prepareItem(self, item):
598 # This runs on every iteration of _processOneItem 606 # This runs on every iteration of _processOneItem
599 # Returns True if the item is ready, false otherwise 607 # Returns True if the item is ready, false otherwise
608 ready = True
600 build_set = item.current_build_set 609 build_set = item.current_build_set
601 if not build_set.ref: 610 if not build_set.ref:
602 build_set.setConfiguration() 611 build_set.setConfiguration()
603 if build_set.merge_state == build_set.NEW: 612 if build_set.merge_state == build_set.NEW:
604 return self.scheduleMerge(item, 613 ready = self.scheduleMerge(item,
605 files=['zuul.yaml', '.zuul.yaml'], 614 files=['zuul.yaml', '.zuul.yaml'],
606 dirs=['zuul.d', '.zuul.d']) 615 dirs=['zuul.d', '.zuul.d'])
616 if build_set.files_state == build_set.NEW:
617 ready = self.scheduleFilesChanges(item)
618 if build_set.files_state == build_set.PENDING:
619 ready = False
607 if build_set.merge_state == build_set.PENDING: 620 if build_set.merge_state == build_set.PENDING:
608 return False 621 ready = False
609 if build_set.unable_to_merge: 622 if build_set.unable_to_merge:
610 return False 623 ready = False
611 if build_set.config_errors: 624 if build_set.config_errors:
612 return False 625 ready = False
613 return True 626 return ready
614 627
615 def prepareJobs(self, item): 628 def prepareJobs(self, item):
616 # This only runs once the item is in the pipeline's action window 629 # This only runs once the item is in the pipeline's action window
@@ -820,6 +833,12 @@ class PipelineManager(object):
820 self._resumeBuilds(build.build_set) 833 self._resumeBuilds(build.build_set)
821 return True 834 return True
822 835
836 def onFilesChangesCompleted(self, event):
837 build_set = event.build_set
838 item = build_set.item
839 item.change.files = event.files
840 build_set.files_state = build_set.COMPLETE
841
823 def onMergeCompleted(self, event): 842 def onMergeCompleted(self, event):
824 build_set = event.build_set 843 build_set = event.build_set
825 item = build_set.item 844 item = build_set.item
diff --git a/zuul/merger/client.py b/zuul/merger/client.py
index c89a6fb..51c4afd 100644
--- a/zuul/merger/client.py
+++ b/zuul/merger/client.py
@@ -132,12 +132,14 @@ class MergeClient(object):
132 return job 132 return job
133 133
134 def getFilesChanges(self, connection_name, project_name, branch, 134 def getFilesChanges(self, connection_name, project_name, branch,
135 tosha=None, precedence=zuul.model.PRECEDENCE_HIGH): 135 tosha=None, precedence=zuul.model.PRECEDENCE_HIGH,
136 build_set=None):
136 data = dict(connection=connection_name, 137 data = dict(connection=connection_name,
137 project=project_name, 138 project=project_name,
138 branch=branch, 139 branch=branch,
139 tosha=tosha) 140 tosha=tosha)
140 job = self.submitJob('merger:fileschanges', data, None, precedence) 141 job = self.submitJob('merger:fileschanges', data, build_set,
142 precedence)
141 return job 143 return job
142 144
143 def onBuildCompleted(self, job): 145 def onBuildCompleted(self, job):
@@ -153,9 +155,13 @@ class MergeClient(object):
153 (job, merged, job.updated, commit)) 155 (job, merged, job.updated, commit))
154 job.setComplete() 156 job.setComplete()
155 if job.build_set: 157 if job.build_set:
156 self.sched.onMergeCompleted(job.build_set, 158 if job.name == 'merger:fileschanges':
157 merged, job.updated, commit, files, 159 self.sched.onFilesChangesCompleted(job.build_set, files)
158 repo_state) 160 else:
161 self.sched.onMergeCompleted(job.build_set,
162 merged, job.updated, commit, files,
163 repo_state)
164
159 # The test suite expects the job to be removed from the 165 # The test suite expects the job to be removed from the
160 # internal account after the wake flag is set. 166 # internal account after the wake flag is set.
161 self.jobs.remove(job) 167 self.jobs.remove(job)
diff --git a/zuul/model.py b/zuul/model.py
index d6b1c6a..3b972b9 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1784,6 +1784,10 @@ class BuildSet(object):
1784 self.files = RepoFiles() 1784 self.files = RepoFiles()
1785 self.repo_state = {} 1785 self.repo_state = {}
1786 self.tries = {} 1786 self.tries = {}
1787 if item.change.files is not None:
1788 self.files_state = self.COMPLETE
1789 else:
1790 self.files_state = self.NEW
1787 1791
1788 @property 1792 @property
1789 def ref(self): 1793 def ref(self):
@@ -2584,6 +2588,11 @@ class Ref(object):
2584 return set() 2588 return set()
2585 2589
2586 def updatesConfig(self): 2590 def updatesConfig(self):
2591 if self.files is None:
2592 # If self.files is None we don't know if this change updates the
2593 # config so assume it does as this is a safe default if we don't
2594 # know.
2595 return True
2587 if 'zuul.yaml' in self.files or '.zuul.yaml' in self.files or \ 2596 if 'zuul.yaml' in self.files or '.zuul.yaml' in self.files or \
2588 [True for fn in self.files if fn.startswith("zuul.d/") or 2597 [True for fn in self.files if fn.startswith("zuul.d/") or
2589 fn.startswith(".zuul.d/")]: 2598 fn.startswith(".zuul.d/")]:
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 9cbc8a5..fdd97b3 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -215,6 +215,18 @@ class MergeCompletedEvent(ResultEvent):
215 self.repo_state = repo_state 215 self.repo_state = repo_state
216 216
217 217
218class FilesChangesCompletedEvent(ResultEvent):
219 """A remote fileschanges operation has completed
220
221 :arg BuildSet build_set: The build_set which is ready.
222 :arg list files: List of files changed.
223 """
224
225 def __init__(self, build_set, files):
226 self.build_set = build_set
227 self.files = files
228
229
218class NodesProvisionedEvent(ResultEvent): 230class NodesProvisionedEvent(ResultEvent):
219 """Nodes have been provisioned for a build_set 231 """Nodes have been provisioned for a build_set
220 232
@@ -475,6 +487,11 @@ class Scheduler(threading.Thread):
475 self.result_event_queue.put(event) 487 self.result_event_queue.put(event)
476 self.wake_event.set() 488 self.wake_event.set()
477 489
490 def onFilesChangesCompleted(self, build_set, files):
491 event = FilesChangesCompletedEvent(build_set, files)
492 self.result_event_queue.put(event)
493 self.wake_event.set()
494
478 def onNodesProvisioned(self, req): 495 def onNodesProvisioned(self, req):
479 event = NodesProvisionedEvent(req) 496 event = NodesProvisionedEvent(req)
480 self.result_event_queue.put(event) 497 self.result_event_queue.put(event)
@@ -1107,6 +1124,8 @@ class Scheduler(threading.Thread):
1107 self._doBuildCompletedEvent(event) 1124 self._doBuildCompletedEvent(event)
1108 elif isinstance(event, MergeCompletedEvent): 1125 elif isinstance(event, MergeCompletedEvent):
1109 self._doMergeCompletedEvent(event) 1126 self._doMergeCompletedEvent(event)
1127 elif isinstance(event, FilesChangesCompletedEvent):
1128 self._doFilesChangesCompletedEvent(event)
1110 elif isinstance(event, NodesProvisionedEvent): 1129 elif isinstance(event, NodesProvisionedEvent):
1111 self._doNodesProvisionedEvent(event) 1130 self._doNodesProvisionedEvent(event)
1112 else: 1131 else:
@@ -1264,6 +1283,18 @@ class Scheduler(threading.Thread):
1264 return 1283 return
1265 pipeline.manager.onMergeCompleted(event) 1284 pipeline.manager.onMergeCompleted(event)
1266 1285
1286 def _doFilesChangesCompletedEvent(self, event):
1287 build_set = event.build_set
1288 if build_set is not build_set.item.current_build_set:
1289 self.log.warning("Build set %s is not current", build_set)
1290 return
1291 pipeline = build_set.item.pipeline
1292 if not pipeline:
1293 self.log.warning("Build set %s is not associated with a pipeline",
1294 build_set)
1295 return
1296 pipeline.manager.onFilesChangesCompleted(event)
1297
1267 def _doNodesProvisionedEvent(self, event): 1298 def _doNodesProvisionedEvent(self, event):
1268 request = event.request 1299 request = event.request
1269 request_id = event.request_id 1300 request_id = event.request_id