Add field selector to List, implement for pods.

pkg/apiserver/apiserver_test.go

@@ -88,7 +88,7 @@ type SimpleRESTStorage struct {
 	injectedFunction func(obj runtime.Object) (returnObj runtime.Object, err error)
 }
 
-func (storage *SimpleRESTStorage) List(labels.Selector) (runtime.Object, error) {
+func (storage *SimpleRESTStorage) List(label, field labels.Selector) (runtime.Object, error) {
 	result := &SimpleList{
 		Items: storage.list,
 	}

pkg/apiserver/interfaces.go

@@ -30,8 +30,7 @@ type RESTStorage interface {
 	New() runtime.Object
 
 	// List selects resources in the storage which match to the selector.
-	// TODO: add field selector in addition to label selector.
+	List(label, field labels.Selector) (runtime.Object, error)
-	List(labels.Selector) (runtime.Object, error)
 
 	// Get finds a resource in the storage by id and returns it.
 	// Although it can return an arbitrary error value, IsNotFound(err) is true for the

pkg/apiserver/resthandler.go

@@ -70,12 +70,17 @@ func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w htt
 	case "GET":
 		switch len(parts) {
 		case 1:
-			selector, err := labels.ParseSelector(req.URL.Query().Get("labels"))
+			label, err := labels.ParseSelector(req.URL.Query().Get("labels"))
 			if err != nil {
 				errorJSON(err, h.codec, w)
 				return
 			}
-			list, err := storage.List(selector)
+			field, err := labels.ParseSelector(req.URL.Query().Get("fields"))
+			if err != nil {
+				errorJSON(err, h.codec, w)
+				return
+			}
+			list, err := storage.List(label, field)
 			if err != nil {
 				errorJSON(err, h.codec, w)
 				return

pkg/registry/binding/rest.go

@@ -41,7 +41,7 @@ func NewREST(bindingRegistry Registry) *REST {
 }
 
 // List returns an error because bindings are write-only objects.
-func (*REST) List(selector labels.Selector) (runtime.Object, error) {
+func (*REST) List(label, field labels.Selector) (runtime.Object, error) {
 	return nil, errors.NewNotFound("binding", "list")
 }
 

pkg/registry/binding/rest_test.go

@@ -65,7 +65,7 @@ func TestRESTUnsupported(t *testing.T) {
 	if _, err := b.Get("binding id"); err == nil {
 		t.Errorf("unexpected non-error")
 	}
-	if _, err := b.List(labels.Set{"name": "foo"}.AsSelector()); err == nil {
+	if _, err := b.List(labels.Set{"name": "foo"}.AsSelector(), labels.Everything()); err == nil {
 		t.Errorf("unexpected non-error")
 	}
 	// Try sending wrong object just to get 100% coverage

pkg/registry/controller/rest.go

@@ -97,14 +97,17 @@ func (rs *REST) Get(id string) (runtime.Object, error) {
 }
 
 // List obtains a list of ReplicationControllers that match selector.
-func (rs *REST) List(selector labels.Selector) (runtime.Object, error) {
+func (rs *REST) List(label, field labels.Selector) (runtime.Object, error) {
+	if !field.Empty() {
+		return nil, fmt.Errorf("field selector not supported yet")
+	}
 	controllers, err := rs.registry.ListControllers()
 	if err != nil {
 		return nil, err
 	}
 	filtered := []api.ReplicationController{}
 	for _, controller := range controllers.Items {
-		if selector.Matches(labels.Set(controller.Labels)) {
+		if label.Matches(labels.Set(controller.Labels)) {
 			rs.fillCurrentState(&controller)
 			filtered = append(filtered, controller)
 		}

pkg/registry/controller/rest_test.go

@@ -39,7 +39,7 @@ func TestListControllersError(t *testing.T) {
 	storage := REST{
 		registry: &mockRegistry,
 	}
-	controllers, err := storage.List(nil)
+	controllers, err := storage.List(labels.Everything(), labels.Everything())
 	if err != mockRegistry.Err {
 		t.Errorf("Expected %#v, Got %#v", mockRegistry.Err, err)
 	}
@@ -53,7 +53,7 @@ func TestListEmptyControllerList(t *testing.T) {
 	storage := REST{
 		registry: &mockRegistry,
 	}
-	controllers, err := storage.List(labels.Everything())
+	controllers, err := storage.List(labels.Everything(), labels.Everything())
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -86,7 +86,7 @@ func TestListControllerList(t *testing.T) {
 	storage := REST{
 		registry: &mockRegistry,
 	}
-	controllersObj, err := storage.List(labels.Everything())
+	controllersObj, err := storage.List(labels.Everything(), labels.Everything())
 	controllers := controllersObj.(*api.ReplicationControllerList)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)

pkg/registry/endpoint/rest.go

@@ -43,9 +43,9 @@ func (rs *REST) Get(id string) (runtime.Object, error) {
 }
 
 // List satisfies the RESTStorage interface.
-func (rs *REST) List(selector labels.Selector) (runtime.Object, error) {
+func (rs *REST) List(label, field labels.Selector) (runtime.Object, error) {
-	if !selector.Empty() {
+	if !label.Empty() || !field.Empty() {
-		return nil, errors.New("label selectors are not supported on endpoints")
+		return nil, errors.New("label/field selectors are not supported on endpoints")
 	}
 	return rs.registry.ListEndpoints()
 }

pkg/registry/endpoint/rest_test.go

@@ -79,7 +79,7 @@ func TestEndpointsRegistryList(t *testing.T) {
 			{JSONBase: api.JSONBase{ID: "bar"}},
 		},
 	}
-	s, _ := storage.List(labels.Everything())
+	s, _ := storage.List(labels.Everything(), labels.Everything())
 	sl := s.(*api.EndpointsList)
 	if len(sl.Items) != 2 {
 		t.Fatalf("Expected 2 endpoints, but got %v", len(sl.Items))

pkg/registry/etcd/etcd.go

@@ -60,8 +60,15 @@ func makePodKey(podID string) string {
 	return "/registry/pods/" + podID
 }
 
-// ListPods obtains a list of pods that match selector.
+// ListPods obtains a list of pods with labels that match selector.
 func (r *Registry) ListPods(selector labels.Selector) (*api.PodList, error) {
+	return r.ListPodsPredicate(func(pod *api.Pod) bool {
+		return selector.Matches(labels.Set(pod.Labels))
+	})
+}
+
+// ListPodsPredicate obtains a list of pods that match filter.
+func (r *Registry) ListPodsPredicate(filter func(*api.Pod) bool) (*api.PodList, error) {
 	allPods := api.PodList{}
 	err := r.ExtractList("/registry/pods", &allPods.Items, &allPods.ResourceVersion)
 	if err != nil {
@@ -69,7 +76,7 @@ func (r *Registry) ListPods(selector labels.Selector) (*api.PodList, error) {
 	}
 	filtered := []api.Pod{}
 	for _, pod := range allPods.Items {
-		if selector.Matches(labels.Set(pod.Labels)) {
+		if filter(&pod) {
 			// TODO: Currently nothing sets CurrentState.Host. We need a feedback loop that sets
 			// the CurrentState.Host and Status fields. Here we pretend that reality perfectly
 			// matches our desires.

pkg/registry/minion/rest.go

@@ -86,7 +86,7 @@ func (rs *REST) Get(id string) (runtime.Object, error) {
 	return rs.toApiMinion(id), err
 }
 
-func (rs *REST) List(selector labels.Selector) (runtime.Object, error) {
+func (rs *REST) List(label, field labels.Selector) (runtime.Object, error) {
 	nameList, err := rs.registry.List()
 	if err != nil {
 		return nil, err

pkg/registry/minion/rest_test.go

@@ -67,7 +67,7 @@ func TestMinionREST(t *testing.T) {
 		t.Errorf("delete returned wrong error")
 	}
 
-	list, err := ms.List(labels.Everything())
+	list, err := ms.List(labels.Everything(), labels.Everything())
 	if err != nil {
 		t.Errorf("got error calling List")
 	}

pkg/registry/pod/registry.go

@@ -24,8 +24,10 @@ import (
 
 // Registry is an interface implemented by things that know how to store Pod objects.
 type Registry interface {
-	// ListPods obtains a list of pods that match selector.
+	// ListPods obtains a list of pods having labels which match selector.
 	ListPods(selector labels.Selector) (*api.PodList, error)
+	// ListPodsPredicate obtains a list of pods for which filter returns true.
+	ListPodsPredicate(filter func(*api.Pod) bool) (*api.PodList, error)
 	// Watch for new/changed/deleted pods
 	WatchPods(resourceVersion uint64, filter func(*api.Pod) bool) (watch.Interface, error)
 	// Get a specific pod

pkg/registry/pod/rest.go

@@ -107,8 +107,25 @@ func (rs *REST) Get(id string) (runtime.Object, error) {
 	return pod, err
 }
 
-func (rs *REST) List(selector labels.Selector) (runtime.Object, error) {
+func (rs *REST) podToSelectableFields(pod *api.Pod) labels.Set {
-	pods, err := rs.registry.ListPods(selector)
+	return labels.Set{
+		"ID": pod.ID,
+		"DesiredState.Status": string(pod.DesiredState.Status),
+		"DesiredState.Host":   pod.DesiredState.Host,
+	}
+}
+
+// filterFunc returns a predicate based on label & field selectors that can be passed to registry's
+// ListPods & WatchPods.
+func (rs *REST) filterFunc(label, field labels.Selector) func(*api.Pod) bool {
+	return func(pod *api.Pod) bool {
+		fields := rs.podToSelectableFields(pod)
+		return label.Matches(labels.Set(pod.Labels)) && field.Matches(fields)
+	}
+}
+
+func (rs *REST) List(label, field labels.Selector) (runtime.Object, error) {
+	pods, err := rs.registry.ListPodsPredicate(rs.filterFunc(label, field))
 	if err == nil {
 		for i := range pods.Items {
 			pod := &pods.Items[i]
@@ -122,14 +139,7 @@ func (rs *REST) List(selector labels.Selector) (runtime.Object, error) {
 
 // Watch begins watching for new, changed, or deleted pods.
 func (rs *REST) Watch(label, field labels.Selector, resourceVersion uint64) (watch.Interface, error) {
-	return rs.registry.WatchPods(resourceVersion, func(pod *api.Pod) bool {
+	return rs.registry.WatchPods(resourceVersion, rs.filterFunc(label, field))
-		fields := labels.Set{
-			"ID": pod.ID,
-			"DesiredState.Status": string(pod.DesiredState.Status),
-			"DesiredState.Host":   pod.DesiredState.Host,
-		}
-		return label.Matches(labels.Set(pod.Labels)) && field.Matches(fields)
-	})
 }
 
 func (*REST) New() runtime.Object {

pkg/registry/pod/rest_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 
 	"github.com/fsouza/go-dockerclient"
 )
@@ -129,7 +130,7 @@ func TestListPodsError(t *testing.T) {
 	storage := REST{
 		registry: podRegistry,
 	}
-	pods, err := storage.List(labels.Everything())
+	pods, err := storage.List(labels.Everything(), labels.Everything())
 	if err != podRegistry.Err {
 		t.Errorf("Expected %#v, Got %#v", podRegistry.Err, err)
 	}
@@ -143,7 +144,7 @@ func TestListEmptyPodList(t *testing.T) {
 	storage := REST{
 		registry: podRegistry,
 	}
-	pods, err := storage.List(labels.Everything())
+	pods, err := storage.List(labels.Everything(), labels.Everything())
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -175,7 +176,7 @@ func TestListPodList(t *testing.T) {
 	storage := REST{
 		registry: podRegistry,
 	}
-	podsObj, err := storage.List(labels.Everything())
+	podsObj, err := storage.List(labels.Everything(), labels.Everything())
 	pods := podsObj.(*api.PodList)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -192,6 +193,86 @@ func TestListPodList(t *testing.T) {
 	}
 }
 
+func TestListPodListSelection(t *testing.T) {
+	podRegistry := registrytest.NewPodRegistry(nil)
+	podRegistry.Pods = &api.PodList{
+		Items: []api.Pod{
+			{
+				JSONBase: api.JSONBase{ID: "foo"},
+			}, {
+				JSONBase:     api.JSONBase{ID: "bar"},
+				DesiredState: api.PodState{Host: "barhost"},
+			}, {
+				JSONBase:     api.JSONBase{ID: "baz"},
+				DesiredState: api.PodState{Status: "bazstatus"},
+			}, {
+				JSONBase: api.JSONBase{ID: "qux"},
+				Labels:   map[string]string{"label": "qux"},
+			}, {
+				JSONBase: api.JSONBase{ID: "zot"},
+			},
+		},
+	}
+	storage := REST{
+		registry: podRegistry,
+	}
+
+	table := []struct {
+		label, field string
+		expectedIDs  util.StringSet
+	}{
+		{
+			expectedIDs: util.NewStringSet("foo", "bar", "baz", "qux", "zot"),
+		}, {
+			field:       "ID=zot",
+			expectedIDs: util.NewStringSet("zot"),
+		}, {
+			label:       "label=qux",
+			expectedIDs: util.NewStringSet("qux"),
+		}, {
+			field:       "DesiredState.Status=bazstatus",
+			expectedIDs: util.NewStringSet("baz"),
+		}, {
+			field:       "DesiredState.Host=barhost",
+			expectedIDs: util.NewStringSet("bar"),
+		}, {
+			field:       "DesiredState.Host=",
+			expectedIDs: util.NewStringSet("foo", "baz", "qux", "zot"),
+		}, {
+			field:       "DesiredState.Host!=",
+			expectedIDs: util.NewStringSet("bar"),
+		},
+	}
+
+	for index, item := range table {
+		label, err := labels.ParseSelector(item.label)
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+			continue
+		}
+		field, err := labels.ParseSelector(item.field)
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+			continue
+		}
+		podsObj, err := storage.List(label, field)
+		if err != nil {
+			t.Errorf("unexpected error: %v", err)
+		}
+		pods := podsObj.(*api.PodList)
+
+		if e, a := len(item.expectedIDs), len(pods.Items); e != a {
+			t.Errorf("%v: Expected %v, got %v", index, e, a)
+		}
+		for _, pod := range pods.Items {
+			if !item.expectedIDs.Has(pod.ID) {
+				t.Errorf("%v: Unexpected pod %v", index, pod.ID)
+			}
+			t.Logf("%v: Got pod ID: %v", index, pod.ID)
+		}
+	}
+}
+
 func TestPodDecode(t *testing.T) {
 	podRegistry := registrytest.NewPodRegistry(nil)
 	storage := REST{

pkg/registry/registrytest/pod.go

@@ -40,7 +40,7 @@ func NewPodRegistry(pods *api.PodList) *PodRegistry {
 	}
 }
 
-func (r *PodRegistry) ListPods(selector labels.Selector) (*api.PodList, error) {
+func (r *PodRegistry) ListPodsPredicate(filter func(*api.Pod) bool) (*api.PodList, error) {
 	r.Lock()
 	defer r.Unlock()
 	if r.Err != nil {
@@ -48,7 +48,7 @@ func (r *PodRegistry) ListPods(selector labels.Selector) (*api.PodList, error) {
 	}
 	var filtered []api.Pod
 	for _, pod := range r.Pods.Items {
-		if selector.Matches(labels.Set(pod.Labels)) {
+		if filter(&pod) {
 			filtered = append(filtered, pod)
 		}
 	}
@@ -57,6 +57,12 @@ func (r *PodRegistry) ListPods(selector labels.Selector) (*api.PodList, error) {
 	return &pods, nil
 }
 
+func (r *PodRegistry) ListPods(selector labels.Selector) (*api.PodList, error) {
+	return r.ListPodsPredicate(func(pod *api.Pod) bool {
+		return selector.Matches(labels.Set(pod.Labels))
+	})
+}
+
 func (r *PodRegistry) WatchPods(resourceVersion uint64, filter func(*api.Pod) bool) (watch.Interface, error) {
 	// TODO: wire filter down into the mux; it needs access to current and previous state :(
 	return r.mux.Watch(), nil

pkg/registry/service/rest.go

@@ -113,14 +113,15 @@ func (rs *REST) Get(id string) (runtime.Object, error) {
 	return s, err
 }
 
-func (rs *REST) List(selector labels.Selector) (runtime.Object, error) {
+// TODO: implement field selector?
+func (rs *REST) List(label, field labels.Selector) (runtime.Object, error) {
 	list, err := rs.registry.ListServices()
 	if err != nil {
 		return nil, err
 	}
 	var filtered []api.Service
 	for _, service := range list.Items {
-		if selector.Matches(labels.Set(service.Labels)) {
+		if label.Matches(labels.Set(service.Labels)) {
 			filtered = append(filtered, service)
 		}
 	}

pkg/registry/service/rest_test.go

@@ -319,7 +319,7 @@ func TestServiceRegistryList(t *testing.T) {
 		Selector: map[string]string{"bar2": "baz2"},
 	})
 	registry.List.ResourceVersion = 1
-	s, _ := storage.List(labels.Everything())
+	s, _ := storage.List(labels.Everything(), labels.Everything())
 	sl := s.(*api.ServiceList)
 	if len(fakeCloud.Calls) != 0 {
 		t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls)