From 80b22064d11b2c15cfbb6b8fe3eae52cb1120739 Mon Sep 17 00:00:00 2001 From: Andrii Berehuliak Date: Sun, 22 Sep 2019 19:22:23 +0300 Subject: [PATCH] Improve service tasks grouping on printing When printing services' tasks with `docker service ps` command, tasks are grouped only by task slot. This leads to interleaving tasks from different services when `docker service ps` is called with multiple services. Besides this, global services do not have slots at all and printing tasks for them doesn't group and doesn't properly indent tasks with \_. With this patch all tasks are grouped by service ID, slot and node ID (relevant only for global services) and it fixes issue 533. Before this patch: ```console docker service ps a b c ID NAME IMAGE NODE DESIRED STATE CURRENT STATE ERROR PORTS xbzm6ed776yw c.j1afavbqqhr21jvnid3nnfoyt nginx:alpine docker-desktop Running Running 5 seconds ago 4mcsovp8ckwn \_ c.j1afavbqqhr21jvnid3nnfoyt nginx:alpine docker-desktop Shutdown Shutdown 6 seconds ago qpcgdsx1r21a b.1 nginx:alpine docker-desktop Running Running 2 seconds ago kfjo1hly92l4 a.1 nginx:alpine docker-desktop Running Running 5 seconds ago pubrerosvsw5 b.1 nginx:alpine docker-desktop Shutdown Shutdown 3 seconds ago fu08gfi8tfyv a.1 nginx:alpine docker-desktop Shutdown Shutdown 7 seconds ago pu6qmgyoibq4 b.2 nginx:alpine docker-desktop Running Ready 1 second ago tz1n4hjne6pk \_ b.2 nginx:alpine docker-desktop Shutdown Shutdown less than a second ago xq8dogqcbxd2 a.2 nginx:alpine docker-desktop Running Running 44 seconds ago rm40lofzed0h a.3 nginx:alpine docker-desktop Running Starting less than a second ago sqqj2n9fpi82 b.3 nginx:alpine docker-desktop Running Running 5 seconds ago prv3gymkvqk6 \_ b.3 nginx:alpine docker-desktop Shutdown Shutdown 6 seconds ago qn7c7jmjuo76 a.3 nginx:alpine docker-desktop Shutdown Shutdown less than a second ago wi9330mbabpg a.4 nginx:alpine docker-desktop Running Running 2 seconds ago p5oy6h7nkvc3 \_ a.4 nginx:alpine docker-desktop Shutdown Shutdown 3 seconds ago ``` After this patch: ```console docker service ps a b c ID NAME IMAGE NODE DESIRED STATE CURRENT STATE ERROR PORTS kfjo1hly92l4 a.1 nginx:alpine docker-desktop Running Running 32 seconds ago fu08gfi8tfyv \_ a.1 nginx:alpine docker-desktop Shutdown Shutdown 34 seconds ago 3pam0limnn24 a.2 nginx:alpine docker-desktop Running Running 23 seconds ago xq8dogqcbxd2 \_ a.2 nginx:alpine docker-desktop Shutdown Shutdown 24 seconds ago rm40lofzed0h a.3 nginx:alpine docker-desktop Running Running 26 seconds ago qn7c7jmjuo76 \_ a.3 nginx:alpine docker-desktop Shutdown Shutdown 27 seconds ago wi9330mbabpg a.4 nginx:alpine docker-desktop Running Running 29 seconds ago p5oy6h7nkvc3 \_ a.4 nginx:alpine docker-desktop Shutdown Shutdown 30 seconds ago qpcgdsx1r21a b.1 nginx:alpine docker-desktop Running Running 29 seconds ago pubrerosvsw5 \_ b.1 nginx:alpine docker-desktop Shutdown Shutdown 30 seconds ago pu6qmgyoibq4 b.2 nginx:alpine docker-desktop Running Running 26 seconds ago tz1n4hjne6pk \_ b.2 nginx:alpine docker-desktop Shutdown Shutdown 27 seconds ago sqqj2n9fpi82 b.3 nginx:alpine docker-desktop Running Running 32 seconds ago prv3gymkvqk6 \_ b.3 nginx:alpine docker-desktop Shutdown Shutdown 33 seconds ago xbzm6ed776yw c.j1afavbqqhr21jvnid3nnfoyt nginx:alpine docker-desktop Running Running 32 seconds ago 4mcsovp8ckwn \_ c.j1afavbqqhr21jvnid3nnfoyt nginx:alpine docker-desktop Shutdown Shutdown 33 seconds ago ``` Signed-off-by: Andrii Berehuliak Signed-off-by: Sebastiaan van Stijn --- cli/command/task/print.go | 86 ++++++++++++------- cli/command/task/print_test.go | 35 ++++++++ .../task/testdata/task-print-sorted.golden | 3 + 3 files changed, 92 insertions(+), 32 deletions(-) create mode 100644 cli/command/task/testdata/task-print-sorted.golden diff --git a/cli/command/task/print.go b/cli/command/task/print.go index 761a5e8fa5..3cb2d0b22f 100644 --- a/cli/command/task/print.go +++ b/cli/command/task/print.go @@ -10,25 +10,24 @@ 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) } @@ -36,7 +35,15 @@ func (t tasksBySlot) Less(i, j int) bool { // Besides this, command `docker node ps ` // 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,42 +54,57 @@ 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 } - - var name string - if task.Slot != 0 { - name = fmt.Sprintf("%v.%v", serviceName, task.Slot) - } else { - name = fmt.Sprintf("%v.%v", serviceName, task.NodeID) - } - - // Indent the name if necessary - indentedName := name - if name == prevName { - indentedName = fmt.Sprintf(" \\_ %s", indentedName) - } - prevName = name - - names[task.ID] = name - if tasksCtx.Format.IsTable() { - names[task.ID] = indentedName - } nodes[task.ID] = nodeValue } return FormatWrite(tasksCtx, tasks, names, nodes) } +// 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 + } + 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) + } + } + return t, nil +} + // DefaultFormat returns the default format from the config file, or table // format if nothing is set in the config. func DefaultFormat(configFile *configfile.ConfigFile, quiet bool) string { diff --git a/cli/command/task/print_test.go b/cli/command/task/print_test.go index 5b5c0a81c3..792ded0b47 100644 --- a/cli/command/task/print_test.go +++ b/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 diff --git a/cli/command/task/testdata/task-print-sorted.golden b/cli/command/task/testdata/task-print-sorted.golden new file mode 100644 index 0000000000..0ff9b67f21 --- /dev/null +++ b/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