Skip to content

progress: Show progress of replicated tasks before they are assigned#237

Merged
thaJeztah merged 1 commit intodocker:masterfrom
aaronlehmann:progress-before-assignment
Jun 27, 2017
Merged

progress: Show progress of replicated tasks before they are assigned#237
thaJeztah merged 1 commit intodocker:masterfrom
aaronlehmann:progress-before-assignment

Conversation

@aaronlehmann
Copy link

@aaronlehmann aaronlehmann commented Jun 24, 2017

This was only showing tasks that belong to nodes that are currently up,
so that tasks on down nodes don't appear to be stuck. But this
unintentionally excludes tasks that haven't been assigned yet, so if a
task is stuck before assignment, for example because no nodes meet its
constraints, a progress bar won't even be shown. The check should only
apply to tasks that have a node assignment.

Related to moby/moby#33807

cc @thaJeztah

This was only showing tasks that belong to nodes that are currently up,
so that tasks on down nodes don't appear to be stuck. But this
unintentionally excludes tasks that haven't been assigned yet, so if a
task is stuck before assignment, for example because no nodes meet its
constraints, a progress bar won't even be shown. The check should only
apply to tasks that have a node assignment.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@codecov-io
Copy link

codecov-io commented Jun 24, 2017

Codecov Report

Merging #237 into master will decrease coverage by 1.52%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #237      +/-   ##
==========================================
- Coverage    47.2%   45.67%   -1.53%     
==========================================
  Files         171      171              
  Lines       11556    11513      -43     
==========================================
- Hits         5455     5259     -196     
- Misses       5790     5947     +157     
+ Partials      311      307       -4

@thaJeztah
Copy link
Member

Without this patch;

$ docker service create --name donotdeploy --constraint "node.labels.somelabel==foobar" --detach=false nginx:alpine
0wx8h7rp50tqg0xnykwbxy4n6
overall progress: 0 out of 1 tasks
1/1:

With this patch applied;

./docker service create --name donotdeploy --constraint "node.labels.somelabel==foobar" --detach=false nginx:alpine
ve5pnzf0swqtapeb0uqnla2cz
overall progress: 0 out of 1 tasks
1/1: pending   [================>                                  ]

@thaJeztah
Copy link
Member

After applying the label;

$ docker service create --name donotdeploy --constraint "node.labels.somelabel==foobar" --detach=false nginx:alpine
ve5pnzf0swqtapeb0uqnla2cz
overall progress: 1 out of 1 tasks
1/1: running   [==================================================>]
verify: Waiting 1 seconds to verify that tasks are stable...

guess I found another issue (verify: Waiting 1 seconds to verify that tasks are stable... isn't remove after it finishes), let me open an issue for that 😄

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah thaJeztah added this to the 17.06.1 milestone Jun 24, 2017
Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐯

@thaJeztah
Copy link
Member

All green 👍

@thaJeztah thaJeztah merged commit 2f58992 into docker:master Jun 27, 2017
nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
[17.09] Add more information to the generated YAML for documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants