Demo: https://asciinema.org/a/4btgvuk1dv3z8zn0y882gqph5

@aaronlehmann what happens if the service created can't be scheduled, because of constraint that cannot be fullfilled for example ? How should it behave ?

I like the feature but it would be a huge breaking change. It feels a bit like the docker-compose up vs docker-compose up -d debate.

I can see use-cases for this, but should we also attach to the service logs after the service was deployed (like docker-compose up)?

Let me add this to the next maintainers meeting to get more eyes / open the discussion

what happens if the service created can't be scheduled, because of constraint that cannot be fullfilled for example ? How should it behave ?

You would see one or more of the tasks stuck at "pending". This synchronous mode gives visibility into the problem, which would otherwise require a "service ps" command to notice.

You would see one or more of the tasks stuck at "pending".

Would it output the error messages / reason for being unable to deploy the task?

Not at present, but that's definitely possible.

+1 for the feature, -1 for changing the default immediately.

I think we need to put it behind a flag for now. We can add a warning when no flag is used, and change the default in a release or two.

Changing the default is going to break people who use this command in scripts.

We should also look at this for docker stack deploy, to show progress, and ideally show logs as well for a stack (to be more like docker-compose)

We discussed this in the maintainers meeting, and like the feature, but (for now) to not be the default.

Huge +1

It's also going to "fix" the workflow for many people that currently do a service create then just a bunch of service ps and then have to inspect tasks to figure out what happened.

Or worse - people that just do service create. Contrary to docker run, a service is not immediately ready and service creation is fully asynchronous. So the service is not responding, the port doesn't work, and the rollout may have failed.

UX Feedback:

• Everything should be aligned. I think the state should be padded to the longest possible value so the progress bars don't "shift" to the left/right when the state changes
• What's the XB/XB at the end?
• Waiting to verify that tasks are stable - this is a bit non obvious, some rewording might help. Also, could we have a countdown? e.g. Waiting X seconds
• Not sure if overall needs its own progress bar. By definition it has a slightly different meaning than the others. On first look it seems like a slow task. Maybe plan text would be better, e.g. Updating service <foo> (2 out of 12 completed)
• What's interesting is error handling here. Could you link to a demo? I think the interesting bits of information are the overall status (the one in UpdateStatus - which I think should be part of the first line), the task error (can we show it somewhere?) and the rollback/pause status
• When the user Ctrl-C's the deployment, we should say that it's continuing in the background
• When we rollback, we should print the error message etc
• When we pause, we should say why and how to continue

Looks amazing so far!

Looks awesome.

• Should Ctrl-C rollback the deployment?
• I think this going on by default might be too much, but an --interactive flag sounds good.

I'm with @dnephin: huge 👍, but I wouldn't change the default.

Nice work!

Regarding re-usability of the service progressbar for other clients that potentially want to display service updates (eg, plugin install), it is better to extract cli/command/service/progress.go into a separate package.

Thanks for all the comments. I've updated this to address the feedback. Details below, and new demo here: https://asciinema.org/a/9f9rgk0e7fczkxi5vfu7yofmw

I think we need to put it behind a flag for now. We can add a warning when no flag is used, and change the default in a release or two.

I've changed the behavior so the default is unchanged, and the -i / --interactive flag is required to see progress info. One thought I had is that if we only enable this by default when the command is running on a TTY, it's less likely we would break scripts. But I'm also okay with putting this behind an option, for now. It also means there's less urgency in fixing the technical implementation issues described in the PR description (though I would like to fix at least some of them before merging).

Everything should be aligned. I think the state should be padded to the longest possible value so the progress bars don't "shift" to the left/right when the state changes

Done.

What's the XB/XB at the end?

This was an artifact of the progress bar code. I've now added an option that suppresses it.

Waiting to verify that tasks are stable - this is a bit non obvious, some rewording might help. Also, could we have a countdown? e.g. Waiting X seconds

Changed this so it has a countdown now.

