Nits + tests

Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
docker · Apr 24, 2018 · 9b348d8 · 9b348d8
1 parent 58985c4
commit 9b348d8
Show file tree

Hide file tree

Showing 4 changed files with 142 additions and 28 deletions.
diff --git a/cli/command/stack/kubernetes/client.go b/cli/command/stack/kubernetes/client.go
@@ -65,12 +65,12 @@ func (s *Factory) ReplicationControllers() corev1.ReplicationControllerInterface
 	return s.coreClientSet.ReplicationControllers(s.namespace)
 }
 
-// ReplicaSets return a client for kubernetes replace sets
+// ReplicaSets returns a client for kubernetes replace sets
 func (s *Factory) ReplicaSets() typesappsv1beta2.ReplicaSetInterface {
 	return s.appsClientSet.ReplicaSets(s.namespace)
 }
 
-// DaemonSets return a client for kubernetes daemon sets
+// DaemonSets returns a client for kubernetes daemon sets
 func (s *Factory) DaemonSets() typesappsv1beta2.DaemonSetInterface {
 	return s.appsClientSet.DaemonSets(s.namespace)
 }
diff --git a/cli/command/stack/kubernetes/conversion_test.go b/cli/command/stack/kubernetes/conversion_test.go
@@ -19,7 +19,7 @@ func TestReplicasConversionNeedsAService(t *testing.T) {
 		Items: []appsv1beta2.ReplicaSet{makeReplicaSet("unknown", 0, 0)},
 	}
 	services := apiv1.ServiceList{}
-	_, _, err := replicasToServices(&replicas, &services)
+	_, _, err := replicasToServices(&replicas, &appsv1beta2.DaemonSetList{}, &services)
 	assert.ErrorContains(t, err, "could not find service")
 }
 
