refactor GitLab auto-merge handling: improve pipeline validation, ret…

test/ci_provider/gitlab/merge_request_test.py

@@ -1,6 +1,8 @@
 import unittest
 from unittest.mock import patch, MagicMock
 
+from gitlab.exceptions import GitlabMRClosedError
+
 from valhalla.ci_provider.gitlab.merge_request import GitLabValhallaMergeRequest
 from valhalla.common.get_config import MergeRequestConfig
 
@@ -199,10 +201,13 @@ def test_enable_auto_merge(self, mock_sleep, mock_repo, mock_warn, mock_info, mo
             mock_get_gitlab_client.return_value = mock_gitlab_client
 
             mock_repo.return_value.head.commit.hexsha = "new-sha"
-            stale_mr_obj = MagicMock(sha="old-sha", detailed_merge_status="mergeable")
-            checking_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="checking")
-            mock_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="mergeable")
-            mock_project.mergerequests.get.side_effect = [stale_mr_obj, checking_mr_obj, mock_mr_obj]
+            pipeline = {'id': 7, 'sha': "new-sha"}
+            stale_mr_obj = MagicMock(sha="old-sha", detailed_merge_status="mergeable", head_pipeline=pipeline)
+            no_pipeline_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="mergeable", head_pipeline=None)
+            checking_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="checking", head_pipeline=pipeline)
+            mock_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="mergeable", head_pipeline=pipeline)
+            mock_project.mergerequests.get.side_effect = [stale_mr_obj, no_pipeline_mr_obj, checking_mr_obj,
+                                                          mock_mr_obj]
 
             merge_request = GitLabValhallaMergeRequest()
             config = MergeRequestConfig(enabled=True, target_branch="main", title="T", description="D", reviewers=[],
@@ -213,8 +218,9 @@ def test_enable_auto_merge(self, mock_sleep, mock_repo, mock_warn, mock_info, mo
             hook.enable_auto_merge()
 
             # then:
-            self.assertEqual(mock_sleep.call_count, 2)
+            self.assertEqual(mock_sleep.call_count, 3)
             stale_mr_obj.merge.assert_not_called()
+            no_pipeline_mr_obj.merge.assert_not_called()
             checking_mr_obj.merge.assert_not_called()
             mock_mr_obj.merge.assert_called_once_with(sha="new-sha", merge_when_pipeline_succeeds=True)
             mock_info.assert_any_call("Auto-merge enabled, merge request will be merged when approvals and pipeline succeed")
@@ -237,7 +243,8 @@ def test_enable_auto_merge_exception(self, mock_sleep, mock_repo, mock_warn, moc
             mock_gitlab_client.projects.get.return_value = mock_project
             mock_get_gitlab_client.return_value = mock_gitlab_client
             mock_repo.return_value.head.commit.hexsha = "new-sha"
-            mock_project.mergerequests.get.return_value = MagicMock(sha="new-sha", detailed_merge_status="need_rebase")
+            mock_project.mergerequests.get.return_value = MagicMock(sha="new-sha", detailed_merge_status="need_rebase",
+                                                                         head_pipeline={'sha': "new-sha"})
             mock_project.mergerequests.get.return_value.merge.side_effect = Exception("405 Method Not Allowed")
 
             merge_request = GitLabValhallaMergeRequest()
@@ -252,6 +259,75 @@ def test_enable_auto_merge_exception(self, mock_sleep, mock_repo, mock_warn, moc
             mock_warn.assert_any_call("Could not enable auto-merge for merge request because: 405 Method Not Allowed "
                                       "(detailed_merge_status: need_rebase)")
 
+    @patch("valhalla.ci_provider.gitlab.merge_request.get_gitlab_client")
+    @patch("valhalla.ci_provider.gitlab.merge_request.get_project_id")
+    @patch("valhalla.ci_provider.gitlab.merge_request.resolve")
+    @patch("valhalla.ci_provider.gitlab.merge_request.info")
+    @patch("valhalla.ci_provider.gitlab.merge_request.warn")
+    @patch("valhalla.ci_provider.gitlab.merge_request.Repo")
+    @patch("valhalla.ci_provider.gitlab.merge_request.time.sleep")
+    def test_enable_auto_merge_when_pipeline_never_starts(self, mock_sleep, mock_repo, mock_warn, mock_info,
+                                                          mock_resolve, mock_get_project_id, mock_get_gitlab_client):
+        with patch.dict('os.environ', {'CI_COMMIT_BRANCH': 'feature-branch'}):
+            # given:
+            mock_get_project_id.return_value = "123"
+            mock_resolve.side_effect = lambda x: x
+            mock_gitlab_client = MagicMock()
+            mock_project = MagicMock()
+            mock_gitlab_client.projects.get.return_value = mock_project
+            mock_get_gitlab_client.return_value = mock_gitlab_client
+            mock_repo.return_value.head.commit.hexsha = "new-sha"
+            mock_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="mergeable", head_pipeline=None)
+            mock_project.mergerequests.get.return_value = mock_mr_obj
+
+            merge_request = GitLabValhallaMergeRequest()
+            config = MergeRequestConfig(enabled=True, target_branch="main", title="T", description="D", reviewers=[],
+                                        auto_merge=True)
+
+            # when:
+            hook = merge_request.create(config)
+            hook.enable_auto_merge()
+
+            # then:
+            self.assertEqual(mock_sleep.call_count, 60)
+            mock_warn.assert_any_call("Pipeline for merge request did not start in time, trying to enable auto-merge anyway")
+            mock_mr_obj.merge.assert_called_once_with(sha="new-sha", merge_when_pipeline_succeeds=True)
+
+    @patch("valhalla.ci_provider.gitlab.merge_request.get_gitlab_client")
+    @patch("valhalla.ci_provider.gitlab.merge_request.get_project_id")
+    @patch("valhalla.ci_provider.gitlab.merge_request.resolve")
+    @patch("valhalla.ci_provider.gitlab.merge_request.info")
+    @patch("valhalla.ci_provider.gitlab.merge_request.warn")
+    @patch("valhalla.ci_provider.gitlab.merge_request.Repo")
+    @patch("valhalla.ci_provider.gitlab.merge_request.time.sleep")
+    def test_enable_auto_merge_retries_on_405(self, mock_sleep, mock_repo, mock_warn, mock_info, mock_resolve,
+                                              mock_get_project_id, mock_get_gitlab_client):
+        with patch.dict('os.environ', {'CI_COMMIT_BRANCH': 'feature-branch'}):
+            # given:
+            mock_get_project_id.return_value = "123"
+            mock_resolve.side_effect = lambda x: x
+            mock_gitlab_client = MagicMock()
+            mock_project = MagicMock()
+            mock_gitlab_client.projects.get.return_value = mock_project
+            mock_get_gitlab_client.return_value = mock_gitlab_client
+            mock_repo.return_value.head.commit.hexsha = "new-sha"
+            mock_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="mergeable", head_pipeline={'sha': "new-sha"})
+            mock_mr_obj.merge.side_effect = [GitlabMRClosedError("Method Not Allowed", response_code=405), None]
+            mock_project.mergerequests.get.return_value = mock_mr_obj
+
+            merge_request = GitLabValhallaMergeRequest()
+            config = MergeRequestConfig(enabled=True, target_branch="main", title="T", description="D", reviewers=[],
+                                        auto_merge=True)
+
+            # when:
+            hook = merge_request.create(config)
+            hook.enable_auto_merge()
+
+            # then:
+            self.assertEqual(mock_mr_obj.merge.call_count, 2)
+            mock_info.assert_any_call("Auto-merge enabled, merge request will be merged when approvals and pipeline succeed")
+            self.assertFalse(any("Could not enable auto-merge" in c.args[0] for c in mock_warn.call_args_list))
+
     @patch("valhalla.ci_provider.gitlab.merge_request.get_gitlab_client")
     @patch("valhalla.ci_provider.gitlab.merge_request.get_project_id")
     @patch("valhalla.ci_provider.gitlab.merge_request.resolve")

valhalla/ci_provider/gitlab/merge_request.py

@@ -3,6 +3,7 @@
 from typing import List
 
 from git import Repo
+from gitlab.exceptions import GitlabError
 
 from valhalla.ci_provider.git_host import MergeRequest
 from valhalla.ci_provider.gitlab.common import get_gitlab_client, get_project_id
@@ -59,15 +60,27 @@ def _enable_auto_merge():
             try:
                 head_sha = Repo('.').head.commit.hexsha
                 mr_obj = self.project.mergerequests.get(mr_iid, iid=True)
-                # GitLab refreshes the MR asynchronously after push; enabling auto-merge before that
-                # (or without sha) makes GitLab abort it with "source branch was updated" or reject it with 405
-                for _ in range(30):
+                # GitLab refreshes the MR and creates its pipeline asynchronously after push; enabling auto-merge
+                # before that (or without sha) makes GitLab abort it with "source branch was updated" or reject it
+                # with 405, because without a pipeline there is nothing to wait for and it tries to merge right away
+                for _ in range(60):
                     if (mr_obj.sha == head_sha and
+                            (getattr(mr_obj, 'head_pipeline', None) or {}).get('sha') == head_sha and
                             getattr(mr_obj, 'detailed_merge_status', None) not in ('checking', 'preparing', 'unchecked')):
                         break
                     time.sleep(2)
                     mr_obj = self.project.mergerequests.get(mr_iid, iid=True)
-                mr_obj.merge(sha=head_sha, merge_when_pipeline_succeeds=True)
+                else:
+                    warn("Pipeline for merge request did not start in time, trying to enable auto-merge anyway")
+                for attempt in range(3):
+                    try:
+                        mr_obj.merge(sha=head_sha, merge_when_pipeline_succeeds=True)
+                        break
+                    except GitlabError as e:
+                        if e.response_code != 405 or attempt == 2:
+                            raise
+                        time.sleep(5)
+                        mr_obj = self.project.mergerequests.get(mr_iid, iid=True)
                 info("Auto-merge enabled, merge request will be merged when approvals and pipeline succeed")
             except Exception as e:
                 status = getattr(mr_obj, 'detailed_merge_status', 'unknown')

valhalla/commit/commit.py

@@ -101,10 +101,7 @@ def push(self, token):
     def __get_push_url(self, token):
         origin = self.repository.remote(name='origin')
         remote_url = origin.url
-        info(f"Remote url: {remote_url}")
         remote_url = remote_url.replace("https://", "").replace("http://", "")
         trimmed_url = remote_url.split('@')[-1] if '@' in remote_url else remote_url
-        info(f"trimmed_url: {trimmed_url}")
-        push_url = "https://{}:{}@{}".format("valhalla-bot", token, trimmed_url)
-        info(f"push_url: {push_url}")
-        return push_url
+        info(f"Remote url: {trimmed_url}")
+        return "https://{}:{}@{}".format("valhalla-bot", token, trimmed_url)