diff --git a/tests/unit/test_merger_repo.py b/tests/unit/test_merger_repo.py index 182dea1201..fd78726ab3 100644 --- a/tests/unit/test_merger_repo.py +++ b/tests/unit/test_merger_repo.py @@ -960,10 +960,6 @@ class TestMerger(ZuulTestCase): self.waitUntilSettled() self.assertEqual(A.data['status'], 'MERGED') - # Stop the merger so we can modify the git repo - self.merge_server.stop() - self.merge_server.join() - # Add an index.lock file fpath = os.path.join(self.merger_src_root, 'review.example.com', 'org', 'org%2Fproject1', '.git', 'index.lock') @@ -971,9 +967,6 @@ class TestMerger(ZuulTestCase): pass self.assertTrue(os.path.exists(fpath)) - # Start a new merger and check that we can still merge things - self._startMerger() - # This will fail if git can't modify the repo due to a stale lock file. B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B') B.addApproval('Code-Review', 2) diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py index 09968eac6c..d400c253bb 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -546,16 +546,29 @@ class Repo(object): log.debug("Repo is already at %s" % ref) else: log.debug("Checking out %s" % ref) - # Perform a hard reset to the correct ref before checking out so - # that we clean up anything that might be left over from a merge - # while still only preparing the working copy once. - repo.head.reference = ref - repo.head.reset(working_tree=True) - repo.git.clean('-x', '-f', '-d') - repo.git.checkout(ref) - + try: + self._checkout(repo, ref) + except Exception: + lock_path = f"{self.local_path}/.git/index.lock" + if os.path.isfile(lock_path): + log.warning("Deleting stale index.lock file: %s", + lock_path) + os.unlink(lock_path) + # Retry the checkout + self._checkout(repo, ref) + else: + raise return repo.head.commit + def _checkout(self, repo, ref): + # Perform a hard reset to the correct ref before checking out so + # that we clean up anything that might be left over from a merge + # while still only preparing the working copy once. + repo.head.reference = ref + repo.head.reset(working_tree=True) + repo.git.clean('-x', '-f', '-d') + repo.git.checkout(ref) + def cherryPick(self, ref, zuul_event_id=None): log = get_annotated_logger(self.log, zuul_event_id) repo = self.createRepoObject(zuul_event_id) diff --git a/zuul/merger/server.py b/zuul/merger/server.py index a55cec3baa..b843c8f3e4 100644 --- a/zuul/merger/server.py +++ b/zuul/merger/server.py @@ -15,7 +15,6 @@ import json import logging -import os import socket import sys import threading @@ -156,24 +155,6 @@ class BaseMergeServer(metaclass=ABCMeta): def start(self): self.log.debug('Starting merger') - self.log.debug('Cleaning any stale git index.lock files') - for (dirpath, dirnames, filenames) in os.walk(self.merge_root): - if '.git' in dirnames: - # Only recurse into .git dirs - dirnames.clear() - dirnames.append('.git') - elif dirpath.endswith('/.git'): - # Recurse no further - dirnames.clear() - if 'index.lock' in filenames: - fp = os.path.join(dirpath, 'index.lock') - try: - os.unlink(fp) - self.log.debug('Removed stale git lock: %s' % fp) - except Exception: - self.log.exception( - 'Unable to remove stale git lock: ' - '%s this may result in failed merges' % fp) self._merger_running = True self.merger_thread.start()