Skip to content

Commit

Permalink
Set temporary branch for external MRs
Browse files Browse the repository at this point in the history
If it's to be reliable, CI should be run on the target project,
even when the MR comes from a fork. This commit introduces the
temp_branch argument for specifying the temporary branch name
that will be used for running the CI in the target project.
  • Loading branch information
Jellby committed Jul 20, 2018
1 parent be6061e commit 31f0f4f
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 21 deletions.
7 changes: 7 additions & 0 deletions marge/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@ def regexp(str_regex):
default='.*',
help='Only process MRs whose target branches match the given regular expression.\n',
)
parser.add_argument(
'--temp-branch',
type=str,
default="",
help='Temporary branch name for external merge requests (disabled if empty).\n',
)
parser.add_argument(
'--debug',
action='store_true',
Expand Down Expand Up @@ -253,6 +259,7 @@ def main(args=None):
embargo=options.embargo,
ci_timeout=options.ci_timeout,
use_merge_strategy=options.use_merge_strategy,
temp_branch=options.temp_branch,
),
batch=options.batch,
)
Expand Down
63 changes: 54 additions & 9 deletions marge/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
from collections import namedtuple
from datetime import datetime, timedelta

from . import git
from . import git, gitlab
from .commit import Commit
from .interval import IntervalUnion
from .project import Project
from .user import User
Expand Down Expand Up @@ -114,19 +115,23 @@ def add_trailers(self, merge_request):
return sha

def get_mr_ci_status(self, merge_request, commit_sha=None):
temp_branch = self.opts.temp_branch
if commit_sha is None:
commit_sha = merge_request.sha
pipelines = Pipeline.pipelines_by_branch(
merge_request.source_project_id,
merge_request.source_branch,
self._api,
)
if temp_branch and merge_request.source_project_id != self._project.id:
pid = self._project.id
ref = temp_branch
self.update_temp_branch(merge_request, commit_sha)
else:
pid = merge_request.source_project_id
ref = merge_request.source_branch
pipelines = Pipeline.pipelines_by_branch(pid, ref, self._api)
current_pipeline = next(iter(pipelines), None)

if current_pipeline and current_pipeline.sha == commit_sha:
ci_status = current_pipeline.status
else:
log.warning('No pipeline listed for %s on branch %s', commit_sha, merge_request.source_branch)
log.warning('No pipeline listed for %s on branch %s', commit_sha, ref)
ci_status = None

return ci_status
Expand Down Expand Up @@ -196,7 +201,7 @@ def fetch_source_project(self, merge_request):
remote = 'source'
remote_url = source_project.ssh_url_to_repo
self._repo.fetch(
remote=remote,
remote_name=remote,
remote_url=remote_url,
)
return source_project, remote_url, remote
Expand Down Expand Up @@ -281,6 +286,43 @@ def update_from_target_branch_and_push(
else:
assert source_repo_url is not None

def update_temp_branch(self, merge_request, commit_sha):
api = self._api
project_id = self._project.id
temp_branch = self.opts.temp_branch
waiting_time_in_secs = 30

try:
sha_branch = Commit.last_on_branch(project_id, temp_branch, api).id
except gitlab.NotFound:
sha_branch = None
if sha_branch != commit_sha:
log.info('Setting up %s in target project', temp_branch)
self.delete_temp_branch(merge_request.source_project_id)
self._project.create_branch(temp_branch, commit_sha, api)
self._project.protect_branch(temp_branch, api)
merge_request.comment(
('The temporary branch **{branch}** was updated to [{sha:.8}](../commit/{sha}) ' +
'and local pipelines will be used.').format(
branch=temp_branch, sha=commit_sha
)
)

time.sleep(waiting_time_in_secs)

def delete_temp_branch(self, source_project_id):
temp_branch = self.opts.temp_branch

if temp_branch and source_project_id != self._project.id:
try:
self._project.unprotect_branch(temp_branch, self._api)
except gitlab.ApiError:
pass
try:
self._project.delete_branch(temp_branch, self._api)
except gitlab.ApiError:
pass


def _get_reviewer_names_and_emails(approvals, api):
"""Return a list ['A. Prover <[email protected]', ...]` for `merge_request.`"""
Expand All @@ -298,6 +340,7 @@ def _get_reviewer_names_and_emails(approvals, api):
'embargo',
'ci_timeout',
'use_merge_strategy',
'temp_branch',
]


Expand All @@ -312,7 +355,8 @@ def requests_commit_tagging(self):
def default(
cls, *,
add_tested=False, add_part_of=False, add_reviewers=False, reapprove=False,
approval_timeout=None, embargo=None, ci_timeout=None, use_merge_strategy=False
approval_timeout=None, embargo=None, ci_timeout=None, use_merge_strategy=False,
temp_branch=""
):
approval_timeout = approval_timeout or timedelta(seconds=0)
embargo = embargo or IntervalUnion.empty()
Expand All @@ -326,6 +370,7 @@ def default(
embargo=embargo,
ci_timeout=ci_timeout,
use_merge_strategy=use_merge_strategy,
temp_branch=temp_branch,
)


