Harden automerge on slower Self-Hosted GitLab instances #45368
malmor
started this conversation in
Suggest an Idea
Replies: 1 comment 1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Tell us more.
Hey there,
we recently stumbled upon an issue where Renovate auto-merged a dependency update, while the pipeline was failing. After digging a bit more into it, and checking the debug logs, it turns out that our GitLab instance was kinda slow with creating the new pipeline - and Renovate merged before a new test was ever started.
For reference:
platformAutomergefeature:enabledWe traced this back to the
tryPrAutomergefunction here:renovate/lib/modules/platform/gitlab/index.ts
Lines 579 to 703 in dd72cf0
Before Renovate calls the
/mergeendpoint, it checks the merge request status a few times (with customizable retries and backoff), until this check passes:renovate/lib/modules/platform/gitlab/index.ts
Lines 641 to 646 in dd72cf0
We did some tests by polling a merge request while adding a commit, here is some example logging:
Output from polling merge request via API every 0.25s
Here we can see that GitLab considered the merge request
mergeablewhile thepipeline.idstill pointed to the old commit - so Renovate might merge before a pipeline got created.I am aware that the root cause here is a (occasionally) slow GitLab instance, and the fact that our repository did not have the
Pipeline must succeedfeature enabled - which would have blocked the merge in the first place. But since there are some scenarios where that feature is disabled, the question is if the existing path could maybe be hardened a bit.Looking at this check again:
renovate/lib/modules/platform/gitlab/index.ts
Lines 641 to 646 in dd72cf0
This check only waits for a pipeline to be part of the merge request, which can also be for an older commit. Looking at the logs above, we can see that the
pipeline.shadid not match the mainshauntil the new pipeline started.What do you think about adding this as an additional hardening to the condition? This would ensure that the pipeline / status is actually based on a pipeline for that last commit, and the retries / backoffs would help for slower instances. In the end this does not break existing flows, since the fallback (after all retires) still just attempts to merge.
I'd be happy to create a pull request for that.
Cheers, Malte
All reactions