From ac534577c3b001a6d70dd54d57b1e604b93a70f7 Mon Sep 17 00:00:00 2001 From: Simon Westphahl Date: Tue, 7 Feb 2023 12:50:24 +0100 Subject: [PATCH] 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 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 --- tests/unit/test_merger_repo.py | 58 ++++++++++++++++++++++++++++++++++ zuul/merger/merger.py | 28 +++++++++++++++- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_merger_repo.py b/tests/unit/test_merger_repo.py index 7806db347d..f907cb8b46 100644 --- a/tests/unit/test_merger_repo.py +++ b/tests/unit/test_merger_repo.py @@ -163,6 +163,64 @@ class TestMergerRepo(ZuulTestCase): work_repo.reset() 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): parent_path = os.path.join(self.upstream_root, 'org/project1') remote_sha = self.create_commit('org/project1') diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py index 1e2de27b8d..e4688a1b7c 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -332,6 +332,26 @@ class Repo(object): messages.append("Cleaning empty ref dir %s" % 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 def refNameToZuulRef(ref_name: str) -> str: return "refs/zuul/{}".format( @@ -384,6 +404,8 @@ class Repo(object): messages.append("Delete stale Zuul ref {}".format(ref)) 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 # directory foo behind that will block creating the reference foo # 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) log.debug("Rebasing %s with args %s", ref, args) 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 def fetch(self, ref, zuul_event_id=None):