From f676fc93c3791f72938a6be9c7517ac620c02d1c Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sat, 23 Jul 2016 09:58:58 -0700 Subject: [PATCH] Fix partial/full filter issue in `service tasks --filter` This fix tries to address the issue related to 24108 and 24790, and also the case from 24620#issuecomment-233715656 The reason for the failure case in the above mentioned issues is that currently Task names are actually indexed by Service Name (`e.ServiceAnnotations.Name`) To fix it, a pull request in swarmkit (swarmkit/pull/1193) has been opened separately. This fix adds the integration tests for the above mentioned issues. Swarmkit revendoring is needed to completely fix the issues. This fix fixes 24108. This fix fixes 24790. This fix is related to 24620. Signed-off-by: Yong Tang --- cli/command/task/print.go | 30 ++++++++---- daemon/cluster/convert/task.go | 6 ++- .../cluster/executor/container/container.go | 10 +++- integration-cli/docker_cli_swarm_test.go | 49 +++++++++++++++++++ 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/cli/command/task/print.go b/cli/command/task/print.go index 963aea95ce..b9d6b3eaf4 100644 --- a/cli/command/task/print.go +++ b/cli/command/task/print.go @@ -16,7 +16,7 @@ import ( ) const ( - psTaskItemFmt = "%s\t%s\t%s\t%s\t%s\t%s %s ago\t%s\n" + psTaskItemFmt = "%s\t%s\t%s\t%s\t%s %s ago\t%s\n" maxErrLength = 30 ) @@ -48,11 +48,12 @@ func Print(dockerCli *command.DockerCli, ctx context.Context, tasks []swarm.Task // Ignore flushing errors defer writer.Flush() - fmt.Fprintln(writer, strings.Join([]string{"ID", "NAME", "IMAGE", "NODE", "DESIRED STATE", "CURRENT STATE", "ERROR"}, "\t")) + fmt.Fprintln(writer, strings.Join([]string{"NAME", "IMAGE", "NODE", "DESIRED STATE", "CURRENT STATE", "ERROR"}, "\t")) - prevName := "" + prevServiceName := "" + prevSlot := 0 for _, task := range tasks { - serviceValue, err := resolver.Resolve(ctx, swarm.Service{}, task.ServiceID) + serviceName, err := resolver.Resolve(ctx, swarm.Service{}, task.ServiceID) if err != nil { return err } @@ -61,17 +62,27 @@ func Print(dockerCli *command.DockerCli, ctx context.Context, tasks []swarm.Task return err } - name := serviceValue - if task.Slot > 0 { - name = fmt.Sprintf("%s.%d", name, task.Slot) + name := task.Annotations.Name + // TODO: This is the fallback .. in case task name is not present in + // Annotations (upgraded from 1.12). + // We may be able to remove the following in the future. + if name == "" { + if task.Slot != 0 { + name = fmt.Sprintf("%v.%v.%v", serviceName, task.Slot, task.ID) + } else { + name = fmt.Sprintf("%v.%v.%v", serviceName, task.NodeID, task.ID) + } } // Indent the name if necessary indentedName := name - if prevName == name { + // Since the new format of the task name is .., we should only compare + // and here. + if prevServiceName == serviceName && prevSlot == task.Slot { indentedName = fmt.Sprintf(" \\_ %s", indentedName) } - prevName = name + prevServiceName = serviceName + prevSlot = task.Slot // Trim and quote the error message. taskErr := task.Status.Err @@ -85,7 +96,6 @@ func Print(dockerCli *command.DockerCli, ctx context.Context, tasks []swarm.Task fmt.Fprintf( writer, psTaskItemFmt, - task.ID, indentedName, task.Spec.ContainerSpec.Image, nodeValue, diff --git a/daemon/cluster/convert/task.go b/daemon/cluster/convert/task.go index e3be8b8eaf..5f3c3471c9 100644 --- a/daemon/cluster/convert/task.go +++ b/daemon/cluster/convert/task.go @@ -21,7 +21,11 @@ func TaskFromGRPC(t swarmapi.Task) types.Task { } task := types.Task{ - ID: t.ID, + ID: t.ID, + Annotations: types.Annotations{ + Name: t.Annotations.Name, + Labels: t.Annotations.Labels, + }, ServiceID: t.ServiceID, Slot: int(t.Slot), NodeID: t.NodeID, diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index c0415fa2bf..16b8e9eb7c 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -143,11 +143,19 @@ func (c *containerConfig) config() *enginecontainer.Config { } func (c *containerConfig) labels() map[string]string { + taskName := c.task.Annotations.Name + if taskName == "" { + if c.task.Slot != 0 { + taskName = fmt.Sprintf("%v.%v.%v", c.task.ServiceAnnotations.Name, c.task.Slot, c.task.ID) + } else { + taskName = fmt.Sprintf("%v.%v.%v", c.task.ServiceAnnotations.Name, c.task.NodeID, c.task.ID) + } + } var ( system = map[string]string{ "task": "", // mark as cluster task "task.id": c.task.ID, - "task.name": fmt.Sprintf("%v.%v", c.task.ServiceAnnotations.Name, c.task.Slot), + "task.name": taskName, "node.id": c.task.NodeID, "service.id": c.task.ServiceID, "service.name": c.task.ServiceAnnotations.Name, diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 306c55577e..e2cee8689a 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -274,3 +274,52 @@ func (s *DockerSwarmSuite) TestSwarmRemoveInternalNetwork(c *check.C) { c.Assert(strings.TrimSpace(out), checker.Contains, name) c.Assert(strings.TrimSpace(out), checker.Contains, "is a pre-defined network and cannot be removed") } + +// Test case for #24108, also the case from: +// https://github.com/docker/docker/pull/24620#issuecomment-233715656 +func (s *DockerSwarmSuite) TestSwarmTaskListFilter(c *check.C) { + d := s.AddDaemon(c, true, true) + + name := "redis-cluster-md5" + out, err := d.Cmd("service", "create", "--name", name, "--replicas=3", "busybox", "top") + c.Assert(err, checker.IsNil) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") + + filter := "name=redis-cluster" + + out, err = d.Cmd("service", "ps", "--filter", filter, name) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Contains, name+".1") + c.Assert(out, checker.Contains, name+".2") + c.Assert(out, checker.Contains, name+".3") + + out, err = d.Cmd("service", "ps", "--filter", "name="+name+".1", name) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Contains, name+".1") + c.Assert(out, checker.Not(checker.Contains), name+".2") + c.Assert(out, checker.Not(checker.Contains), name+".3") + + out, err = d.Cmd("service", "ps", "--filter", "name=none", name) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Not(checker.Contains), name+".1") + c.Assert(out, checker.Not(checker.Contains), name+".2") + c.Assert(out, checker.Not(checker.Contains), name+".3") + + name = "redis-cluster-sha1" + out, err = d.Cmd("service", "create", "--name", name, "--mode=global", "busybox", "top") + c.Assert(err, checker.IsNil) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") + + filter = "name=redis-cluster" + out, err = d.Cmd("service", "ps", "--filter", filter, name) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Contains, name) + + out, err = d.Cmd("service", "ps", "--filter", "name="+name, name) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Contains, name) + + out, err = d.Cmd("service", "ps", "--filter", "name=none", name) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Not(checker.Contains), name) +}