diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index f926b85b2f9..862944439b2 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/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, } diff --git a/pkg/apiserver/interfaces.go b/pkg/apiserver/interfaces.go index c2a4f2c5751..0c1fb2cce6e 100644 --- a/pkg/apiserver/interfaces.go +++ b/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(labels.Selector) (runtime.Object, error) + List(label, field 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 diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index 4b6977b9e8d..03208acb143 100644 --- a/pkg/apiserver/resthandler.go +++ b/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 diff --git a/pkg/registry/binding/rest.go b/pkg/registry/binding/rest.go index 980ecf4a8ab..bfc7b6e150f 100644 --- a/pkg/registry/binding/rest.go +++ b/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") } diff --git a/pkg/registry/binding/rest_test.go b/pkg/registry/binding/rest_test.go index 3bbc216f287..4a1b8f30077 100644 --- a/pkg/registry/binding/rest_test.go +++ b/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 diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index d494c7791af..cfd39bcbcf7 100644 --- a/pkg/registry/controller/rest.go +++ b/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) } diff --git a/pkg/registry/controller/rest_test.go b/pkg/registry/controller/rest_test.go index 8768d7847b6..ee8f298777d 100644 --- a/pkg/registry/controller/rest_test.go +++ b/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) diff --git a/pkg/registry/endpoint/rest.go b/pkg/registry/endpoint/rest.go index 554dadd21eb..7e620dfcde0 100644 --- a/pkg/registry/endpoint/rest.go +++ b/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) { - if !selector.Empty() { - return nil, errors.New("label selectors are not supported on endpoints") +func (rs *REST) List(label, field labels.Selector) (runtime.Object, error) { + if !label.Empty() || !field.Empty() { + return nil, errors.New("label/field selectors are not supported on endpoints") } return rs.registry.ListEndpoints() } diff --git a/pkg/registry/endpoint/rest_test.go b/pkg/registry/endpoint/rest_test.go index e14e9aaa120..978babb0e9f 100644 --- a/pkg/registry/endpoint/rest_test.go +++ b/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)) diff --git a/pkg/registry/etcd/etcd.go b/pkg/registry/etcd/etcd.go index c74b78a9fa5..e95c0e11abd 100644 --- a/pkg/registry/etcd/etcd.go +++ b/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. diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index e66a0f9b6e1..7b512f68d6e 100644 --- a/pkg/registry/minion/rest.go +++ b/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 diff --git a/pkg/registry/minion/rest_test.go b/pkg/registry/minion/rest_test.go index d1a71c883e9..3b4c79265c8 100644 --- a/pkg/registry/minion/rest_test.go +++ b/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") } diff --git a/pkg/registry/pod/registry.go b/pkg/registry/pod/registry.go index ef12477e060..876e9118208 100644 --- a/pkg/registry/pod/registry.go +++ b/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 diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index 136197d5dd5..48e4b82fed2 100644 --- a/pkg/registry/pod/rest.go +++ b/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) { - pods, err := rs.registry.ListPods(selector) +func (rs *REST) podToSelectableFields(pod *api.Pod) labels.Set { + 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 { - 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) - }) + return rs.registry.WatchPods(resourceVersion, rs.filterFunc(label, field)) } func (*REST) New() runtime.Object { diff --git a/pkg/registry/pod/rest_test.go b/pkg/registry/pod/rest_test.go index a06dc5fbe79..e7dfea42da5 100644 --- a/pkg/registry/pod/rest_test.go +++ b/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{ diff --git a/pkg/registry/registrytest/pod.go b/pkg/registry/registrytest/pod.go index 27f33ff74c6..0634ac1a770 100644 --- a/pkg/registry/registrytest/pod.go +++ b/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 diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 14740cec876..b0315af43ac 100644 --- a/pkg/registry/service/rest.go +++ b/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) } } diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index 9ec77bcb4eb..b9d3d99585e 100644 --- a/pkg/registry/service/rest_test.go +++ b/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)