From 678c2fde9831d9c59cfd89cb03445d369a297d19 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 19 Jan 2021 14:41:51 +0100 Subject: [PATCH] UX: don't reverse progress-bars when rolling back 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 --- cli/command/service/progress/progress.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/cli/command/service/progress/progress.go b/cli/command/service/progress/progress.go index 3ed41cb9e4..dc95a02a6a 100644 --- a/cli/command/service/progress/progress.go +++ b/cli/command/service/progress/progress.go @@ -67,13 +67,6 @@ func terminalState(state swarm.TaskState) bool { return numberedStates[state] > numberedStates[swarm.TaskStateRunning] } -func stateToProgress(state swarm.TaskState, rollback bool) int64 { - if !rollback { - return numberedStates[state] - } - return numberedStates[swarm.TaskStateRunning] - numberedStates[state] -} - // ServiceProgress outputs progress information for convergence of a service. // nolint: gocyclo func ServiceProgress(ctx context.Context, client client.APIClient, serviceID string, progressWriter io.WriteCloser) error { @@ -343,7 +336,7 @@ func (u *replicatedProgressUpdater) update(service swarm.Service, tasks []swarm. running++ } - u.writeTaskProgress(task, mappedSlot, replicas, rollback) + u.writeTaskProgress(task, mappedSlot, replicas) } if !u.done { @@ -389,7 +382,7 @@ func (u *replicatedProgressUpdater) tasksBySlot(tasks []swarm.Task, activeNodes return tasksBySlot } -func (u *replicatedProgressUpdater) writeTaskProgress(task swarm.Task, mappedSlot int, replicas uint64, rollback bool) { +func (u *replicatedProgressUpdater) writeTaskProgress(task swarm.Task, mappedSlot int, replicas uint64) { if u.done || replicas > maxProgressBars || uint64(mappedSlot) > replicas { return } @@ -406,7 +399,7 @@ func (u *replicatedProgressUpdater) writeTaskProgress(task swarm.Task, mappedSlo u.progressOut.WriteProgress(progress.Progress{ ID: fmt.Sprintf("%d/%d", mappedSlot, replicas), Action: fmt.Sprintf("%-[1]*s", longestState, task.Status.State), - Current: stateToProgress(task.Status.State, rollback), + Current: numberedStates[task.Status.State], Total: maxProgress, HideCounts: true, }) @@ -463,7 +456,7 @@ func (u *globalProgressUpdater) update(service swarm.Service, tasks []swarm.Task running++ } - u.writeTaskProgress(task, nodeCount, rollback) + u.writeTaskProgress(task, nodeCount) } } @@ -507,7 +500,7 @@ func (u *globalProgressUpdater) tasksByNode(tasks []swarm.Task) map[string]swarm return tasksByNode } -func (u *globalProgressUpdater) writeTaskProgress(task swarm.Task, nodeCount int, rollback bool) { +func (u *globalProgressUpdater) writeTaskProgress(task swarm.Task, nodeCount int) { if u.done || nodeCount > maxProgressBars { return } @@ -524,7 +517,7 @@ func (u *globalProgressUpdater) writeTaskProgress(task swarm.Task, nodeCount int u.progressOut.WriteProgress(progress.Progress{ ID: stringid.TruncateID(task.NodeID), Action: fmt.Sprintf("%-[1]*s", longestState, task.Status.State), - Current: stateToProgress(task.Status.State, rollback), + Current: numberedStates[task.Status.State], Total: maxProgress, HideCounts: true, })