diff --git a/staging/src/k8s.io/apimachinery/pkg/fields/selector.go b/staging/src/k8s.io/apimachinery/pkg/fields/selector.go index 69014d0e91c..1305dde086c 100644 --- a/staging/src/k8s.io/apimachinery/pkg/fields/selector.go +++ b/staging/src/k8s.io/apimachinery/pkg/fields/selector.go @@ -40,6 +40,8 @@ type Selector interface { // Transform returns a new copy of the selector after TransformFunc has been // applied to the entire selector, or an error if fn returns an error. + // If for a given requirement both field and value are transformed to empty + // string, the requirement is skipped. Transform(fn TransformFunc) (Selector, error) // Requirements converts this interface to Requirements to expose @@ -79,6 +81,9 @@ func (t *hasTerm) Transform(fn TransformFunc) (Selector, error) { if err != nil { return nil, err } + if len(field) == 0 && len(value) == 0 { + return Everything(), nil + } return &hasTerm{field, value}, nil } @@ -115,6 +120,9 @@ func (t *notHasTerm) Transform(fn TransformFunc) (Selector, error) { if err != nil { return nil, err } + if len(field) == 0 && len(value) == 0 { + return Everything(), nil + } return ¬HasTerm{field, value}, nil } @@ -169,13 +177,15 @@ func (t andTerm) RequiresExactMatch(field string) (string, bool) { } func (t andTerm) Transform(fn TransformFunc) (Selector, error) { - next := make([]Selector, len([]Selector(t))) - for i, s := range []Selector(t) { + next := make([]Selector, 0, len([]Selector(t))) + for _, s := range []Selector(t) { n, err := s.Transform(fn) if err != nil { return nil, err } - next[i] = n + if !n.Empty() { + next = append(next, n) + } } return andTerm(next), nil } diff --git a/staging/src/k8s.io/apimachinery/pkg/fields/selector_test.go b/staging/src/k8s.io/apimachinery/pkg/fields/selector_test.go index 458d1574a53..ff6245ca9e1 100644 --- a/staging/src/k8s.io/apimachinery/pkg/fields/selector_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/fields/selector_test.go @@ -325,3 +325,73 @@ func TestRequiresExactMatch(t *testing.T) { } } } + +func TestTransform(t *testing.T) { + testCases := []struct { + name string + selector string + transform func(field, value string) (string, string, error) + result string + isEmpty bool + }{ + { + name: "empty selector", + selector: "", + transform: func(field, value string) (string, string, error) { return field, value, nil }, + result: "", + isEmpty: true, + }, + { + name: "no-op transform", + selector: "a=b,c=d", + transform: func(field, value string) (string, string, error) { return field, value, nil }, + result: "a=b,c=d", + isEmpty: false, + }, + { + name: "transform one field", + selector: "a=b,c=d", + transform: func(field, value string) (string, string, error) { + if field == "a" { + return "e", "f", nil + } + return field, value, nil + }, + result: "e=f,c=d", + isEmpty: false, + }, + { + name: "remove field to make empty", + selector: "a=b", + transform: func(field, value string) (string, string, error) { return "", "", nil }, + result: "", + isEmpty: true, + }, + { + name: "remove only one field", + selector: "a=b,c=d,e=f", + transform: func(field, value string) (string, string, error) { + if field == "c" { + return "", "", nil + } + return field, value, nil + }, + result: "a=b,e=f", + isEmpty: false, + }, + } + + for i, tc := range testCases { + result, err := ParseAndTransformSelector(tc.selector, tc.transform) + if err != nil { + t.Errorf("[%d] unexpected error during Transform: %v", i, err) + } + if result.Empty() != tc.isEmpty { + t.Errorf("[%d] expected empty: %t, got: %t", i, tc.isEmpty, result.Empty) + } + if result.String() != tc.result { + t.Errorf("[%d] unexpected result: %s", i, result.String()) + } + } + +} diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index a6867426c5d..d08a5ba78a5 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -1032,7 +1032,16 @@ func (e *Store) Watch(ctx genericapirequest.Context, options *metainternalversio func (e *Store) WatchPredicate(ctx genericapirequest.Context, p storage.SelectionPredicate, resourceVersion string) (watch.Interface, error) { if name, ok := p.MatchesSingle(); ok { if key, err := e.KeyFunc(ctx, name); err == nil { - w, err := e.Storage.Watch(ctx, key, resourceVersion, p) + // For performance reasons, we can optimize the further computations of + // selector, by removing then "matches-single" fields, because they are + // already satisfied by choosing appropriate key. + sp, err := p.RemoveMatchesSingleRequirements() + if err != nil { + glog.Warningf("Couldn't remove matches-single requirements: %v", err) + // Since we couldn't optimize selector, reset to the original one. + sp = p + } + w, err := e.Storage.Watch(ctx, key, resourceVersion, sp) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go b/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go index c4f79288d94..8878245d1f2 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go @@ -65,16 +65,41 @@ func (s *SelectionPredicate) MatchesLabelsAndFields(l labels.Set, f fields.Set) return matched } +const matchesSingleField = "metadata.name" + +func removeMatchesSingleField(field, value string) (string, string, error) { + if field == matchesSingleField { + return "", "", nil + } + return field, value, nil +} + // MatchesSingle will return (name, true) if and only if s.Field matches on the object's // name. func (s *SelectionPredicate) MatchesSingle() (string, bool) { - // TODO: should be namespace.name - if name, ok := s.Field.RequiresExactMatch("metadata.name"); ok { + if name, ok := s.Field.RequiresExactMatch(matchesSingleField); ok { return name, true } return "", false } +func (s *SelectionPredicate) RemoveMatchesSingleRequirements() (SelectionPredicate, error) { + var fieldsSelector fields.Selector + if s.Field != nil { + var err error + fieldsSelector, err = s.Field.Transform(removeMatchesSingleField) + if err != nil { + return SelectionPredicate{}, err + } + } + return SelectionPredicate{ + Label: s.Label, + Field: fieldsSelector, + GetAttrs: s.GetAttrs, + IndexFields: s.IndexFields, + }, nil +} + // For any index defined by IndexFields, if a matcher can match only (a subset) // of objects that return for a given index, a pair (, ) // wil be returned.