fix: send pipelineID in gitlabs SetCommitStatus if the mr gets found, fallback to branch ref #4785
Conversation
… fallback to branch ref
fitz7
requested review from
GenPage,
lukemassa and
nitrocode
and removed request for
a team
|
lgtm |
|
@lukemassa please take a look when you have some time. |
There was a problem hiding this comment.
I was never able to reproduce the issues in #3373 and #3722, but I could see how this could potential fix them. In general, it makes sense to me to call UpdateStatus with pipeline ID instead of a ref if we have access to it, since it'll be more accurate. So overall I don't see anything wrong with this PR, and there's a chance it could help, so sounds good to me! :)
|
thanks @fitz7 for the contribution |
|
FWIW, we gave this patch a try and it isn't working as expected. With this applied (on top of HEAD from yesterday), only the first pipeline is visible... all subsequent executions aren't linked to the MR nor the corresponding commits. Will circle back with more details. |
PR runatlantis#4785 was a partial fix, as it might link a commit to the wrong pipeline when GitLab is slow to update the HeadPipeline. This patch ensures the latest commit is always linked to the correct pipeline by querying the pipeline associated with the commit's ref, instead of relying on HeadPipeline. Tested in our environment and it looks good so far. Signed-off-by: Raul Gutierrez Segales <rsegales@nvidia.com>
what
The SetCommitStatus call in the GitLab clients UpdateStatus function will now send the pipelineID when the MR is found.
why
This should prevent Atlantis from making duplicate pipelines in GitLab
tests
These changes have been running on our private Atlantis for around a month
references
This should fix
#3373 and #3722
additional notes
I've also opened #4784 separately as it's not quite the same issue. Let me know if it would be better to combine these