From 1ef585f65dd0079a22286ec404940685baa1f80b Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Tue, 27 Jun 2017 14:57:47 -0700
Subject: [PATCH] progress: Show task error in place of progress bar

If a task encounters an error, the interactive "service create" and
"service update" commands should show that error instead of showing a
stuck progress bar.

To validate:

docker service create --detach=false --name broken --restart-condition=none --replicas 3 busybox asdf
and
docker service create --detach=false --name broken --mode global --restart-condition none busybox asdf

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
---
 cli/command/service/progress/progress.go | 71 ++++++++++++++++++++----
 1 file changed, 61 insertions(+), 10 deletions(-)

diff --git a/cli/command/service/progress/progress.go b/cli/command/service/progress/progress.go
index d8300ce8d2..7c3f751a68 100644
--- a/cli/command/service/progress/progress.go
+++ b/cli/command/service/progress/progress.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"os/signal"
+	"strings"
 	"time"
 
 	"github.com/docker/docker/api/types"
@@ -29,6 +30,13 @@ var (
 		swarm.TaskStateReady:     7,
 		swarm.TaskStateStarting:  8,
 		swarm.TaskStateRunning:   9,
+
+		// The following states are not actually shown in progress
+		// output, but are used internally for ordering.
+		swarm.TaskStateComplete: 10,
+		swarm.TaskStateShutdown: 11,
+		swarm.TaskStateFailed:   12,
+		swarm.TaskStateRejected: 13,
 	}
 
 	longestState int
@@ -45,17 +53,21 @@ type progressUpdater interface {
 
 func init() {
 	for state := range numberedStates {
-		if len(state) > longestState {
+		if !terminalState(state) && len(state) > longestState {
 			longestState = len(state)
 		}
 	}
 }
 
+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 int64(len(numberedStates)) - numberedStates[state]
+	return numberedStates[swarm.TaskStateRunning] - numberedStates[state]
 }
 
 // ServiceProgress outputs progress information for convergence of a service.
@@ -230,6 +242,18 @@ func writeOverallProgress(progressOut progress.Output, numerator, denominator in
 	})
 }
 
+func truncError(errMsg string) string {
+	// Remove newlines from the error, which corrupt the output.
+	errMsg = strings.Replace(errMsg, "\n", " ", -1)
+
+	// Limit the length to 75 characters, so that even on narrow terminals
+	// this will not overflow to the next line.
+	if len(errMsg) > 75 {
+		errMsg = errMsg[:74] + "…"
+	}
+	return errMsg
+}
+
 type replicatedProgressUpdater struct {
 	progressOut progress.Output
 
@@ -303,7 +327,23 @@ func (u *replicatedProgressUpdater) update(service swarm.Service, tasks []swarm.
 			u.slotMap[task.Slot] = mappedSlot
 		}
 
-		if !u.done && replicas <= maxProgressBars && uint64(mappedSlot) <= replicas {
+		if !terminalState(task.DesiredState) && task.Status.State == swarm.TaskStateRunning {
+			running++
+		}
+
+		if u.done || replicas > maxProgressBars || uint64(mappedSlot) > replicas {
+			continue
+		}
+
+		if task.Status.Err != "" {
+			u.progressOut.WriteProgress(progress.Progress{
+				ID:     fmt.Sprintf("%d/%d", mappedSlot, replicas),
+				Action: truncError(task.Status.Err),
+			})
+			continue
+		}
+
+		if !terminalState(task.DesiredState) && !terminalState(task.Status.State) {
 			u.progressOut.WriteProgress(progress.Progress{
 				ID:         fmt.Sprintf("%d/%d", mappedSlot, replicas),
 				Action:     fmt.Sprintf("%-[1]*s", longestState, task.Status.State),
@@ -312,9 +352,6 @@ func (u *replicatedProgressUpdater) update(service swarm.Service, tasks []swarm.
 				HideCounts: true,
 			})
 		}
-		if task.Status.State == swarm.TaskStateRunning {
-			running++
-		}
 	}
 
 	if !u.done {
@@ -335,6 +372,7 @@ type globalProgressUpdater struct {
 	done        bool
 }
 
+// nolint: gocyclo
 func (u *globalProgressUpdater) update(service swarm.Service, tasks []swarm.Task, activeNodes map[string]swarm.Node, rollback bool) (bool, error) {
 	// If there are multiple tasks with the same node ID, favor the one
 	// with the *lowest* desired state. This can happen in restart
@@ -388,7 +426,23 @@ func (u *globalProgressUpdater) update(service swarm.Service, tasks []swarm.Task
 
 	for _, task := range tasksByNode {
 		if node, nodeActive := activeNodes[task.NodeID]; nodeActive {
-			if !u.done && nodeCount <= maxProgressBars {
+			if !terminalState(task.DesiredState) && task.Status.State == swarm.TaskStateRunning {
+				running++
+			}
+
+			if u.done || nodeCount > maxProgressBars {
+				continue
+			}
+
+			if task.Status.Err != "" {
+				u.progressOut.WriteProgress(progress.Progress{
+					ID:     stringid.TruncateID(node.ID),
+					Action: truncError(task.Status.Err),
+				})
+				continue
+			}
+
+			if !terminalState(task.DesiredState) && !terminalState(task.Status.State) {
 				u.progressOut.WriteProgress(progress.Progress{
 					ID:         stringid.TruncateID(node.ID),
 					Action:     fmt.Sprintf("%-[1]*s", longestState, task.Status.State),
@@ -397,9 +451,6 @@ func (u *globalProgressUpdater) update(service swarm.Service, tasks []swarm.Task
 					HideCounts: true,
 				})
 			}
-			if task.Status.State == swarm.TaskStateRunning {
-				running++
-			}
 		}
 	}