From b1ff1991675a9b0a392b6b7853e39dc5eb58d018 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Fri, 1 Jun 2018 00:18:26 +0200 Subject: [PATCH] Fix service filtering by name on Kubernetes to mimic Swarm filtering * Kubernetes native filtering (server side) is an exact match, now filtering on name is made client-side to add prefix-matching Signed-off-by: Silvin Lubecki --- cli/command/stack/kubernetes/services.go | 36 +++++--- cli/command/stack/kubernetes/services_test.go | 86 ++++++++++++------- 2 files changed, 80 insertions(+), 42 deletions(-) diff --git a/cli/command/stack/kubernetes/services.go b/cli/command/stack/kubernetes/services.go index 5a1db529dd..a2d13e5672 100644 --- a/cli/command/stack/kubernetes/services.go +++ b/cli/command/stack/kubernetes/services.go @@ -2,13 +2,13 @@ package kubernetes import ( "fmt" - "sort" "strings" "github.com/docker/cli/cli/command/formatter" "github.com/docker/cli/cli/command/stack/options" "github.com/docker/cli/kubernetes/labels" "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/swarm" appsv1beta2 "k8s.io/api/apps/v1beta2" corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" @@ -49,15 +49,7 @@ func parseLabelFilters(rawFilters []string) map[string][]string { } func generateLabelSelector(f filters.Args, stackName string) string { - names := f.Get("name") - sort.Strings(names) - for _, n := range names { - if strings.HasPrefix(n, stackName+"_") { - // also accepts with unprefixed service name (for compat with existing swarm scripts where service names are prefixed by stack names) - names = append(names, strings.TrimPrefix(n, stackName+"_")) - } - } - selectors := append(generateSelector(parseLabelFilters(f.Get("label"))), labels.SelectorForStack(stackName, names...)) + selectors := append(generateSelector(parseLabelFilters(f.Get("label"))), labels.SelectorForStack(stackName)) return strings.Join(selectors, ",") } @@ -120,6 +112,7 @@ func RunServices(dockerCli *KubeCli, opts options.Services) error { if err != nil { return err } + services = filterServicesByName(services, filters.Get("name"), stackName) if opts.Quiet { info = map[string]formatter.ServiceListInfo{} @@ -140,3 +133,26 @@ func RunServices(dockerCli *KubeCli, opts options.Services) error { } return formatter.ServiceListWrite(servicesCtx, services, info) } + +func filterServicesByName(services []swarm.Service, names []string, stackName string) []swarm.Service { + if len(names) == 0 { + return services + } + prefix := stackName + "_" + // Accepts unprefixed service name (for compatibility with existing swarm scripts where service names are prefixed by stack names) + for i, n := range names { + if !strings.HasPrefix(n, prefix) { + names[i] = stackName + "_" + n + } + } + // Filter services + result := []swarm.Service{} + for _, s := range services { + for _, n := range names { + if strings.HasPrefix(s.Spec.Name, n) { + result = append(result, s) + } + } + } + return result +} diff --git a/cli/command/stack/kubernetes/services_test.go b/cli/command/stack/kubernetes/services_test.go index 41a3348e0b..89cb9b7e4e 100644 --- a/cli/command/stack/kubernetes/services_test.go +++ b/cli/command/stack/kubernetes/services_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/swarm" "github.com/gotestyourself/gotestyourself/assert" "github.com/gotestyourself/gotestyourself/assert/cmp" ) @@ -23,27 +24,6 @@ func TestServiceFiltersLabelSelectorGen(t *testing.T) { "com.docker.stack.namespace=test", }, }, - { - name: "single-name filter", - stackName: "test", - filters: filters.NewArgs(filters.KeyValuePair{Key: "name", Value: "svc-test"}), - expectedSelectorParts: []string{ - "com.docker.stack.namespace=test", - "com.docker.service.name=svc-test", - }, - }, - { - name: "multi-name filter", - stackName: "test", - filters: filters.NewArgs( - filters.KeyValuePair{Key: "name", Value: "svc-test"}, - filters.KeyValuePair{Key: "name", Value: "svc-test2"}, - ), - expectedSelectorParts: []string{ - "com.docker.stack.namespace=test", - "com.docker.service.name in (svc-test,svc-test2)", - }, - }, { name: "label present filter", stackName: "test", @@ -92,17 +72,6 @@ func TestServiceFiltersLabelSelectorGen(t *testing.T) { "label2=test2", }, }, - { - name: "name filter with stackName prefix", - stackName: "test", - filters: filters.NewArgs( - filters.KeyValuePair{Key: "name", Value: "test_svc1"}, - ), - expectedSelectorParts: []string{ - "com.docker.stack.namespace=test", - "com.docker.service.name in (test_svc1,svc1)", - }, - }, } for _, c := range cases { @@ -114,3 +83,56 @@ func TestServiceFiltersLabelSelectorGen(t *testing.T) { }) } } +func TestServiceFiltersServiceByName(t *testing.T) { + cases := []struct { + name string + filters []string + services []swarm.Service + expectedServices []swarm.Service + }{ + { + name: "no filter", + filters: []string{}, + services: makeServices("s1", "s2"), + expectedServices: makeServices("s1", "s2"), + }, + { + name: "single-name filter", + filters: []string{"s1"}, + services: makeServices("s1", "s2"), + expectedServices: makeServices("s1"), + }, + { + name: "filter by prefix", + filters: []string{"prefix"}, + services: makeServices("prefix-s1", "prefix-s2", "s2"), + expectedServices: makeServices("prefix-s1", "prefix-s2"), + }, + { + name: "multi-name filter", + filters: []string{"s1", "s2"}, + services: makeServices("s1", "s2", "s3"), + expectedServices: makeServices("s1", "s2"), + }, + { + name: "stack name prefix is valid", + filters: []string{"stack_s1"}, + services: makeServices("s1", "s11", "s2"), + expectedServices: makeServices("s1", "s11"), + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + result := filterServicesByName(c.services, c.filters, "stack") + assert.DeepEqual(t, c.expectedServices, result) + }) + } +} + +func makeServices(names ...string) []swarm.Service { + result := make([]swarm.Service, len(names)) + for i, n := range names { + result[i] = swarm.Service{Spec: swarm.ServiceSpec{Annotations: swarm.Annotations{Name: "stack_" + n}}} + } + return result +}