@@ -124,7 +124,7 @@ func TestKubernetesServiceToSwarmServiceConversion(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		swarmServices, listInfo, err := replicasToServices(tc.replicas, tc.services)
+		swarmServices, listInfo, err := replicasToServices(tc.replicas, &appsv1beta2.DaemonSetList{}, tc.services)
 		assert.NilError(t, err)
 		assert.DeepEqual(t, tc.expectedServices, swarmServices)
 		assert.DeepEqual(t, tc.expectedListInfo, listInfo)

diff --git a/cli/command/stack/kubernetes/services.go b/cli/command/stack/kubernetes/services.go
@@ -7,6 +7,7 @@ import (
 	"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"
 	appsv1beta2 "k8s.io/api/apps/v1beta2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -29,6 +30,38 @@ func generateValuedLabelsSelector(valuedLabels map[string][]string) []string {
 	return result
 }
 
+func parseLabelFilters(rawFilters []string) ([]string, map[string][]string) {
+	var presentLabels []string
+	valuedLabels := map[string][]string{}
+	for _, rawLabel := range rawFilters {
+		equalIndex := strings.Index(rawLabel, "=")
+		if equalIndex == -1 {
+			presentLabels = append(presentLabels, rawLabel)
+		} else {
+			key := rawLabel[:equalIndex]
+			val := rawLabel[equalIndex+1:]
+			valuedLabels[key] = append(valuedLabels[key], val)
+		}
+	}
+	return presentLabels, valuedLabels
+}
+
+func generateLabelSelector(f filters.Args, stackName string) string {
+	names := f.Get("name")
+
+	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+"_"))
+		}
+	}
+
+	presentLabels, valuedLabels := parseLabelFilters(f.Get("label"))
+	selectors := append(presentLabels, labels.SelectorForStack(stackName, names...))
+	selectors = append(selectors, generateValuedLabelsSelector(valuedLabels)...)
+	return strings.Join(selectors, ",")
+}
+
 // RunServices is the kubernetes implementation of docker stack services
 func RunServices(dockerCli *KubeCli, opts options.Services) error {
 	// Initialize clients
@@ -52,31 +85,8 @@ func RunServices(dockerCli *KubeCli, opts options.Services) error {
 		return err
 	}
 	modes := filters.Get("mode")
-	names := filters.Get("name")
-	var presentLabels []string
-	valuedLabels := map[string][]string{}
-
-	for _, rawLabel := range filters.Get("label") {
-		equalIndex := strings.Index(rawLabel, "=")
-		if equalIndex == -1 {
-			presentLabels = append(presentLabels, rawLabel)
-		} else {
-			key := rawLabel[:equalIndex]
-			val := rawLabel[equalIndex+1:]
-			valuedLabels[key] = append(valuedLabels[key], val)
-		}
-	}
-
-	for _, n := range names {
-		if strings.HasPrefix(n, opts.Namespace+"_") {
-			// 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, opts.Namespace+"_"))
-		}
-	}
 
-	selectors := append(presentLabels, labels.SelectorForStack(opts.Namespace, names...))
-	selectors = append(selectors, generateValuedLabelsSelector(valuedLabels)...)
-	labelSelector := strings.Join(selectors, ",")
+	labelSelector := generateLabelSelector(filters, opts.Namespace)
 	var replicasList *appsv1beta2.ReplicaSetList
 	if len(modes) == 0 || filters.ExactMatch("mode", "replicated") {
 		if replicasList, err = replicas.List(metav1.ListOptions{LabelSelector: labelSelector}); err != nil {

diff --git a/cli/command/stack/kubernetes/services_test.go b/cli/command/stack/kubernetes/services_test.go
@@ -0,0 +1,104 @@
+package kubernetes
+
+import (
+	"testing"
+
+	"github.com/docker/docker/api/types/filters"
+	"github.com/gotestyourself/gotestyourself/assert"
+	"github.com/gotestyourself/gotestyourself/assert/cmp"
+)
+
+func TestSerficeFiltersLabelSelectorGen(t *testing.T) {
+	cases := []struct {
+		name                  string
+		stackName             string
+		filters               filters.Args
+		expectedSelectorParts []string
+	}{
+		{
+			name:      "no-filter",
+			stackName: "test",
+			filters:   filters.NewArgs(),
+			expectedSelectorParts: []string{
+				"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",
+			filters: filters.NewArgs(
+				filters.KeyValuePair{Key: "label", Value: "label-is-present"},
+			),
+			expectedSelectorParts: []string{
+				"com.docker.stack.namespace=test",
+				"label-is-present",
+			},
+		},
+		{
+			name:      "single value label filter",
+			stackName: "test",
+			filters: filters.NewArgs(
+				filters.KeyValuePair{Key: "label", Value: "label1=test"},
+			),
+			expectedSelectorParts: []string{
+				"com.docker.stack.namespace=test",
+				"label1=test",
+			},
+		},
+		{
+			name:      "multi value label filter",
+			stackName: "test",
+			filters: filters.NewArgs(
+				filters.KeyValuePair{Key: "label", Value: "label1=test"},
+				filters.KeyValuePair{Key: "label", Value: "label1=test2"},
+			),
+			expectedSelectorParts: []string{
+				"com.docker.stack.namespace=test",
+				"label1 in (test,test2)",
+			},
+		},
+		{
+			name:      "2 different labels filter",
+			stackName: "test",
+			filters: filters.NewArgs(
+				filters.KeyValuePair{Key: "label", Value: "label1=test"},
+				filters.KeyValuePair{Key: "label", Value: "label2=test2"},
+			),
+			expectedSelectorParts: []string{
+				"com.docker.stack.namespace=test",
+				"label1=test",
+				"label2=test2",
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			result := generateLabelSelector(c.filters, c.stackName)
+			for _, toFind := range c.expectedSelectorParts {
+				assert.Assert(t, cmp.Contains(result, toFind))
+			}
+		})
+	}
+}