From 9b348d821584f2dd1730f9ebdc18dfe934df18a5 Mon Sep 17 00:00:00 2001 From: Simon Ferquel Date: Tue, 24 Apr 2018 22:11:53 +0200 Subject: [PATCH] Nits + tests Signed-off-by: Simon Ferquel --- cli/command/stack/kubernetes/client.go | 4 +- .../stack/kubernetes/conversion_test.go | 4 +- cli/command/stack/kubernetes/services.go | 58 ++++++---- cli/command/stack/kubernetes/services_test.go | 104 ++++++++++++++++++ 4 files changed, 142 insertions(+), 28 deletions(-) create mode 100644 cli/command/stack/kubernetes/services_test.go diff --git a/cli/command/stack/kubernetes/client.go b/cli/command/stack/kubernetes/client.go index 8ecb039ed76a..d076ad461f32 100644 --- 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 index abc4163e52a1..ef32f130dda6 100644 --- 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 index a06ad8f9c56e..242ea1d3e475 100644 --- 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 new file mode 100644 index 000000000000..1d7f6b430b6d --- /dev/null +++ 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)) + } + }) + } +}