Skip to content

Commit

Permalink
UX: don't reverse progress-bars when rolling back
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@thaJeztah
thaJeztah committed Jun 22, 2021
1 parent 12e8782 commit 678c2fd
Showing 1 changed file with 6 additions and 13 deletions.
19 changes: 6 additions & 13 deletions cli/command/service/progress/progress.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
})
Expand Down Expand Up @@ -463,7 +456,7 @@ func (u *globalProgressUpdater) update(service swarm.Service, tasks []swarm.Task
running++
}

u.writeTaskProgress(task, nodeCount, rollback)
u.writeTaskProgress(task, nodeCount)
}
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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,
})
Expand Down

0 comments on commit 678c2fd

Please sign in to comment.