Browse Source

Clean up stale git index.lock files on merger startup

We've noticed that if zuul executors (and presumably mergers) don't shut
down gracefully that they may leak git index.lock files in the .git dirs
of the merger repos. Since these repos should be dedicated to zuul's use
without outside interference we can reasonably safely remove any present
index.lock files when starting zuul mergers (and executors).

This implementation does an os.walk under the merger repos root looking
for .git dirs and once it has found them checks for any index.lock
files. This happens before starting the gearman worker which should
avoid any races with these resources.

Change-Id: Ie043453bcdf4500a3718da6f705c882431acafdf
changes/70/751370/3
Clark Boylan 5 months ago
parent
commit
0f7982fee0
2 changed files with 56 additions and 0 deletions
  1. +37
    -0
      tests/unit/test_merger_repo.py
  2. +19
    -0
      zuul/merger/server.py

+ 37
- 0
tests/unit/test_merger_repo.py View File

@ -731,3 +731,40 @@ class TestMerger(ZuulTestCase):
ref_map[foo_zuul_ref].commit.hexsha,
merge_state[("gerrit", "org/project", "foo")]
)
def test_stale_index_lock_cleanup(self):
# Stop the running executor's merger. We needed it running to merge
# things during test boostrapping but now it is just in the way.
self.executor_server.merger_gearworker.stop()
self.executor_server.merger_gearworker.join()
# Start the merger and do a merge to populate the repo on disk
self._startMerger()
A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
A.addApproval('Code-Review', 2)
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
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', 'project1', '.git', 'index.lock')
with open(fpath, 'w'):
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)
self.fake_gerrit.addEvent(B.addApproval('Approved', 1))
self.waitUntilSettled()
self.assertEqual(B.data['status'], 'MERGED')
self.assertFalse(os.path.exists(fpath))

+ 19
- 0
zuul/merger/server.py View File

@ -14,6 +14,7 @@
import json
import logging
import os
import threading
from abc import ABCMeta
@ -103,6 +104,24 @@ class BaseMergeServer(metaclass=ABCMeta):
def start(self):
self.log.debug('Starting merger worker')
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_gearworker.start()
def stop(self):


Loading…
Cancel
Save