Merge pull request #1101 from silvin-lubecki/filter-stack-service

Fix service filtering by name on Kubernetes

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
+}

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
+}