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

2 files changed, 85 insertions, 1 deletions

diff --git a/tests/unit/test_merger_repo.py b/tests/unit/test_merger_repo.py
index 7806db347..f907cb8b4 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 1e2de27b8..e4688a1b7 100644
--- a/zuul/merger/merger.py
+++ b/zuul/merger/merger.py
@@ -333,6 +333,26 @@ class Repo(object):
                 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(
             hashlib.sha1(ref_name.encode("utf-8")).hexdigest()
@@ -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):