-
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
Fix: edge-case in gitlab_client #2495
Conversation
@jamengual could you please review when you get a chance? This probably isn't very elegant and I'd like some input on: https://github.com/runatlantis/atlantis/pull/2495/files#diff-22d1bafb2e6e476aabe2f33484daa3f74932c6e0266f0830d7d948b8b9f60183R236-R243 |
for _, p := range pipelines { | ||
if p.Status == "skipped" { | ||
isPipelineSkipped = true | ||
break | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if there's a better way to navigate to a common HeadPipeline
Anything I can do to help move things along? |
@jukie Sorry we have been away/sick/busy we will try to review it soon. |
Hi @jukie, I was unable to reproduce this behavior locally. |
@michelmzs no this would be mering branchB into the mainline branch and I can say it's still happening with latest version of Atlantis and Gitlab version 15.2.2 To elaborate on the steps (will update above as well):
|
Followed the exactly steps above, and still couldn't reproduce (GitLab 14.9.x, I'll try in 15.2.2). |
Interesting, I hadn't realized that was a new layout in 15.x. I'll also look at whether this is unintended behavior that could be fixed in later 15.x versions or a possible bug in Gitlab vs here. |
@michelmzs I've confirmed this again locally and against the publicly hosted gitlab.com here: https://gitlab.com/Jukie/tf-demo/-/merge_requests/3#note_1130657254 |
@michelmzs any updates here? It's a confirmed issue with 15.x but I'm unclear whether it's something that should be resolved here or in Gitlab |
Sorry for late reply @jukie, I haven't tested it locally yet. On the changes, instead of looking for the parent pipeline, maybe we should address the job status update during the plan phase in the current merge request. I don't believe that issuing a plan status for another merge request than the current one is a good fit. |
Thank you @jukie and @michelmzs Looks like this PR can be closed since #2636 was merged. |
Closes #2484
If 2 GitLab Merge Requests reference the same commit Gitlab doesn't separate the associated pipeline runs so in that case whatever MR/branch was pushed or opened second will have an empty
mr.HeadPipeline
. In this case Atlantis should lookup associated pipelines and their status by using the commit sha.Reproduction Steps (Copied from issue)
git checkout -b coolStuffA
coolStuffA
checked out. From here create a new branch:git checkout -b coolStuffB
git push origin HEAD
coolStuffB
into mainruntime error: invalid memory address or nil pointer dereference