Expand Down
29 changes: 20 additions & 9 deletions marge/merge_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,26 @@ def comment(self, message):

return self._api.call(POST(notes_url, {'body': message}))

def accept(self, remove_branch=False, sha=None):
return self._api.call(PUT(
'/projects/{0.project_id}/merge_requests/{0.iid}/merge'.format(self),
dict(
should_remove_source_branch=remove_branch,
merge_when_pipeline_succeeds=True,
sha=sha or self.sha, # if provided, ensures what is merged is what we want (or fails)
),
))
def accept(self, remove_branch=False, sha=None, trust_pipeline=True, project=None):
reset = False
if not trust_pipeline:
# Temporarily remove flag, because we want to merge based on another pipeline
if project.only_allow_merge_if_pipeline_succeeds:
reset = True
project.set_only_allow_merge_if_pipeline_succeeds(False, self._api)
try:
result = self._api.call(PUT(
'/projects/{0.project_id}/merge_requests/{0.iid}/merge'.format(self),
dict(
should_remove_source_branch=remove_branch,
merge_when_pipeline_succeeds=trust_pipeline,
sha=sha or self.sha, # if provided, ensures what is merged is what we want (or fails)
),
))
finally:
if reset:
project.set_only_allow_merge_if_pipeline_succeeds(True, self._api)
return result

def close(self):
return self._api.call(PUT(
Expand Down
31 changes: 30 additions & 1 deletion marge/project.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import logging as log
from enum import Enum, unique
from functools import partial
from requests.utils import quote

from . import gitlab


GET = gitlab.GET
GET, PUT, POST, DELETE = gitlab.GET, gitlab.PUT, gitlab.POST, gitlab.DELETE


class Project(gitlab.Resource):
Expand Down Expand Up @@ -72,6 +73,34 @@ def access_level(self):
assert effective_access is not None, "GitLab failed to provide user permissions on project"
return AccessLevel(effective_access['access_level'])

def set_only_allow_merge_if_pipeline_succeeds(self, value, api):
return api.call(PUT(
'/projects/%s' % self.info['id'],
{'only_allow_merge_if_pipeline_succeeds': value}
))

def protect_branch(self, branch, api):
return api.call(POST(
'/projects/%s/protected_branches' % self.info['id'],
{'name': branch}
))

def unprotect_branch(self, branch, api):
return api.call(DELETE('/projects/{project_id}/protected_branches/{name}'.format(
project_id=self.info['id'], name=quote(branch, safe='')
)))

def create_branch(self, branch, sha, api):
return api.call(POST(
'/projects/%s/repository/branches' % self.info['id'],
{'branch': branch, 'ref': sha}
))

def delete_branch(self, branch, api):
return api.call(DELETE('/projects/{project_id}/repository/branches/{name}'.format(
project_id=self.info['id'], name=quote(branch, safe='')
)))


@unique
class AccessLevel(Enum):
Expand Down
10 changes: 8 additions & 2 deletions marge/single_merge_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,18 @@ def update_merge_request_and_accept(self, approvals):

self.maybe_reapprove(merge_request, approvals)

if source_project.only_allow_merge_if_pipeline_succeeds:
trust = True
if self._project.only_allow_merge_if_pipeline_succeeds:
self.wait_for_ci_to_pass(merge_request, actual_sha)
time.sleep(2)
if self.opts.temp_branch and source_project is not self._project:
trust = False

self.ensure_mergeable_mr(merge_request)

try:
merge_request.accept(remove_branch=True, sha=actual_sha)
merge_request.accept(remove_branch=True, sha=actual_sha, trust_pipeline=trust,
project=self._project)
except gitlab.NotAcceptable as err:
new_target_sha = Commit.last_on_branch(self._project.id, merge_request.target_branch, api).id
# target_branch has moved under us since we updated, just try again
Expand Down Expand Up @@ -126,6 +130,8 @@ def update_merge_request_and_accept(self, approvals):
log.exception('Unanticipated ApiError from GitLab on merge attempt')
raise CannotMerge('had some issue with GitLab, check my logs...')
else:
# temp_branch can be removed before because it was only used for CI, not for merging
self.delete_temp_branch(source_project.id)
self.wait_for_branch_to_be_merged()
updated_into_up_to_date_target_branch = True

Expand Down
1 change: 1 addition & 0 deletions tests/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ def test_default(self):
embargo=marge.interval.IntervalUnion.empty(),
ci_timeout=timedelta(minutes=15),
use_merge_strategy=False,
temp_branch='',
)

def test_default_ci_time(self):
Expand Down

0 comments on commit 31f0f4f

Please sign in to comment.