Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: send pipelineID in gitlabs SetCommitStatus if the mr gets found, fallback to branch ref #4785

Merged

Conversation

fitz7
Copy link
Contributor

@fitz7 fitz7 commented Jul 25, 2024

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

@fitz7 fitz7 requested review from a team as code owners July 25, 2024 10:52
@fitz7 fitz7 requested review from GenPage, lukemassa and nitrocode and removed request for a team July 25, 2024 10:52
@github-actions github-actions bot added go Pull requests that update Go code provider/gitlab labels Jul 25, 2024
@chenrui333 chenrui333 added the bug Something isn't working label Jul 25, 2024
@chenrui333
Copy link
Member

lgtm

@jamengual
Copy link
Contributor

@lukemassa please take a look when you have some time.

Copy link
Contributor

@lukemassa lukemassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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! :)

@jamengual jamengual merged commit ed03d35 into runatlantis:main Aug 8, 2024
28 checks passed
@jamengual
Copy link
Contributor

thanks @fitz7 for the contribution

@rgs1
Copy link

rgs1 commented Aug 21, 2024

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.

rgs1 added a commit to rgs1/atlantis that referenced this pull request Aug 21, 2024
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go Pull requests that update Go code provider/gitlab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants