Show that a deploy is running in MR

what about multiple deploys within the same MR pipeline? Say to staging, production and review app?

should we refer to the build within the pipeline: Build #12345678 in pipeline #12345678 ...

Let's begin with:

if it hasn't deployed yet: Pipeline __#12345678__ is creating a deployment to __production__ and will overwrite the __latest deployment__

if not merged, has deploy active: Pipeline __#12345678__ is the most recent deployment to __production__

if not merged, did deploy, but failed: The deployment of this pipeline #12345678 to __production__ did not complete

if merged but still has a deploy active: Pipeline __#12345678__ still has an active deployment to __production__

Perhaps with the changes from https://gitlab.com/gitlab-org/gitlab-ce/issues/25072 in there as well.

@markpundsack what do you think?

@dimitrieh I think your suggestions work. I'm not positive about the last one. What does "still has an active deployment" mean?

I don't think we need to include build numbers or the "will overwrite the latest deployment" parts. The important thing is the environment. But if we want to include that for consistency, that's fine.

I'd like to differentiate when the pipeline will deploy, if everything else passes, vs is deploying. Pipeline [#123] may deploy to [staging] vs Pipeline [#123] is deploying to [staging]. Maybe even include the text "if everything else passes" or some other phrasing to clarify what this means. A more technically correct, but ugly, phrase would be Pipeline [#123] contains a deployment to [staging] that will run if previous stages succeed.

If there are multiple environments, perhaps we only include those that aren't manual, unless the manual trigger has been triggered. In practice, that will only be one environment at a time, but if for some reason someone configures multiple deploys, we should handle it.

@markpundsack

I'm not positive about the last one. What does "still has an active deployment" mean?

I thought it would notify of this:

But once the MR is merged, that pipeline notice goes away. But a deploy of that MRs code may still be ongoing

I don't think we need to include build numbers or the "will overwrite the latest deployment" parts. The important thing is the environment. But if we want to include that for consistency, that's fine.

I agree :) as you are always seeing the latest pipeline.

If there are multiple environments, perhaps we only include those that aren't manual, unless the manual trigger has been triggered. In practice, that will only be one environment at a time, but if for some reason someone configures multiple deploys, we should handle it.

Seems good, we can just list them under each other in those cases?

So with your notes we arrive at:

if it hasn't deployed yet, but hasn't arrived at deploy build: Pipeline __#12345678__ may deploy to __production__ if it passes all required prior builds/stages

if it hasn't deployed yet and has arrived at deploy build: Pipeline __#12345678__ is deploying to __production__

if not merged, has deploy active: Pipeline __#12345678__ is the most recent deployment to __production__

if not merged, did deploy, but failed: The deployment to __production__ of pipeline #12345678 did not complete

if merged but still has a deploy active: Pipeline __#12345678__ still has an active deployment to __production__

mentioned in issue gitlab-foss#24717 (moved)

added deploy in GitLab FOSS label

@dimitrieh OK, I finally get what the last one is about. I think it's a bit more complicated though. :)

There are two situations where a deploy would run after a merge.

Closing a review app,
A pipeline running on the target branch.

