-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Update GitLab Pipeline Type Detection to use Head Pipeline Property #3887
feat: Update GitLab Pipeline Type Detection to use Head Pipeline Property #3887
Conversation
I like the retry option, on self-hosted instances I experience even longer delays, so I suggest at least a simple retry/backoff. |
Similar issue here with the delays on self-hosted instances. In my case we exclusively use (merge result) MR pipelines to do some static checks on TF and provider versions, among other things. We need the merge-result pipelines to enforce changes to these checks without forcing users to rebase their branches. In the event that:
GitLab seems to consider the "last" pipeline to be successful and Atlantis considers the MR mergable. The user is now able to As such, I'd like to suggest having a configuration option to never fall-through to spawning the branch-pipeline job if |
I've added one retry to get the MR head pipeline details with a 2 second delay. In my tests with a locally deployed GitLab instance, it never retries (but the GitLab instance is not busy). |
This is outside the scope of this PR, and from my testing, if |
/cherry-pick release-0.26 |
Cherry-pick failed with |
/cherry-pick release-0.26 |
…rty (#3887) * Fix GitLab Mulitple Pipelines * Add logger to tests and fix test ref * Add retry to GetMergeRequest * Update retries --------- Co-authored-by: PePe Amengual <[email protected]>
@jamengual after this change we are seeing:
Given that https://github.com/runatlantis/atlantis/blob/main/server/events/vcs/gitlab_client.go#L391 cc: @X-Guardian |
there is a PR inflight to fix this I believe #3996 |
…rty (runatlantis#3887) * Fix GitLab Mulitple Pipelines * Add logger to tests and fix test ref * Add retry to GetMergeRequest * Update retries --------- Co-authored-by: PePe Amengual <[email protected]>
…rty (runatlantis#3887) * Fix GitLab Mulitple Pipelines * Add logger to tests and fix test ref * Add retry to GetMergeRequest * Update retries --------- Co-authored-by: PePe Amengual <[email protected]>
what
The GitLab pipeline detection code is currently using a legacy property,
pipeline
of the Get Single MR API to get details of the pipeline for an MR. This PR changes the property used toHeadPipeline
. It also uses therefTarget
from theHeadPipeline
object rather than manually calculating the ref, and adds a debug line.The
TestGitlabClient_UpdateStatus
test has also been updated to add a logger to the GitLab client and fix the expected ref to match the ref in thepipelineSuccess
mock data.why
GitLab recommend using the
HeadPipeline
property of rather thanPipeline
, ref: GitLab REST API - Get single MR - Response and in tests I have done, when creating an MR and straight away calling the 'Get Single MR' API, thePipeline
property returns null in the first 3 seconds, whereasHeadPipeline
only returns null for the first second. A null value would make Atlantis use the branch reference for the pipeline rather than the pipeline reference, causing multiple pipelines to display in the UI, as raised in #3373.I did look at the returned object to see if there was any other property that would indicate whether the
HeadPipeline
field was ready for reading or not, but I couldn't see anything. The problem is that if the repo has no other pipelines defined for it, thePipeline/HeadPipeline
properties will always be null. I've attached the test script and results from a repo using a GitLab CI pipeline below for reference.I was also considering adding a single retry with say a 1 second delay to the Get Single MR API call if the
HeadPipeline
property was null to try and reduce the race condition window further. Interested in any opinions.tests
GitLab REST API Test Script
Results