Not sure if overall needs its own progress bar. By definition it has a slightly different meaning than the others. On first look it seems like a slow task. Maybe plan text would be better, e.g. Updating service (2 out of 12 completed)

Changed to plain text

What's interesting is error handling here. Could you link to a demo? I think the interesting bits of information are the overall status (the one in UpdateStatus - which I think should be part of the first line), the task error (can we show it somewhere?) and the rollback/pause status

Demo linked above. I've done some more testing and refinement of this now. Unfortunately it's not especially straightforward to show the task error, because the ID of the task that paused an update is inside a textual message. But we can figure something out if this would be a big usability win.

When the user Ctrl-C's the deployment, we should say that it's continuing in the background

Done

When we rollback, we should print the error message etc

Blocked on #31108

When we pause, we should say why and how to continue

It says why. I'm not sure about the "how to continue" part. Any suggestions for what the message should say?

Regarding re-usability of the service progressbar for other clients that potentially want to display service updates (eg, plugin install), it is better to extract cli/command/service/progress.go into a separate package.

Moved to github.com/docker/docker/cli/command/service/progress.

It says why. I'm not sure about the "how to continue" part. Any suggestions for what the message should say?

docker service update <foo> should continue the paused deployment right?

Regarding the verification time - how are we picking the 45 seconds?

Also, is this looking at task status or update status? As in, do we take into account the rollback threshold?

docker service update <foo> should continue the paused deployment right?

Yeah, but I'm not sure we want to recommend that, because usually there's an underlying problem that should be fixed.

Regarding the verification time - how are we picking the 45 seconds?

I set --update-monitor 45s when I created the service, to demonstrate failures (by manually stopping some containers). The default is 5s.

Also, is this looking at task status or update status? As in, do we take into account the rollback threshold?

Sort of a mix of both. Basically, if the update pauses or rolls back, we'll see that in the update status. But the progress bars and countdown are driven by task status.

Looks great :)

Do we have some CLI designers in the house? @dnephin feedback?

Design LGTM

Hi
Is it possible to manage "docker service rm" action too ?
And what about "docker stack deploy/rm" command which launch multiple "docker service" actions ?
Thanx for reading

Thanks everyone!

Is there a lil asciinema or GIF of this?

@nathanleclaire: https://asciinema.org/a/9f9rgk0e7fczkxi5vfu7yofmw (slightly out of date, but mostly the same as what was merged)

Hey guys, per @vieux request Play With Docker has been updated with this patch. You can play with it there. Let me know in case you need me to change anything.

@aaronlehmann Great to see this merged 👍 Is there going to be a follow-up for docker service scale? I know that I can do docker service update --detach=false --replicas=5, but docker service scale allows scaling multiple services more than one service at a time, which is potentially a more complex operation to show than updating a single service.

Also, a minor detail: --replicas 5, --name my_service, etc. work, but --detach false does not; only --detach=false. Is this intentional?

but --detach false does not

Could it be that (because false is a built-in / shell command), that the shell evaluates false if there's no = in between?

@thaJeztah No, that would require me putting false inside backticks or a subshell. I have checked, and the --disable-content-trust argument to e.g. docker run behaves the same. I guess the difference between boolean and string arguments is that boolean arguments will be interpreted as set to true if specified on the command line, requiring an explicit =false to set them to false.

@sirlatrom interesting; could you open an issue for that, so that it can be looked into? It should be either fixed, or at least documented

@thaJeztah I believe this is how the go flags package (or whatever is being used in the Docker CLI) works, so it's really just working as designed. The --interactive/-i and --tty/-t flags to docker run work the same way, and one can specify e.g. --interactive=false (which is the default in that case).

It's because boolean flags can take an optional argument. --name requires an argument, so in --name my_service, my_service can be assumed to be the argument. But --detach is valid on its own, so false could be an entirely separate argument.

@marcosnils Cool!

This looks great, thank you.

Just a quick one if anyone is aware - are there any plans to integrate this with

docker stack deploy

which in a nutshell creates/updates a collection of services?

Thanks again!

@ozlatkov see #32367

@thaJeztah - Thank you.