Wait for merger to complete before item is ready

In zuul v2, prepareRef waited until the merger had completed its
work before telling the pipeline manager that the item was ready.
This was important as it ensured there was a zuul ref on a merger
for jobs to be able to fetch from.  We dropped that in v3 because
the jobs would build their own repo states on the executor, however,
we're about to put something similar back in that we're going have
the merger return the complete repo state so that the executors can
later use that to reconstruct the work the merger performed.

Because prepareItem (essentially the v3 version of prepareRef) did
not wait for the merger result before declaring the item ready, we
can end up launching jobs before the merger completes.  This isn't
a big deal, yet, but it could still produce surprising results and
make for extra work.  This change restores the old behavior, as well
as makes a test less suceptible to race conditions (again, which
are more likely to be seen in future changes).

Similarly, we also lost setting ready to false in the case of a merge
conflict (a behavior which was present in v2).  This corrects that
as well to ensure that we don't execute jobs if there is a merge
conflict.

Because some tests were written with the assumption that only one
merger is running, and we are now running two (a dedicated merger and
one in the executor), they are more likely to fail due to race
conditions, especially now that we are waiting for mergers to complete
before executing jobs.  This change also removes the dedicated merger
from the test infrastructure so we rely only on the merger in the
executor.

Change-Id: I6d4da3d901d99290ef99b478ea9a4659058fe839
This commit is contained in:
James E. Blair 2017-05-05 10:13:45 -07:00
parent 2cf8d2ac6d
commit 8d144dc4f8
3 changed files with 7 additions and 6 deletions

View File

@ -1777,8 +1777,6 @@ class ZuulTestCase(BaseTestCase):
old_urlopen = urllib.request.urlopen
urllib.request.urlopen = URLOpenerFactory
self._startMerger()
self.executor_server = RecordingExecutorServer(
self.config, self.connections,
jobdir_root=self.test_root,
@ -2047,8 +2045,6 @@ class ZuulTestCase(BaseTestCase):
def shutdown(self):
self.log.debug("Shutting down after tests")
self.executor_client.stop()
self.merge_server.stop()
self.merge_server.join()
self.merge_client.stop()
self.executor_server.stop()
self.sched.stop()

View File

@ -1012,6 +1012,7 @@ class TestScheduler(ZuulTestCase):
self.fake_gerrit.addEvent(A.addApproval('approved', 1))
self.waitUntilSettled()
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
@ -1509,8 +1510,8 @@ class TestScheduler(ZuulTestCase):
tenant = self.sched.abide.tenants.get('tenant-one')
trusted, project = tenant.getProject('org/project')
url = self.fake_gerrit.getGitUrl(project)
self.merge_server.merger._addProject('review.example.com',
'org/project', url)
self.executor_server.merger._addProject('review.example.com',
'org/project', url)
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
A.addPatchset(large=True)
# TODOv3(jeblair): add hostname to upstream root

View File

@ -522,6 +522,10 @@ class PipelineManager(object):
build_set.setConfiguration()
if build_set.merge_state == build_set.NEW:
return self.scheduleMerge(item, ['zuul.yaml', '.zuul.yaml'])
if build_set.merge_state == build_set.PENDING:
return False
if build_set.unable_to_merge:
return False
if build_set.config_error:
return False
return True