Skip to content

Commit

Permalink
feat: use Notes API for Gitlab Automerge to support merge trains
Browse files Browse the repository at this point in the history 
closes #5573

Gitlab's /merge API does not support merge trains, but sending "/merge"
over the MR comment (notes) API does work:
https://gitlab.com/gitlab-org/gitlab/-/issues/32665

Signed-off-by: Florian Ströger <[email protected]>
  • Loading branch information
@Preisschild
Preisschild committed Feb 7, 2025
1 parent a5f044a commit 39e6f08
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 111 deletions.
1 change: 0 additions & 1 deletion docs/usage/faq.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ Some major platform features are not supported at all by Renovate.
| --------------------------------------- | ----------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Jira issues | Bitbucket | [#20568](https://github.com/renovatebot/renovate/issues/20568) |
| Jira issues | Bitbucket Server | [#3796](https://github.com/renovatebot/renovate/issues/3796) |
| Merge trains | GitLab | [#5573](https://github.com/renovatebot/renovate/issues/5573) |
| Configurable merge strategy and message | Only Bitbucket, Forgejo and Gitea for now | [#10867](https://github.com/renovatebot/renovate/issues/10867) [#10869](https://github.com/renovatebot/renovate/issues/10869) [#10870](https://github.com/renovatebot/renovate/issues/10870) |

## What is this `main` branch I see in the documentation?
Expand Down
97 changes: 13 additions & 84 deletions lib/modules/platform/gitlab/index.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2143,7 +2143,7 @@ describe('modules/platform/gitlab/index', () => {
.reply(200)
.get('/api/v4/projects/undefined/merge_requests/12345')
.reply(200)
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200);
expect(
await gitlab.createPr({
Expand Down Expand Up @@ -2187,9 +2187,7 @@ describe('modules/platform/gitlab/index', () => {
.reply(200, reply_body)
.get('/api/v4/projects/undefined/merge_requests/12345')
.reply(200, reply_body)
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.reply(405, {})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200, {});
process.env.RENOVATE_X_GITLAB_AUTO_MERGEABLE_CHECK_ATTEMPS = '3';
const pr = await gitlab.createPr({
Expand Down Expand Up @@ -2219,7 +2217,6 @@ describe('modules/platform/gitlab/index', () => {
[100],
[400],
[900],
[100],
]);
});

Expand All @@ -2245,7 +2242,7 @@ describe('modules/platform/gitlab/index', () => {
.reply(200, reply_body)
.get('/api/v4/projects/undefined/merge_requests/12345')
.reply(200, reply_body)
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200, {});
process.env.RENOVATE_X_GITLAB_AUTO_MERGEABLE_CHECK_ATTEMPS = '3';
const pr = await gitlab.createPr({
Expand Down Expand Up @@ -2274,75 +2271,6 @@ describe('modules/platform/gitlab/index', () => {
expect(timers.setTimeout.mock.calls).toMatchObject([[100], [400], [900]]);
});

it('should retry auto merge creation on 405 method not allowed', async () => {
await initPlatform('15.6.0-ee');
const reply_body = {
detailed_merge_status: 'pending',
};
httpMock
.scope(gitlabApiHost)
.post('/api/v4/projects/undefined/merge_requests')
.reply(200, {
id: 1,
iid: 12345,
title: 'some title',
source_branch: 'some-branch',
target_branch: 'master',
description: 'the-body',
})
.get('/api/v4/projects/undefined/merge_requests/12345')
.reply(200, reply_body)
.get('/api/v4/projects/undefined/merge_requests/12345')
.reply(200, reply_body)
.get('/api/v4/projects/undefined/merge_requests/12345')
.reply(200, reply_body)
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.reply(405, {})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.reply(405, {})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.reply(200, {});
process.env.RENOVATE_X_GITLAB_AUTO_MERGEABLE_CHECK_ATTEMPS = '3';
const pr = await gitlab.createPr({
sourceBranch: 'some-branch',
targetBranch: 'master',
prTitle: 'some-title',
prBody: 'the-body',
platformPrOptions: {
usePlatformAutomerge: true,
},
});
expect(pr).toMatchObject({
number: 12345,
sourceBranch: 'some-branch',
title: 'some title',
});
expect(logger.debug).toHaveBeenCalledWith(
'PR not yet in mergeable state. Retrying 1',
);
expect(logger.debug).toHaveBeenCalledWith(
'PR not yet in mergeable state. Retrying 2',
);
expect(logger.debug).toHaveBeenCalledWith(
'PR not yet in mergeable state. Retrying 3',
);
expect(logger.debug).toHaveBeenCalledWith(
expect.any(Object),
'Automerge on PR creation failed. Retrying 1',
);
expect(logger.debug).toHaveBeenCalledWith(
expect.any(Object),
'Automerge on PR creation failed. Retrying 2',
);
expect(timers.setTimeout.mock.calls).toMatchObject([
[100],
[400],
[900],
[100],
[400],
]);
});

it('should not retry if MR is mergeable and pipeline is running', async () => {
await initPlatform('15.6.0-ee');
const reply_body = {
Expand All @@ -2364,7 +2292,7 @@ describe('modules/platform/gitlab/index', () => {
})
.get('/api/v4/projects/undefined/merge_requests/12345')
.reply(200, reply_body)
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200, {});
const pr = await gitlab.createPr({
sourceBranch: 'some-branch',
Expand Down Expand Up @@ -2486,7 +2414,7 @@ describe('modules/platform/gitlab/index', () => {
status: 'success',
},
})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200)
.get('/api/v4/projects/undefined/merge_requests/12345/approval_rules')
.reply(200, [])
Expand Down Expand Up @@ -2580,7 +2508,7 @@ describe('modules/platform/gitlab/index', () => {
status: 'success',
},
})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200)
.get('/api/v4/projects/undefined/merge_requests/12345/approval_rules')
.reply(200, [
Expand Down Expand Up @@ -2638,7 +2566,7 @@ describe('modules/platform/gitlab/index', () => {
status: 'success',
},
})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200)
.get('/api/v4/projects/undefined/merge_requests/12345/approval_rules')
.reply(200, [
Expand Down Expand Up @@ -2707,7 +2635,7 @@ describe('modules/platform/gitlab/index', () => {
status: 'success',
},
})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200)
.get('/api/v4/projects/undefined/merge_requests/12345/approval_rules')
.reply(200, [
Expand Down Expand Up @@ -2786,7 +2714,7 @@ describe('modules/platform/gitlab/index', () => {
status: 'success',
},
})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200)
.get('/api/v4/projects/undefined/merge_requests/12345/approval_rules')
.reply(200, [
Expand Down Expand Up @@ -2836,7 +2764,7 @@ describe('modules/platform/gitlab/index', () => {
status: 'success',
},
})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200)
.get('/api/v4/projects/undefined/merge_requests/12345/approval_rules')
.reply(200, [])
Expand Down Expand Up @@ -3376,7 +3304,7 @@ describe('modules/platform/gitlab/index', () => {
status: 'running',
},
})
.put('/api/v4/projects/undefined/merge_requests/12345/merge')
.post('/api/v4/projects/undefined/merge_requests/12345/notes')
.reply(200);

await expect(gitlab.reattemptPlatformAutomerge?.(pr)).toResolve();
Expand All @@ -3391,11 +3319,12 @@ describe('modules/platform/gitlab/index', () => {
it('merges the PR', async () => {
httpMock
.scope(gitlabApiHost)
.put('/api/v4/projects/undefined/merge_requests/1/merge')
.post('/api/v4/projects/undefined/merge_requests/1/notes')
.reply(200);
expect(
await gitlab.mergePr({
id: 1,
branchName: "some-branch"
}),
).toBeTrue();
});
Expand Down
57 changes: 31 additions & 26 deletions lib/modules/platform/gitlab/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,8 @@ async function tryPrAutomerge(
250,
);

let diffHeadSha = "";

// Check for correct merge request status before setting `merge_when_pipeline_succeeds` to `true`.
for (let attempt = 1; attempt <= retryTimes; attempt += 1) {
const { body } = await gitlabApi.getJsonUnchecked<{
Expand All @@ -666,9 +668,13 @@ async function tryPrAutomerge(
pipeline: {
status: string;
};
sha: string;
}>(`projects/${config.repository}/merge_requests/${pr}`, {
memCache: false,
});

diffHeadSha = body.sha;

// detailed_merge_status is available with Gitlab >=15.6.0
const use_detailed_merge_status = !!body.detailed_merge_status;
const detailed_merge_status_check =
Expand All @@ -690,28 +696,15 @@ async function tryPrAutomerge(
await setTimeout(mergeDelay * attempt ** 2); // exponential backoff
}

// Even if Gitlab returns a "merge-able" merge request status, enabling auto-merge sometimes
// returns a 405 Method Not Allowed. It seems to be a timing issue within Gitlab.
for (let attempt = 1; attempt <= retryTimes; attempt += 1) {
try {
await gitlabApi.putJson(
`projects/${config.repository}/merge_requests/${pr}/merge`,
{
body: {
should_remove_source_branch: true,
merge_when_pipeline_succeeds: true,
},
},
);
break;
} catch (err) {
logger.debug(
{ err },
`Automerge on PR creation failed. Retrying ${attempt}`,
);
}
await setTimeout(mergeDelay * attempt ** 2); // exponential backoff
}
await gitlabApi.postJson(
`projects/${config.repository}/merge_requests/${pr}/notes`,
{
body: {
body: "/merge",
merge_request_diff_head_sha: diffHeadSha,
},
},
);
}
} catch (err) /* istanbul ignore next */ {
logger.debug({ err }, 'Automerge on PR creation failed');
Expand Down Expand Up @@ -858,13 +851,25 @@ export async function reattemptPlatformAutomerge({
logger.debug(`PR platform automerge re-attempted...prNo: ${iid}`);
}

export async function mergePr({ id }: MergePRConfig): Promise<boolean> {
export async function mergePr({ id, branchName}: MergePRConfig): Promise<boolean> {
if (!branchName) {
logger.warn('Failed to get the branch name');
return false;
}

const branchSha = git.getBranchCommit(branchName);

if (!branchSha) {
logger.warn('Failed to get the branch commit SHA');
return false;
}
try {
await gitlabApi.putJson(
`projects/${config.repository}/merge_requests/${id}/merge`,
await gitlabApi.postJson(
`projects/${config.repository}/merge_requests/${id}/notes`,
{
body: {
should_remove_source_branch: true,
body: "/merge",
merge_request_diff_head_sha: branchSha,
},
},
);
Expand Down

0 comments on commit 39e6f08

Please sign in to comment.