Improve service tasks grouping on printing

cli/command/task/print.go

@@ -10,33 +10,40 @@ import (
 	"github.com/docker/cli/cli/command/idresolver"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/docker/api/types/swarm"
+	"vbom.ml/util/sortorder"
 )
 
-type tasksBySlot []swarm.Task
+type tasksSortable []swarm.Task
 
-func (t tasksBySlot) Len() int {
+func (t tasksSortable) Len() int {
 	return len(t)
 }
 
-func (t tasksBySlot) Swap(i, j int) {
+func (t tasksSortable) Swap(i, j int) {
 	t[i], t[j] = t[j], t[i]
 }
 
-func (t tasksBySlot) Less(i, j int) bool {
-	// Sort by slot.
-	if t[i].Slot != t[j].Slot {
-		return t[i].Slot < t[j].Slot
+func (t tasksSortable) Less(i, j int) bool {
+	if t[i].Name != t[j].Name {
+		return sortorder.NaturalLess(t[i].Name, t[j].Name)
 	}
-
-	// If same slot, sort by most recent.
+	// Sort tasks for the same service and slot by most recent.
 	return t[j].Meta.CreatedAt.Before(t[i].CreatedAt)
 }
 
 // Print task information in a format.
 // Besides this, command `docker node ps <node>`
 // and `docker stack ps` will call this, too.
 func Print(ctx context.Context, dockerCli command.Cli, tasks []swarm.Task, resolver *idresolver.IDResolver, trunc, quiet bool, format string) error {
-	sort.Stable(tasksBySlot(tasks))
+	tasks, err := generateTaskNames(ctx, tasks, resolver)
+	if err != nil {
+		return err
+	}
+
+	// First sort tasks, so that all tasks (including previous ones) of the same
+	// service and slot are together. This must be done first, to print "previous"
+	// tasks indented
+	sort.Stable(tasksSortable(tasks))
 
 	names := map[string]string{}
 	nodes := map[string]string{}
@@ -47,40 +54,55 @@ func Print(ctx context.Context, dockerCli command.Cli, tasks []swarm.Task, resol
 		Trunc:  trunc,
 	}
 
+	var indent string
+	if tasksCtx.Format.IsTable() {
+		indent = ` \_ `
+	}
 	prevName := ""
 	for _, task := range tasks {
-		serviceName, err := resolver.Resolve(ctx, swarm.Service{}, task.ServiceID)
-		if err != nil {
-			return err
+		if task.Name == prevName {
+			// Indent previous tasks of the same slot
+			names[task.ID] = indent + task.Name
+		} else {
+			names[task.ID] = task.Name
 		}
+		prevName = task.Name
 
 		nodeValue, err := resolver.Resolve(ctx, swarm.Node{}, task.NodeID)
 		if err != nil {
 			return err
 		}
+		nodes[task.ID] = nodeValue
+	}
 
-		var name string
-		if task.Slot != 0 {
-			name = fmt.Sprintf("%v.%v", serviceName, task.Slot)
-		} else {
-			name = fmt.Sprintf("%v.%v", serviceName, task.NodeID)
-		}
+	return FormatWrite(tasksCtx, tasks, names, nodes)
+}
 
-		// Indent the name if necessary
-		indentedName := name
-		if name == prevName {
-			indentedName = fmt.Sprintf(" \\_ %s", indentedName)
+// generateTaskNames generates names for the given tasks, and returns a copy of
+// the slice with the 'Name' field set.
+//
+// Depending if the "--no-resolve" option is set, names have the following pattern:
+//
+// - ServiceName.Slot or ServiceID.Slot for tasks that are part of a replicated service
+// - ServiceName.NodeName or ServiceID.NodeID for tasks that are part of a global service
+//
+// Task-names are not unique in cases where "tasks" contains previous/rotated tasks.
+func generateTaskNames(ctx context.Context, tasks []swarm.Task, resolver *idresolver.IDResolver) ([]swarm.Task, error) {
+	// Use a copy of the tasks list, to not modify the original slice
+	t := append(tasks[:0:0], tasks...)
+
+	for i, task := range t {
+		serviceName, err := resolver.Resolve(ctx, swarm.Service{}, task.ServiceID)
+		if err != nil {
+			return nil, err
 		}
-		prevName = name
-
-		names[task.ID] = name
-		if tasksCtx.Format.IsTable() {
-			names[task.ID] = indentedName
+		if task.Slot != 0 {
+			t[i].Name = fmt.Sprintf("%v.%v", serviceName, task.Slot)
+		} else {
+			t[i].Name = fmt.Sprintf("%v.%v", serviceName, task.NodeID)
 		}
-		nodes[task.ID] = nodeValue
 	}
-
-	return FormatWrite(tasksCtx, tasks, names, nodes)
+	return t, nil
 }
 
 // DefaultFormat returns the default format from the config file, or table

cli/command/task/print_test.go

@@ -15,6 +15,41 @@ import (
 	"gotest.tools/golden"
 )
 
+func TestTaskPrintSorted(t *testing.T) {
+	apiClient := &fakeClient{
+		serviceInspectWithRaw: func(ref string, options types.ServiceInspectOptions) (swarm.Service, []byte, error) {
+			if ref == "service-id-one" {
+				return *Service(ServiceName("service-name-1")), nil, nil
+			}
+			return *Service(ServiceName("service-name-10")), nil, nil
+		},
+	}
+
+	cli := test.NewFakeCli(apiClient)
+	tasks := []swarm.Task{
+		*Task(
+			TaskID("id-foo"),
+			TaskServiceID("service-id-ten"),
+			TaskNodeID("id-node"),
+			WithTaskSpec(TaskImage("myimage:mytag")),
+			TaskDesiredState(swarm.TaskStateReady),
+			WithStatus(TaskState(swarm.TaskStateFailed), Timestamp(time.Now().Add(-2*time.Hour))),
+		),
+		*Task(
+			TaskID("id-bar"),
+			TaskServiceID("service-id-one"),
+			TaskNodeID("id-node"),
+			WithTaskSpec(TaskImage("myimage:mytag")),
+			TaskDesiredState(swarm.TaskStateReady),
+			WithStatus(TaskState(swarm.TaskStateFailed), Timestamp(time.Now().Add(-2*time.Hour))),
+		),
+	}
+
+	err := Print(context.Background(), cli, tasks, idresolver.New(apiClient, false), false, false, formatter.TableFormatKey)
+	assert.NilError(t, err)
+	golden.Assert(t, cli.OutBuffer().String(), "task-print-sorted.golden")
+}
+
 func TestTaskPrintWithQuietOption(t *testing.T) {
 	quiet := true
 	trunc := false

cli/command/task/testdata/task-print-sorted.golden

@@ -0,0 +1,3 @@
+ID                  NAME                IMAGE               NODE                DESIRED STATE       CURRENT STATE        ERROR               PORTS
+id-bar              service-name-1.1    myimage:mytag       id-node             Ready               Failed 2 hours ago                       
+id-foo              service-name-10.1   myimage:mytag       id-node             Ready               Failed 2 hours ago