From da5111891cc980b14620265c8455a166c2f1cc5e Mon Sep 17 00:00:00 2001
From: Daniel Smith <dbsmith@google.com>
Date: Thu, 16 Apr 2015 16:25:54 -0700
Subject: [PATCH] Fix selector handling in listers.go.

---
 pkg/client/cache/listers.go      | 42 +++++++++++++++++++++++++++++---
 pkg/client/cache/listers_test.go | 31 +++++++++++++++++++++++
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/pkg/client/cache/listers.go b/pkg/client/cache/listers.go
index 4e1dd78272c..0afd64b8661 100644
--- a/pkg/client/cache/listers.go
+++ b/pkg/client/cache/listers.go
@@ -39,10 +39,16 @@ type StoreToPodLister struct {
 	Store
 }
 
-// TODO Get rid of the selector because that is confusing because the user might not realize that there has already been
-// some selection at the caching stage.  Also, consistency will facilitate code generation.  However, the pkg/client
-// is inconsistent too.
+// Please note that selector is filtering among the pods that have gotten into
+// the store; there may have been some filtering that already happened before
+// that.
+//
+// TODO: converge on the interface in pkg/client.
 func (s *StoreToPodLister) List(selector labels.Selector) (pods []*api.Pod, err error) {
+	// TODO: it'd be great to just call
+	// s.Pods(api.NamespaceAll).List(selector), however then we'd have to
+	// remake the list.Items as a []*api.Pod. So leave this separate for
+	// now.
 	for _, m := range s.Store.List() {
 		pod := m.(*api.Pod)
 		if selector.Matches(labels.Set(pod.Labels)) {
@@ -52,6 +58,32 @@ func (s *StoreToPodLister) List(selector labels.Selector) (pods []*api.Pod, err
 	return pods, nil
 }
 
+// Pods is taking baby steps to be more like the api in pkg/client
+func (s *StoreToPodLister) Pods(namespace string) storePodsNamespacer {
+	return storePodsNamespacer{s.Store, namespace}
+}
+
+type storePodsNamespacer struct {
+	store     Store
+	namespace string
+}
+
+// Please note that selector is filtering among the pods that have gotten into
+// the store; there may have been some filtering that already happened before
+// that.
+func (s storePodsNamespacer) List(selector labels.Selector) (pods api.PodList, err error) {
+	list := api.PodList{}
+	for _, m := range s.store.List() {
+		pod := m.(*api.Pod)
+		if s.namespace == api.NamespaceAll || s.namespace == pod.Namespace {
+			if selector.Matches(labels.Set(pod.Labels)) {
+				list.Items = append(list.Items, *pod)
+			}
+		}
+	}
+	return list, nil
+}
+
 // Exists returns true if a pod matching the namespace/name of the given pod exists in the store.
 func (s *StoreToPodLister) Exists(pod *api.Pod) (bool, error) {
 	_, exists, err := s.Store.Get(pod)
@@ -116,6 +148,10 @@ func (s *StoreToServiceLister) GetPodServices(pod *api.Pod) (services []api.Serv
 		if service.Namespace != pod.Namespace {
 			continue
 		}
+		if service.Spec.Selector == nil {
+			// services with nil selectors match nothing, not everything.
+			continue
+		}
 		selector = labels.Set(service.Spec.Selector).AsSelector()
 		if selector.Matches(labels.Set(pod.Labels)) {
 			services = append(services, service)
diff --git a/pkg/client/cache/listers_test.go b/pkg/client/cache/listers_test.go
index 3f759c3c715..5e57208559a 100644
--- a/pkg/client/cache/listers_test.go
+++ b/pkg/client/cache/listers_test.go
@@ -90,3 +90,34 @@ func TestStoreToPodLister(t *testing.T) {
 		t.Errorf("Unexpected pod exists")
 	}
 }
+
+func TestStoreToServiceLister(t *testing.T) {
+	store := NewStore(MetaNamespaceKeyFunc)
+	store.Add(&api.Service{
+		ObjectMeta: api.ObjectMeta{Name: "foo"},
+		Spec: api.ServiceSpec{
+			Selector: map[string]string{},
+		},
+	})
+	store.Add(&api.Service{ObjectMeta: api.ObjectMeta{Name: "bar"}})
+	ssl := StoreToServiceLister{store}
+
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name:   "foopod",
+			Labels: map[string]string{"role": "foo"},
+		},
+	}
+
+	services, err := ssl.GetPodServices(pod)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	if len(services) != 1 {
+		t.Fatalf("Expected 1 service, got %v", len(services))
+	}
+	if e, a := "foo", services[0].Name; e != a {
+		t.Errorf("Expected service %q, got %q", e, a)
+	}
+}