From 8b4bfbd77d19a35b2e5a8c378bfe39624361b7c1 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Tue, 28 Nov 2023 10:11:59 +0100 Subject: [PATCH] feat(gitlab): allow override mergeable check attemps and use exponential backoff (#26008) --- docs/usage/self-hosted-experimental.md | 23 ++++++++++----- lib/modules/platform/gitlab/index.spec.ts | 36 +++++++++++------------ lib/modules/platform/gitlab/index.ts | 13 ++++---- lib/util/number.spec.ts | 16 +++++++++- lib/util/number.ts | 19 ++++++++++++ 5 files changed, 75 insertions(+), 32 deletions(-) diff --git a/docs/usage/self-hosted-experimental.md b/docs/usage/self-hosted-experimental.md index cfed99f956..be972bb87b 100644 --- a/docs/usage/self-hosted-experimental.md +++ b/docs/usage/self-hosted-experimental.md @@ -40,6 +40,21 @@ If set to any string, Renovate will use this as the `user-agent` it sends with H If set to an integer, Renovate will use this as max page number for docker tags lookup on docker registries, instead of the default 20 pages. This is useful for registries which ignores the `n` parameter in the query string and only return 50 tags per page. +## `RENOVATE_X_GITLAB_AUTO_MERGEABLE_CHECK_ATTEMPS` + +If set to an positive integer, Renovate will use this as the number of attempts to check if a merge request on GitLab is mergable before trying to automerge. +The formula for the delay between attempts is `250 * attempt * attempt` milliseconds. + +Default value: `5` (attempts results in max. 13.75 seconds timeout). + +## `RENOVATE_X_GITLAB_BRANCH_STATUS_DELAY` + +Adjust default time (in milliseconds) given to GitLab to create pipelines for a commit pushed by Renovate. + +Can be useful for slow-running, self-hosted GitLab instances that don't react fast enough for the default delay to help. + +Default value: `1000` (milliseconds). + ## `RENOVATE_X_HARD_EXIT` If set to any value, Renovate will use a "hard" `process.exit()` once all work is done, even if a sub-process is otherwise delaying Node.js from exiting. @@ -129,14 +144,6 @@ If set, Renovate will rewrite GitHub Enterprise Server's pagination responses to !!! note For the GitHub Enterprise Server platform only. -## `RENOVATE_X_GITLAB_BRANCH_STATUS_DELAY` - -Adjust default time (in milliseconds) given to GitLab to create pipelines for a commit pushed by Renovate. - -Can be useful for slow-running, self-hosted GitLab instances that don't react fast enough for the default delay to help. - -Default value: `1000` (milliseconds). - ## `OTEL_EXPORTER_OTLP_ENDPOINT` If set, Renovate will export OpenTelemetry data to the supplied endpoint. diff --git a/lib/modules/platform/gitlab/index.spec.ts b/lib/modules/platform/gitlab/index.spec.ts index 9ac1caba63..9a30d26c55 100644 --- a/lib/modules/platform/gitlab/index.spec.ts +++ b/lib/modules/platform/gitlab/index.spec.ts @@ -51,6 +51,7 @@ describe('modules/platform/gitlab/index', () => { }); delete process.env.GITLAB_IGNORE_REPO_URL; delete process.env.RENOVATE_X_GITLAB_BRANCH_STATUS_DELAY; + delete process.env.RENOVATE_X_GITLAB_AUTO_MERGEABLE_CHECK_ATTEMPS; }); async function initFakePlatform(version: string) { @@ -1791,17 +1792,12 @@ describe('modules/platform/gitlab/index', () => { .get('/api/v4/projects/undefined/merge_requests/12345') .reply(200) .get('/api/v4/projects/undefined/merge_requests/12345') - .reply(200, { - merge_status: 'can_be_merged', - pipeline: { - id: 29626725, - sha: '2be7ddb704c7b6b83732fdd5b9f09d5a397b5f8f', - ref: 'patch-28', - status: 'success', - }, - }) + .reply(200) + .get('/api/v4/projects/undefined/merge_requests/12345') + .reply(200) .put('/api/v4/projects/undefined/merge_requests/12345/merge') .reply(200); + process.env.RENOVATE_X_GITLAB_AUTO_MERGEABLE_CHECK_ATTEMPS = '3'; expect( await gitlab.createPr({ sourceBranch: 'some-branch', @@ -1813,15 +1809,19 @@ describe('modules/platform/gitlab/index', () => { usePlatformAutomerge: true, }, }), - ).toMatchInlineSnapshot(` - { - "id": 1, - "iid": 12345, - "number": 12345, - "sourceBranch": "some-branch", - "title": "some title", - } - `); + ).toEqual({ + id: 1, + iid: 12345, + number: 12345, + sourceBranch: 'some-branch', + title: 'some title', + }); + + expect(timers.setTimeout.mock.calls).toMatchObject([ + [250], + [1000], + [2250], + ]); }); it('raises with squash enabled when repository squash option is default_on', async () => { diff --git a/lib/modules/platform/gitlab/index.ts b/lib/modules/platform/gitlab/index.ts index b186d67f60..be38419752 100644 --- a/lib/modules/platform/gitlab/index.ts +++ b/lib/modules/platform/gitlab/index.ts @@ -23,6 +23,7 @@ import * as git from '../../../util/git'; import * as hostRules from '../../../util/host-rules'; import { setBaseUrl } from '../../../util/http/gitlab'; import type { HttpResponse } from '../../../util/http/types'; +import { parseInteger } from '../../../util/number'; import * as p from '../../../util/promises'; import { regEx } from '../../../util/regex'; import { sanitize } from '../../../util/sanitize'; @@ -644,7 +645,11 @@ async function tryPrAutomerge( } const desiredStatus = 'can_be_merged'; - const retryTimes = 8; // results in max. 5 min. timeout if no pipeline created + // The default value of 5 attempts results in max. 13.75 seconds timeout if no pipeline created. + const retryTimes = parseInteger( + process.env.RENOVATE_X_GITLAB_AUTO_MERGEABLE_CHECK_ATTEMPS, + 5, + ); // Check for correct merge request status before setting `merge_when_pipeline_succeeds` to `true`. for (let attempt = 1; attempt <= retryTimes; attempt += 1) { @@ -658,7 +663,7 @@ async function tryPrAutomerge( if (body.merge_status === desiredStatus && body.pipeline !== null) { break; } - await setTimeout(500 * attempt); + await setTimeout(250 * attempt ** 2); // exponential backoff } await gitlabApi.putJson( @@ -938,9 +943,7 @@ export async function setBranchStatus({ try { // give gitlab some time to create pipelines for the sha await setTimeout( - process.env.RENOVATE_X_GITLAB_BRANCH_STATUS_DELAY - ? parseInt(process.env.RENOVATE_X_GITLAB_BRANCH_STATUS_DELAY, 10) - : 1000, + parseInteger(process.env.RENOVATE_X_GITLAB_BRANCH_STATUS_DELAY, 1000), ); await gitlabApi.postJson(url, { body: options }); diff --git a/lib/util/number.spec.ts b/lib/util/number.spec.ts index 4f215c1aa1..61df71896a 100644 --- a/lib/util/number.spec.ts +++ b/lib/util/number.spec.ts @@ -1,4 +1,4 @@ -import { coerceNumber } from './number'; +import { coerceNumber, parseInteger } from './number'; describe('util/number', () => { it.each` @@ -9,4 +9,18 @@ describe('util/number', () => { `('coerceNumber($val, $def) = $expected', ({ val, def, expected }) => { expect(coerceNumber(val, def)).toBe(expected); }); + + it.each` + val | def | expected + ${1} | ${2} | ${2} + ${undefined} | ${2} | ${2} + ${undefined} | ${undefined} | ${0} + ${''} | ${undefined} | ${0} + ${'-1'} | ${undefined} | ${0} + ${'1.1'} | ${undefined} | ${0} + ${'a'} | ${undefined} | ${0} + ${'5'} | ${undefined} | ${5} + `('parseInteger($val, $def) = $expected', ({ val, def, expected }) => { + expect(parseInteger(val, def)).toBe(expected); + }); }); diff --git a/lib/util/number.ts b/lib/util/number.ts index 3c8bac99f5..6e8ebb742d 100644 --- a/lib/util/number.ts +++ b/lib/util/number.ts @@ -1,3 +1,5 @@ +import is from '@sindresorhus/is'; + /** * Coerces a value to a number with optional default value. * @param val the value to coerce @@ -10,3 +12,20 @@ export function coerceNumber( ): number { return val ?? def ?? 0; } + +/** + * Parses a value as a finite positive integer with optional default value. + * If no default value is provided, the default value is 0. + * @param val Value to parse as finite integer. + * @param def Optional default value. + * @returns The parsed value or the default value if the parsed value is not finite. + */ +export function parseInteger( + val: string | undefined | null, + def?: number, +): number { + // Number.parseInt returns NaN if the value is not a finite integer. + const parsed = + is.string(val) && /^\d+$/.test(val) ? Number.parseInt(val, 10) : Number.NaN; + return Number.isFinite(parsed) ? parsed : def ?? 0; +}