For (1), perhaps we need to treat it as a special case and say Pipeline [#123] is closing the `review/foo` environment.

For (2), it probably has the same number of permutations as the regular pipeline, but the difference is that we should make it clear that the pipeline we're referring to is running on the target branch, no the source branch. e.g. Pipeline [#123] running on `master` will deploy to [staging] if prior stages succeed. We might have to make it even clearer why that's displayed. Like "...will deploy this merge request to ..." or "will deploy the code in this merge request to...". But since we aren't that verbose after it's deployed and that hasn't been a problem, perhaps it's not needed.

Also, just realized that currently, merge requests show pipeline information on a separate line from deploy information. So how about:

After first push:

Pipeline [#5277805] running for [12345668].
Will deploy to [review/br-add-onboarding-101] when prior stages succeed.
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

Pipeline [#5277805] running for [12345668].
Deploying to [review/br-add-onboarding-101].
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

After subsequent push:

Pipeline [#5277805] running for [12345668].
Will deploy to [review/br-add-onboarding-101] when prior stages succeed. [View on br-add-onboarding-101.about.gitlab.com]  [Stop environment]
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

Pipeline [#5277805] running for [12345668].
Deploying to [review/br-add-onboarding-101]. [View on br-add-onboarding-101.about.gitlab.com]  [Stop environment]
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

Deployed, before merge:

Pipeline [#5277821] passed for [6ab1ac74].
Deployed to [review/br-add-onboarding-101] 17 minutes ago. [View on br-add-onboarding-101.about.gitlab.com]  [Stop environment]
[Accept Merge Request] The source branch will be removed.  [Modify commit message]

After merge:

Pipeline [#5277821] passed for [6ab1ac74].
Closing [review/br-add-onboarding-101].
Related pipeline [#6324356] on `master` will deploy to [staging] when prior stages succeed. [View on staging.about.gitlab.com]
Merged by Mark Pundsack 5 days ago
The changes were merged into master. The source branch has been removed.
[Revert] [Cherry-pick]

Pipeline [#5277821] passed for [6ab1ac74].
Related pipeline [#6324356] on `master` is deploying to [staging]. [View on staging.about.gitlab.com]
Merged by Mark Pundsack 5 days ago
The changes were merged into master. The source branch has been removed.
[Revert] [Cherry-pick]

Pipeline [#5277821] passed for [6ab1ac74].
Deployed to [staging]. [View on staging.about.gitlab.com]
Merged by Mark Pundsack 5 days ago
The changes were merged into master. The source branch has been removed.
[Revert] [Cherry-pick]

I didn't like starting two lines with Pipeline [#num] so I went with Related pipeline`. But we don't currently say that for the final deploy state. I think that's OK, since at that point it's less likely I need to watch the deploy log, and I'm more interested in the final result. e.g. the environment URL.

Note, I dropped the Stop environment part for staging. Technically, we may still have that, but writing it out made me realize how dumb it is to have an option on a merge request to delete staging or production. There should be some way to protect those environments from accidental deletes. More likely, you just won't define a stop job for those environments, thus we won't include a link.

/cc @victorwu

@markpundsack + @dimitrieh: Is there any intentional design on the pipeline message in the MR info box? And say, how it integrates with the info in the pipelines tab lower in the MR page?

All (?) the info is already available in the pipelines tab. So what's the thinking there? Can we leverage that tab of info for something like this?

For example, when you click a link in the MR info box, GitLab navigates to the pipelines page where you can see all pipelines, but you lose the MR context. Is that a conscious design, versus, say just opening the pipelines tab in the MR and maybe scrolling down?

Here the same info is in the box and in the pipelines tab.

@victorwu

Is there any intentional design on the pipeline message in the MR info box? And say, how it integrates with the info in the pipelines tab lower in the MR page?

Can you elaborate what exactly you propose? As up until now we have adjusted each previous version into something that better suited are needs. I do however think the MR widget can use some more complete overhaul, as its outgrowing its confines with all the additions and changes currently happening.

For example, when you click a link in the MR info box, GitLab navigates to the pipelines page where you can see all pipelines

I think you refer to where you can see all the "builds"? Because that info will be in the context of the MR soon with: https://gitlab.com/gitlab-org/gitlab-ce/issues/23785

Here the same info is in the box and in the pipelines tab.

The pipelines tab shows all pipelines within that merge request. The mr widget shows only the latest one which is the most important one in many cases, often saving a click.

assigned to @dimitrieh

mentioned in issue gitlab-foss#25424 (closed)

@markpundsack I am okey with the changes you proposed :)

More likely, you just won't define a stop job for those environments, thus we won't include a link.

Can't we propose a change that this option is not available for static environments? As you create them manually, you can only delete them manually?

Last thing that bugs me is that I don't like these review app urls. They can become very long as they are created from the branch name. We should perhaps create/show a short-url, resulting in:

View on staging.url

or for review apps:

View on review.url

mentioned in issue gitlab-foss#33206 (moved)

Scheduling

@markpundsack @bikebilly @victorwu can we correctly schedule this issue? (see my proposal below, to indicate how you think about it)

I will be unassigning for now, until this is properly scheduled

Taking another look at this issue

I want to once again take a look at this issue (as it was still assigned to me), as I think it still holds a lot of potential and shows a possible flaw that still resides in the MR widget.

The flaw that I am talking about is that a deployment is a first level item inside the merge request widget, rather than a effect of another first level element, such as a pipeline or merge.

With the changes from https://gitlab.com/gitlab-org/gitlab-ce/issues/33095 we are shifting the merge request widget to have multiple status icons (which all need to be green.. in order to proceed with merging).

The following areas should indicate if a merge is possible. Top to bottom, and a special error that requires text at the item of merge itself so to say.

To illustrate my point of the flaw here some more. The first top level item, the pipeline, runs on the latest commit within that MR. This can stop the merge, plus it can set a deploy in motion.

However the next item is the deploy.. which is a similar item to the pipeline, while it cannot stop the merge, plus it is an effect of that pipeline above it. The new merge request design allows for a little bit of hierarchy. With this we could put it under the pipeline, rather than it being a first level item.

Also https://gitlab.com/gitlab-org/gitlab-ce/issues/25140#note_19526503 tells us there are only a few statuses a deploy can have: will deploy, deploying, deployed, and failed to deploy (can we see this? @markpundsack). I don't think this needs a status icon perse!

New mockups after previous review section

So with the above statements being true, I would like to mockup most of the possibilities described by @markpundsack at https://gitlab.com/gitlab-org/gitlab-ce/issues/25140#note_19526503

After first push:

Pipeline [#5277805] running for [12345668].
Will deploy to [review/br-add-onboarding-101] when prior stages succeed.
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

Pipeline [#5277805] running for [12345668].
Deploying to [review/br-add-onboarding-101].
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

After subsequent push:

Pipeline [#5277805] running for [12345668].
Will deploy to [review/br-add-onboarding-101] when prior stages succeed. [View on br-add-onboarding-101.about.gitlab.com]  [Stop environment]
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

Pipeline [#5277805] running for [12345668].
Deploying to [review/br-add-onboarding-101]. [View on br-add-onboarding-101.about.gitlab.com]  [Stop environment]
[Accept when pipeline succeeds] The source branch will be removed.  [Modify commit message]

Deployed, before merge:

Pipeline [#5277821] passed for [6ab1ac74].
Deployed to [review/br-add-onboarding-101] 17 minutes ago. [View on br-add-onboarding-101.about.gitlab.com]  [Stop environment]
[Accept Merge Request] The source branch will be removed.  [Modify commit message]

After merge:

Pipeline [#5277821] passed for [6ab1ac74].
Closing [review/br-add-onboarding-101].
Related pipeline [#6324356] on `master` will deploy to [staging] when prior stages succeed. [View on staging.about.gitlab.com]
Merged by Mark Pundsack 5 days ago
The changes were merged into master. The source branch has been removed.
[Revert] [Cherry-pick]

Pipeline [#5277821] passed for [6ab1ac74].
Related pipeline [#6324356] on `master` is deploying to [staging]. [View on staging.about.gitlab.com]
Merged by Mark Pundsack 5 days ago
The changes were merged into master. The source branch has been removed.
[Revert] [Cherry-pick]

Pipeline [#5277821] passed for [6ab1ac74].
Deployed to [staging]. [View on staging.about.gitlab.com]
Merged by Mark Pundsack 5 days ago
The changes were merged into master. The source branch has been removed.
[Revert] [Cherry-pick]

Additional mockups for prometheus

To further illustrate why I think this is the right direction, I will provide some mockups that show such situations with prometheus data present.

Additional notes

Note, I dropped the Stop environment part for staging. Technically, we may still have that, but writing it out made me realize how dumb it is to have an option on a merge request to delete staging or production. There should be some way to protect those environments from accidental deletes. More likely, you just won't define a stop job for those environments, thus we won't include a link.

@markpundsack On a sidenote, i think this still deserves an issue, correct? These changes are in the mockups btw!

removed assignee

labeling UX ready in GitLab FOSS for now

added gitlab-foss~~2024186 gitlab-foss~~127616 and removed UX in GitLab FOSS labels

@dimitrieh Thanks for the updated mockups. Some thoughts:

Note, I dropped the Stop environment part for staging. Technically, we may still have that, but writing it out made me realize how dumb it is to have an option on a merge request to delete staging or production. There should be some way to protect those environments from accidental deletes. More likely, you just won't define a stop job for those environments, thus we won't include a link.

On a sidenote, i think this still deserves an issue, correct? These changes are in the mockups btw!

I don't think we need an issue. People just won't define a stop job for staging and production, so it's a non-issue. Removal from the mockups was all that mattered.

Putting the post-merge "will deploy to staging" message under the merge button is pretty cool, but I think it's not technically feasible. We only know about potential deployments for pipelines that are instantiated (e.g. created or running), and the deploy to staging will be in a not-yet-existent pipeline.

Likewise, merge requests can get deployed to various environments in many ways. Picking a convoluted example, you could have a MR that has not been merged, but then another branch is based on that MR, and is deployed to some environment (like a review app, or even possibly staging). Where would that deployment message go in the first MRs widget? It's not associated with any pipeline, it's just an independent fact. That's likely why we made it a separate top-level concern in the MR widget in the first place.

I'm not sure how to reconcile that.

I was going to suggest that, at least today, all deployments are successful, otherwise they're not shown, thus you could still have a check that contributes to the validity of the merge action. But you've suggested above that we should included a message about failed deployments, and that's actually a good idea, but then leaves this hard to reconcile as the failed deployment may not block the merge. If it failed as part of the primary pipeline, and wasn't allowed to fail, then the pipeline itself would block the merge.

A couple of other issues to consider:

Link pipelines via merge commit: https://gitlab.com/gitlab-org/gitlab-ce/issues/33206

This might let us show the staging deployment as related to the pipeline because it would now be linked. Doesn't help for the more abstract case though.

Review App links to changed page: https://gitlab.com/gitlab-org/gitlab-ce/issues/33418

Just more nested information to include. If deployments were under pipelines, then this would add a third level, but not unlike the prometheus metrics proposed above.

@dimitrieh Also, I'd like to see how this interrelates with statuses on other pages, like branch and possibly commit status. We've got issues elsewhere about showing the deployment status on a branch, which is especially useful for the I2P online development flow. e.g. create a new branch, see the review app for it right there, be able to terminal into it, etc. It would be nice if we had a DRY solution.

but then another branch is based on that MR, and is deployed to some environment (like a review app, or even possibly staging). Where would that deployment message go in the first MRs widget?

@markpundsack what does another branch have to do with the current mr and its branch? I feel a little meeting would make this much clearer.. instead of putting it in words :)

but then leaves this hard to reconcile as the failed deployment may not block the merge. If it failed as part of the primary pipeline, and wasn't allowed to fail, then the pipeline itself would block the merge.

This is exactly why I don't want the deployment to be a top level item.

@dimitrieh Also, I'd like to see how this interrelates with statuses on other pages, like branch and possibly commit status. We've got issues elsewhere about showing the deployment status on a branch, which is especially useful for the I2P online development flow. e.g. create a new branch, see the review app for it right there, be able to terminal into it, etc. It would be nice if we had a DRY solution.

I would like for us to see these examples separate from the merge request widget. They are different views, thus different needs are met :) (the items don't have to add up to some sort of merge action as they do in the mr view). Still they could probably borrow similar logic visually.

changed milestone to %Next 2-3 months in gitlab-foss

@dimitrieh Now that I've got this in my head, and I'm running through I2P demos again, I can't help but think this would be a really important change. Having to navigate from the MR page to the project Pipelines page, just to see what's happening after I click Merge is just dumb. I should see the pipeline right there.

@bikebilly I put this as Next 2-3 months. Hopefully it's a relatively small thing.

moved from gitlab-foss#25140 (moved)

Show that a deploy is running in MR

Links

Designs

Child items ...

Activity

Scheduling

Taking another look at this issue

New mockups after previous review section

Additional mockups for prometheus

Additional notes

Admin message

Admin message

Show that a deploy is running in MR

Links

Activity

Scheduling

Taking another look at this issue

New mockups after previous review section

Additional mockups for prometheus

Additional notes