Cleanup old rebase-merge dirs on repo reset

When using the rebase merge-mode a failed "merge" will leave the repo in
a state that Zuul so far could not recover from. The rebase will create
a `.git/rebase-merge` directory which is not removed when the rebase
fails.

To fix this we will abort the rebase when it fails and also remove any
existing `.git/rebase-merge` and `.git/rebase-apply` directories when
resetting the repository.

DEBUG zuul.Merger: [e: ...] Unable to merge {'branch': 'master', 'buildset_uuid': 'f7be4215f37049b4ba0236892a5d8197', 'connection': 'github', 'merge_mode': 5, 'newrev': None, 'number': 71, 'oldrev': None, 'patchset': 'e81d0b248565db290b30d9a638095947b699c76d', 'project': 'org/project', 'ref': 'refs/pull/71/head'}
Traceback (most recent call last):
  File "/opt/zuul/lib/python3.10/site-packages/zuul/merger/merger.py", line 1099, in _mergeChange
    commit = repo.rebaseMerge(
  File "/opt/zuul/lib/python3.10/site-packages/zuul/merger/merger.py", line 626, in rebaseMerge
    repo.git.rebase(*args)
  File "/opt/zuul/lib/python3.10/site-packages/git/cmd.py", line 542, in <lambda>
    return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
  File "/opt/zuul/lib/python3.10/site-packages/git/cmd.py", line 1005, in _call_process
    return self.execute(call, **exec_kwargs)
  File "/opt/zuul/lib/python3.10/site-packages/git/cmd.py", line 822, in execute
    raise GitCommandError(command, status, stderr_value, stdout_value)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git rebase 39fead1852ef01a716a1c6470cee9e4197ff5587
  stderr: 'fatal: It seems that there is already a rebase-merge directory, and
I wonder if you are in the middle of another rebase.  If that is the
case, please try
    git rebase (--continue | --abort | --skip)
If that is not the case, please
    rm -fr ".git/rebase-merge"
and run me again.  I am stopping in case you still have something
valuable there.

Change-Id: I8518cc5e4b3f7bbfc2c2283a2b946dee504991dd
This commit is contained in:
Simon Westphahl 2023-02-07 12:50:24 +01:00
parent 1ae2d929a3
commit ac534577c3
No known key found for this signature in database
2 changed files with 85 additions and 1 deletions

View File

@ -163,6 +163,64 @@ class TestMergerRepo(ZuulTestCase):
work_repo.reset() work_repo.reset()
work_repo.checkout("foobar") work_repo.checkout("foobar")
def test_rebase_merge_conflict_abort(self):
"""Test that a failed rebase is properly aborted and related
directories are cleaned up."""
parent_path = os.path.join(self.upstream_root, 'org/project1')
parent_repo = git.Repo(parent_path)
parent_repo.create_head("feature")
files = {"test.txt": "master"}
self.create_commit("org/project1", files=files, head="master",
message="Add master file")
files = {"test.txt": "feature"}
self.create_commit("org/project1", files=files, head="feature",
message="Add feature file")
work_repo = Repo(parent_path, self.workspace_root,
"none@example.org", "User Name", "0", "0")
item = {"ref": "refs/heads/feature"}
# We expect the rebase to fail because of a conflict, but the
# rebase will be aborted.
with testtools.ExpectedException(git.exc.GitCommandError):
work_repo.rebaseMerge(item, "master")
# Assert that the failed rebase doesn't leave any temporary
# directories behind.
self.assertFalse(
os.path.exists(f"{work_repo.local_path}/.git/rebase-merge"))
self.assertFalse(
os.path.exists(f"{work_repo.local_path}/.git/rebase-apply"))
def test_rebase_merge_conflict_reset_cleanup(self):
"""Test temporary directories of a failed rebase merge are
removed on repo reset."""
parent_path = os.path.join(self.upstream_root, 'org/project1')
parent_repo = git.Repo(parent_path)
parent_repo.create_head("feature")
files = {"master.txt": "master"}
self.create_commit("org/project1", files=files, head="master",
message="Add master file")
files = {"feature.txt": "feature"}
self.create_commit("org/project1", files=files, head="feature",
message="Add feature file")
work_repo = Repo(parent_path, self.workspace_root,
"none@example.org", "User Name", "0", "0")
# Simulate leftovers from a failed rebase
os.mkdir(f"{work_repo.local_path}/.git/rebase-merge")
os.mkdir(f"{work_repo.local_path}/.git/rebase-apply")
# Resetting the repo should clean up any leaked directories
work_repo.reset()
item = {"ref": "refs/heads/feature"}
work_repo.rebaseMerge(item, "master")
def test_set_refs(self): def test_set_refs(self):
parent_path = os.path.join(self.upstream_root, 'org/project1') parent_path = os.path.join(self.upstream_root, 'org/project1')
remote_sha = self.create_commit('org/project1') remote_sha = self.create_commit('org/project1')

View File

@ -332,6 +332,26 @@ class Repo(object):
messages.append("Cleaning empty ref dir %s" % root) messages.append("Cleaning empty ref dir %s" % root)
os.rmdir(root) os.rmdir(root)
@staticmethod
def _cleanup_leaked_rebase_dirs(local_path, log, messages):
for rebase_dir in [".git/rebase-merge", ".git/rebase-apply"]:
leaked_dir = os.path.join(local_path, rebase_dir)
if not os.path.exists(leaked_dir):
continue
if log:
log.debug("Cleaning leaked %s dir", leaked_dir)
else:
messages.append(
f"Cleaning leaked {leaked_dir} dir")
try:
shutil.rmtree(leaked_dir)
except Exception as exc:
msg = f"Failed to remove leaked {leaked_dir} dir:"
if log:
log.exception(msg)
else:
messages.append(f"{msg}\n{exc}")
@staticmethod @staticmethod
def refNameToZuulRef(ref_name: str) -> str: def refNameToZuulRef(ref_name: str) -> str:
return "refs/zuul/{}".format( return "refs/zuul/{}".format(
@ -384,6 +404,8 @@ class Repo(object):
messages.append("Delete stale Zuul ref {}".format(ref)) messages.append("Delete stale Zuul ref {}".format(ref))
Repo._deleteRef(ref.path, repo) Repo._deleteRef(ref.path, repo)
Repo._cleanup_leaked_rebase_dirs(local_path, log, messages)
# Note: Before git 2.13 deleting a a ref foo/bar leaves an empty # Note: Before git 2.13 deleting a a ref foo/bar leaves an empty
# directory foo behind that will block creating the reference foo # directory foo behind that will block creating the reference foo
# in the future. As a workaround we must clean up empty directories # in the future. As a workaround we must clean up empty directories
@ -615,7 +637,11 @@ class Repo(object):
self.fetch(ref, zuul_event_id=zuul_event_id) self.fetch(ref, zuul_event_id=zuul_event_id)
log.debug("Rebasing %s with args %s", ref, args) log.debug("Rebasing %s with args %s", ref, args)
repo.git.checkout('FETCH_HEAD') repo.git.checkout('FETCH_HEAD')
repo.git.rebase(*args) try:
repo.git.rebase(*args)
except Exception:
repo.git.rebase(abort=True)
raise
return repo.head.commit return repo.head.commit
def fetch(self, ref, zuul_event_id=None): def fetch(self, ref, zuul_event_id=None):