This happens on GitLab.com: GitLab Enterprise Edition 14.6.0-pre a02124bc
Expand for output related to GitLab environment info
(For installations with omnibus-gitlab package run and paste the output of:
`sudo gitlab-rake gitlab:env:info`)
(For installations from source run and paste the output of:
`sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)
Results of GitLab application Check
Expand for output related to the GitLab application check
(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true)
(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true)
(we will only investigate if the tests are passing)
Possible fixes
The behavior of the Pipeline widget should stay consistent before and after the MR is merged.
Environments that are expected to deploy for a given pipeline should always be listed in the Pipeline widget for an MR, even post-merge (see example below):
It is here #346666@lyle ! I believe @dfosco will be taking this forward now, but it'd still be nice to hear if that proposal meets your expectations so that this bug can get fixed quicker!
As I was going through the issue body my brain took credit for your work. I completely thought that I had included that section when I created the issue and was confused. By even having made that mistake it's clear that your proposal meets my expectations! Thanks for putting it together
Not sure if this the same or a similar problem, but i sometimes don't see a manual "Deploy" button in my merge requests and then have to go into the pipeline to start a downstream pipeline
Merge request (click to expand)Pipeline view (click to expand)
@nicolewilliams tough to say. The frontend really only loads whatever the backend sends it, but it also polls for updates, so it could be a timing issue.
I don't think it is though, it seems it's a little too reproducible to be timing related I'm leaning towards backend, yeah
I spent more time on this earlier today, only to find out the draft merge request I created was not, in fact, fixing the issue!
To put it simply, going through the code and commit history, I realised this is not a bug, but a feature! 🤯
Let me explain 🪄
Before !20066 (merged), deployments were loaded in the environments widget from the associated merge pipeline (see here, and here).
However, after the introduction of deployment_merge_requests join table, we started - in !20066 (merged) - to display deployments linked to merge requests directly and not via pipelines, but we still kept a fallback to ensure old projects would have the correct deployments showing in the widget.
So, this was done purposefully, and is the reason there's inconsistency, as explained below.
Breakdown
To get a bit deeper here, let's examine briefly how the current implementation works:
Environments widget before merge: loads environments via head pipeline.
We do not show environments when they have a manual job because there's no deployment created yet (see point 1).
We fallback to the old way (loading via merge pipeline) if there are no deployments linked to the merge request.
There are no major issues with the widget before merge, it seemingly works fine most of the time.
After merging, however, is where the problem lies. I experimented a bit with that, and found out the issue can be sorted out by loading deployments via the merge pipeline only, as can be seen in screenshots below (using .gitlab-ci.yml file provided by @lyle).
Screenshots
Example 1 (merged - manual jobs already deployed)
Example 2 (merged - manual jobs can be deployed)
Example 1 (not merged - no merge pipeline yet)
Nonetheless, this left me with a number of questions:
Shall we load only existing deployments in the widget? Or also prospective ones? (manual jobs yet to be executed)
What happens to the changes in !20066 (merged) if we were to load via merge pipeline only?
Do we still consider this a bug given the above?
How to move forward?
From my point of view, I see two paths forward here:
We either decide the expected behaviour is the current one, and move forward + close this issue.
Or, we still consider this a bug, and then we would have two fixes:
Load deployments via the merge pipeline only, and drop some of the changes from !20066 (merged).
Try to create EnvironmentStatus objects for prospective deployments (i.e. manual jobs) in for_deployed_merge_request.
@ahmed.hemdan This is a great summary and same with my understanding.
I think this is categorized as bugux and there is room to improve it. We'd need UX help to settle the ideal design down at first. For now, I'll explain my thought on this issue:
Post-merge pipeline shouldn't have the same-look environment widget since it's displaying different information, which is "In which environment the MR is live". It should be displayed in a dedicated widget space aside from the current under-pipeline environment widget space.
With that, Post-merge pipeline can restore the original behavior in the under-pipeline environment widget space, that showing related environments to the post-merge pipeline.
I still remember that when Delivery group was working on !20066 (merged), they decided to reuse the existing environment widget space since they didn't have UXer/frontender. I think it's time to properly address the frontend part.
From a backend technical standpoint, it still seems like a small task (same backend-weight1 applies) as we will reuse existing code, provided that UX has weighed in on whether to introduce a dedicated widget for post-merge pipeline as explained above by Shinya.
Shall I keep this around for %15.4 or push it to the next milestone?
Shall we load only existing deployments in the widget? Or also prospective ones? (manual jobs yet to be executed)
Prospective ones used to be displayed. Without that the play button on the environments would never be used. Having staging be auto deploy in default branch and prod be manual is a rather common workflow I'd imagine. Ensuring the dev does not even need to leave the MR to trigger the prod deploy was a big selling point of Gitlab...until it stopped working.
Hey @ahmed.hemdan and @shinya.maeda - thanks so much for the proposal and summary here. Sorry for the delay I was just getting my head wrapped around the problem.
I think this is categorized as bugux and there is room to improve it. We'd need UX help to settle the ideal design down at first. For now, I'll explain my thought on this issue:
I agree I think there is a lot of room for improvement here with consistency. I would also category this as a bugux as I think UX input here would be incredibly helpful.
There should be a dedicated widget for post-merge pipeline, showing "in which environment this MR is live. In that case, we could use the merge pipeline to load deployments for that widget, which will fix this.
I love this idea, it breaks up information and makes the flow easier to understand. To summarize what is being proposed here, it is a fix that would include a newly designed post-merge pipeline widget that does not currently exist. And then we keep the environment widget for the before-merge state since the two show different information. We want this new widget to show information about where the MR is live.
Shall we load only existing deployments in the widget? Or also prospective ones? (manual jobs yet to be executed)
If we do show manual jobs that have yet to be executed we would have to design out a separate state for the widget (waiting?). I agree though I think this should be included, and to @jimmy-outschool's point losing out on what used to exist would be detrimental.
Shall I keep this around for %15.4 or push it to the next milestone?
As mentioned I'll be OOO for the rest of 15.4, so it might be best to push this to 15.5 since it appears this would be in need of mocksups. Thoughts @rayana?
Great analysis of the situation @ahmed.hemdan , only thing I have to add here is I agree with @jimmy-outschool that showing the manual deploy actions seems like it would be very valuable to end users. Showing environments where the MR is currently deployed and the environments where it can potentially be deployed should give them enough information to act upon in the MR.
Thank you everyone for the feedback. I really appreciate it!
Prospective ones used to be displayed. Without that the play button on the environments would never be used. Having staging be auto deploy in default branch and prod be manual is a rather common workflow I'd imagine. Ensuring the dev does not even need to leave the MR to trigger the prod deploy was a big selling point of Gitlab...until it stopped working.
@jimmy-outschool that was my understanding as well, thanks for confirming it: we displayed prospective deployments before and we do not anymore. I agree having it back makes sense and will be helpful for developers in general.
I love this idea, it breaks up information and makes the flow easier to understand. To summarize what is being proposed here, it is a fix that would include a newly designed post-merge pipeline widget that does not currently exist. And then we keep the environment widget for the before-merge state since the two show different information. We want this new widget to show information about where the MR is live.
@emilybauman Thank you for taking the time to weigh in on this before going for vacation.
That's correct. The proposed solution is to have two separate widgets:
Pre-merge environments widget: displays environments where the MR will be deployed / can be deployed.
Post-merge pipeline widget: displays where the MR is currently deployed / can be deployed.
As mentioned I'll be OOO for the rest of 15.4, so it might be best to push this to 15.5 since it appears this would be in need of mocksups. Thoughts @rayana?
I updated the milestone to 15.5, so we have a proper chance to validate the solution and come up with designs, etc.
Great analysis of the situation @ahmed.hemdan , only thing I have to add here is I agree with @jimmy-outschool that showing the manual deploy actions seems like it would be very valuable to end users. Showing environments where the MR is currently deployed and the environments where it can potentially be deployed should give them enough information to act upon in the MR.
@acook.gitlab agreed, having both of pieces of information makes sense to me.
No worries @vshushlin, I appreciate you bringing it up! I agree with prioritizing it in an upcoming milestone as it seems to have customer comments - but open to suggestions from @cbalane as well
To be clear this represents a regression in behavior on which folks already depended. This should be prioritized as such. It feels a bit off this can languish for a year and still be pushed off.
I just found this issue after checking the issues before I'm writing my own on a hard to reproduce environment related problem in our self hosted GitLab. After reading the response from @ahmed.hemdan I'm wondering if this is maybe related...
Could someone take a wild guess about this?
If it's not related some small hints what we should look for in logs on this behavior that we can add to an issue of our own would be very appreciated. Until now we are still searching for the needle in the haystack.
The backstory from us:
Some of the development teams using our GitLab platform are making heavy use of automatic dependency management using renovate with the automerge feature enabled and pipelines for merge requests.
Renovate will check for updates on dependencys within the project every time it runs and create merge requests which will update the dependency versions according to the configuration provided.
This will trigger their merge request pipeline, which defines a total of 9 environments in their pipeline definition. Only 2 environments here are relevant in the context of a merge request.
For exactly these 2 environments the on_stop action is not always executed if a merge request is closed/merged. Sometimes it works and sometimes it doesn't.
We are already banging our heads against this some weeks and have not found any cause or at least a hint in the logs about this. We just see that the on_stop action is not executed. We also cannot reproduce this in a way where this always happens. It's like a traffic sign in these projects.
The projects where we see this behavior are rather old (created <= 2020) and we have seen the first occurence of this problem after the date we updated GitLab to 15.4.
The automerge feature from renovate basically just sets the flag "merge when pipeline succeeds", so we have excluded renovate as the cause. Addtionally we have also seen this happening for merge requests someone created manually within GitLab and setting the "merge when pipeline succeeds" flag.
According to the documentation we also think the pipeline definition is correct.
I did my best to reduce the full pipeline definition to the relevant parts for these 2 environments since I certainly don't want to bother you with the full definition with ~10.000 LoC.
@nagyv-gitlab - I'd love to get your feedback on this proposal. Based on the above conversation we were looking at creating two different environment widgets for pre/post merge. Pre merge would live on its own and show where the MR was deployed, while post merge would live under the pipeline widget similar to what we have now.
Pre-Merge
Post-Merge
I'll have to mock these up in context with a page next, but wanted to get an initial idea down for some early feedback.
@afontaine from a high level what would the frontend need from backend that doesn't exist yet for this change? Do you need a way to ask for "will be deployed" separately from "can be deployed?"
Once we know what frontend will need we could start working on the functionality separately so it's ready.
@hustewart we currently have the ci_environments_status endpoint (app/controllers/projects/merge_requests_controller.rb:322 that takes an optional environment_target status to decide on pre vs post merge deployments. I think, looking at all this discussion, is that the backend needs to (doesn't currently) deliver manual deployment jobs via that endpoint.
The frontendseems to already support showing them via the code (see app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_info.vue:41), and I think we need to ensure the post-merge deployments includes the pre-merge deployments.
I understand there's some UI changes beyond that, but those seem to not need anything new from the backend to support, aside from what's mentioned above (which we should already support.)
I also think there's a bit of trouble here on what pipeline definition we want to show deployments from, whether it's the MR head or the merged result (and what happens if there is no merged result because of a conflict or something), which is probably why this is so inconsistent to begin with.
all that to say: primarily we need the backend to deliver manual, not-started deployments to the frontend. That seems to be the critical missing piece.
Ah but - yeah it probably makes sense to just create a separate backend issue to do that since this issue has other considerations. I'll create one this week and link to it.
It looks like we have everything to start working on this issue, but we want to first deliver the backend change, so I'll mark this issue as workflowready for development and blocked by #456551.