UX: don't reverse progress-bars when rolling back #2940
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thaJeztah
force-pushed
the
rollback_progress_bars
branch
from
2dee5b2 to
11d9491
Compare
Contributor
|
@thaJeztah linter is failing: |
Member
Author
Ah, dang. I separated this from the other PR, probably forgot to remove some things; I'll have a look |
thaJeztah
force-pushed
the
rollback_progress_bars
branch
from
11d9491 to
8039b6d
Compare
Member
Author
|
Think it should be fixed now |
Codecov Report
@@ Coverage Diff @@
## master #2940 +/- ##
==========================================
- Coverage 57.11% 57.11% -0.01%
==========================================
Files 299 299
Lines 18666 18663 -3
==========================================
- Hits 10661 10659 -2
+ Misses 7145 7144 -1
Partials 860 860 |
Member
Author
|
@tiborvass @cpuguy83 ptal |
Commit https://github.com/moby/moby//commit/330a0035334871d92207b583c1c36d52a244753f introduced "synchronous" service update and rollback, using progress bars to show current status for each task. As part of that change, progress bars were "reversed" when doing a rollback, to indicate that status was rolled back to a previous state. Reversing direction is somewhat confusing, as progress bars now return to their "initial" state to indicate it was "completed"; for an "automatic" rollback, this may be somewhat clear (progress bars "move to the right", then "roll back" if the update failed), but when doing a manual rollback, it feels counter-intuitive (rolling back is the _expected_ outcome). This patch removes the code to reverse the direction of progress-bars, and makes progress-bars always move from left ("start") to right ("finished"). Before this patch ---------------------------------------- 1. create a service with automatic rollback on failure $ docker service create --update-failure-action=rollback --name foo --tty --replicas=5 nginx:alpine 9xi1w3mv5sqtyexsuh78qg0cb overall progress: 5 out of 5 tasks 1/5: running [==================================================>] 2/5: running [==================================================>] 3/5: running [==================================================>] 4/5: running [==================================================>] 5/5: running [==================================================>] verify: Waiting 2 seconds to verify that tasks are stable... 2. update the service, making it fail after 3 seconds $ docker service update --entrypoint="/bin/sh -c 'sleep 3; exit 1'" foo overall progress: rolling back update: 2 out of 5 tasks 1/5: running [==================================================>] 2/5: running [==================================================>] 3/5: starting [============================================> ] 4/5: running [==================================================>] 5/5: running [==================================================>] 3. Once the service starts failing, automatic rollback is started; progress-bars now move in the reverse direction; overall progress: rolling back update: 3 out of 5 tasks 1/5: ready [===========> ] 2/5: ready [===========> ] 3/5: running [> ] 4/5: running [> ] 5/5: running [> ] 4. When the rollback is completed, the progressbars are at the "start" to indicate they completed; overall progress: rolling back update: 5 out of 5 tasks 1/5: running [> ] 2/5: running [> ] 3/5: running [> ] 4/5: running [> ] 5/5: running [> ] rollback: update rolled back due to failure or early termination of task bndiu8a998agr8s6sjlg9tnrw verify: Service converged After this patch ---------------------------------------- Progress bars always go from left to right; also in a rollback situation; After updating to the "faulty" entrypoint, task are deployed: $ docker service update --entrypoint="/bin/sh -c 'sleep 3; exit 1'" foo foo overall progress: 1 out of 5 tasks 1/5: 2/5: running [==================================================>] 3/5: ready [======================================> ] 4/5: 5/5: Once tasks start failing, rollback is started, and presented the same as a regular update; progress bars go from left to right; overall progress: rolling back update: 3 out of 5 tasks 1/5: ready [======================================> ] 2/5: starting [============================================> ] 3/5: running [==================================================>] 4/5: running [==================================================>] 5/5: running [==================================================>] rollback: update rolled back due to failure or early termination of task c11dxd7ud3d5pq8g45qkb4rjx Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
rollback_progress_bars
branch
from
8039b6d to
678c2fd
Compare
Codecov Report
@@ Coverage Diff @@
## master #2940 +/- ##
==========================================
- Coverage 57.09% 57.09% -0.01%
==========================================
Files 299 299
Lines 18731 18728 -3
==========================================
- Hits 10695 10693 -2
+ Misses 7166 7165 -1
Partials 870 870 |
Member
Author
|
Rebased to get a fresh run of CI |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit https://github.com/moby/moby//commit/330a0035334871d92207b583c1c36d52a244753f (moby/moby#31144) introduced "synchronous" service update and rollback, using progress bars to show current status for each task.
As part of that change, progress bars were "reversed" when doing a rollback, to indicate that status was rolled back to a previous state.
Reversing direction is somewhat confusing, as progress bars now return to their "initial" state to indicate it was "completed"; for an "automatic" rollback, this may be somewhat clear (progress bars "move to the right", then "roll back" if the update failed), but when doing a manual rollback, it feels counter-intuitive (rolling back is the expected outcome).
This patch removes the code to reverse the direction of progress-bars, and makes progress-bars always move from left ("start") to right ("finished").
Before this patch
create a service with automatic rollback on failure
update the service, making it fail after 3 seconds
Once the service starts failing, automatic rollback is started; progress-bars now move in the reverse direction;
When the rollback is completed, the progressbars are at the "start" to indicate they completed;
After this patch
Progress bars always go from left to right; also in a rollback situation;
After updating to the "faulty" entrypoint, task are deployed:
Once tasks start failing, rollback is started, and presented the same as a regular
update; progress bars go from left to right;
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)