Skip to content

Commit 452eedb

Browse files
committed
client: client.MatchingFields support multiple indexes
add support for multiple indexes when using client.MatchingFields
1 parent bb09db8 commit 452eedb

File tree

7 files changed

+148
-49
lines changed

7 files changed

+148
-49
lines changed

pkg/cache/cache_test.go

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1941,13 +1941,13 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
19411941
Expect(err).NotTo(HaveOccurred())
19421942

19431943
By("indexing the Namespace objects with fixed values before starting")
1944-
pod := &corev1.Namespace{}
1944+
ns := &corev1.Namespace{}
19451945
indexerValues := []string{"a", "b", "c"}
19461946
fieldName := "fixedValues"
19471947
indexFunc := func(obj client.Object) []string {
19481948
return indexerValues
19491949
}
1950-
Expect(informer.IndexField(context.TODO(), pod, fieldName, indexFunc)).To(Succeed())
1950+
Expect(informer.IndexField(context.TODO(), ns, fieldName, indexFunc)).To(Succeed())
19511951

19521952
By("running the cache and waiting for it to sync")
19531953
go func() {
@@ -1968,6 +1968,51 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
19681968
Expect(indexerValues[1]).To(Equal("b"))
19691969
Expect(indexerValues[2]).To(Equal("c"))
19701970
})
1971+
1972+
It("should be able to matching fields with multiple indexes", func() {
1973+
By("creating the cache")
1974+
informer, err := cache.New(cfg, cache.Options{})
1975+
Expect(err).NotTo(HaveOccurred())
1976+
1977+
pod := &corev1.Pod{}
1978+
By("indexing pods with namespace before starting")
1979+
fieldName1 := "indexByNamespace"
1980+
indexFunc1 := func(obj client.Object) []string {
1981+
return []string{obj.(*corev1.Pod).Namespace}
1982+
}
1983+
Expect(informer.IndexField(context.TODO(), pod, fieldName1, indexFunc1)).To(Succeed())
1984+
By("indexing pods with restart policy before starting")
1985+
fieldName2 := "indexByPolicy"
1986+
indexFunc2 := func(obj client.Object) []string {
1987+
return []string{string(obj.(*corev1.Pod).Spec.RestartPolicy)}
1988+
}
1989+
Expect(informer.IndexField(context.TODO(), pod, fieldName2, indexFunc2)).To(Succeed())
1990+
1991+
By("running the cache and waiting for it to sync")
1992+
go func() {
1993+
defer GinkgoRecover()
1994+
Expect(informer.Start(informerCacheCtx)).To(Succeed())
1995+
}()
1996+
Expect(informer.WaitForCacheSync(informerCacheCtx)).To(BeTrue())
1997+
1998+
By("listing pods with namespace index")
1999+
listObj := &corev1.PodList{}
2000+
Expect(informer.List(context.Background(), listObj,
2001+
client.MatchingFields{fieldName1: testNamespaceTwo})).To(Succeed())
2002+
Expect(listObj.Items).To(HaveLen(3))
2003+
2004+
By("listing pods with restart policy index")
2005+
listObj = &corev1.PodList{}
2006+
Expect(informer.List(context.Background(), listObj,
2007+
client.MatchingFields{fieldName2: string(corev1.RestartPolicyAlways)})).To(Succeed())
2008+
Expect(listObj.Items).To(HaveLen(2))
2009+
2010+
By("listing Namespaces with both fixed indexers 1 and 2")
2011+
listObj = &corev1.PodList{}
2012+
Expect(informer.List(context.Background(), listObj,
2013+
client.MatchingFields{fieldName1: testNamespaceTwo, fieldName2: string(corev1.RestartPolicyAlways)})).To(Succeed())
2014+
Expect(listObj.Items).To(HaveLen(2))
2015+
})
19712016
})
19722017
Context("with unstructured objects", func() {
19732018
It("should be able to get informer for the object", func() {

pkg/cache/internal/cache_reader.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"k8s.io/apimachinery/pkg/labels"
2727
"k8s.io/apimachinery/pkg/runtime"
2828
"k8s.io/apimachinery/pkg/runtime/schema"
29+
"k8s.io/apimachinery/pkg/util/sets"
2930
"k8s.io/client-go/tools/cache"
3031

3132
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -117,16 +118,14 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli
117118

118119
switch {
119120
case listOpts.FieldSelector != nil:
120-
// TODO(directxman12): support more complicated field selectors by
121-
// combining multiple indices, GetIndexers, etc
122-
field, val, requiresExact := selector.RequiresExactMatch(listOpts.FieldSelector)
121+
requires, requiresExact := selector.RequiresExactMatch(listOpts.FieldSelector)
123122
if !requiresExact {
124123
return fmt.Errorf("non-exact field matches are not supported by the cache")
125124
}
126125
// list all objects by the field selector. If this is namespaced and we have one, ask for the
127126
// namespaced index key. Otherwise, ask for the non-namespaced variant by using the fake "all namespaces"
128127
// namespace.
129-
objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(listOpts.Namespace, val))
128+
objs, err = byIndexes(c.indexer, requires, listOpts.Namespace)
130129
case listOpts.Namespace != "":
131130
objs, err = c.indexer.ByIndex(cache.NamespaceIndex, listOpts.Namespace)
132131
default:
@@ -178,6 +177,42 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli
178177
return apimeta.SetList(out, runtimeObjs)
179178
}
180179

180+
func byIndexes(indexer cache.Indexer, requires map[string]string, namespace string) ([]interface{}, error) {
181+
var (
182+
keysSet = sets.NewString()
183+
keys []string
184+
err error
185+
)
186+
for fieldKey, fieldVal := range requires {
187+
keys, err = indexer.IndexKeys(FieldIndexName(fieldKey), KeyToNamespacedKey(namespace, fieldVal))
188+
if err != nil {
189+
return nil, err
190+
}
191+
if len(keys) == 0 {
192+
return nil, nil
193+
}
194+
if keysSet.Len() == 0 {
195+
keysSet = keysSet.Insert(keys...)
196+
} else {
197+
keysSet = keysSet.Intersection(sets.NewString(keys...))
198+
if keysSet.Len() == 0 {
199+
return nil, nil
200+
}
201+
}
202+
}
203+
objs := make([]interface{}, 0, keysSet.Len())
204+
for key := range keysSet {
205+
obj, exist, err := indexer.GetByKey(key)
206+
if err != nil {
207+
return nil, err
208+
}
209+
if exist {
210+
objs = append(objs, obj)
211+
}
212+
}
213+
return objs, nil
214+
}
215+
181216
// objectKeyToStorageKey converts an object key to store key.
182217
// It's akin to MetaNamespaceKeyFunc. It's separate from
183218
// String to allow keeping the key format easily in sync with

pkg/client/example_test.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ func ExampleClient_deleteAllOf() {
247247
}
248248

249249
// This example shows how to set up and consume a field selector over a pod's volumes' secretName field.
250-
func ExampleFieldIndexer_secretName() {
250+
func ExampleFieldIndexer_secretNameNode() {
251251
// someIndexer is a FieldIndexer over a Cache
252252
_ = someIndexer.IndexField(context.TODO(), &corev1.Pod{}, "spec.volumes.secret.secretName", func(o client.Object) []string {
253253
var res []string
@@ -261,8 +261,20 @@ func ExampleFieldIndexer_secretName() {
261261
return res
262262
})
263263

264+
_ = someIndexer.IndexField(context.TODO(), &corev1.Pod{}, "spec.NodeName", func(o client.Object) []string {
265+
nodeName := o.(*corev1.Pod).Spec.NodeName
266+
if nodeName != "" {
267+
return []string{nodeName}
268+
}
269+
return nil
270+
})
271+
264272
// elsewhere (e.g. in your reconciler)
265273
mySecretName := "someSecret" // derived from the reconcile.Request, for instance
274+
myNode := "master-0"
266275
var podsWithSecrets corev1.PodList
267-
_ = c.List(context.Background(), &podsWithSecrets, client.MatchingFields{"spec.volumes.secret.secretName": mySecretName})
276+
_ = c.List(context.Background(), &podsWithSecrets, client.MatchingFields{
277+
"spec.volumes.secret.secretName": mySecretName,
278+
"spec.NodeName": myNode,
279+
})
268280
}

pkg/client/fake/client.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -586,9 +586,7 @@ func (c *fakeClient) filterList(list []runtime.Object, gvk schema.GroupVersionKi
586586
}
587587

588588
func (c *fakeClient) filterWithFields(list []runtime.Object, gvk schema.GroupVersionKind, fs fields.Selector) ([]runtime.Object, error) {
589-
// We only allow filtering on the basis of a single field to ensure consistency with the
590-
// behavior of the cache reader (which we're faking here).
591-
fieldKey, fieldVal, requiresExact := selector.RequiresExactMatch(fs)
589+
requires, requiresExact := selector.RequiresExactMatch(fs)
592590
if !requiresExact {
593591
return nil, fmt.Errorf("field selector %s is not in one of the two supported forms \"key==val\" or \"key=val\"",
594592
fs)
@@ -597,15 +595,24 @@ func (c *fakeClient) filterWithFields(list []runtime.Object, gvk schema.GroupVer
597595
// Field selection is mimicked via indexes, so there's no sane answer this function can give
598596
// if there are no indexes registered for the GroupVersionKind of the objects in the list.
599597
indexes := c.indexes[gvk]
600-
if len(indexes) == 0 || indexes[fieldKey] == nil {
601-
return nil, fmt.Errorf("List on GroupVersionKind %v specifies selector on field %s, but no "+
602-
"index with name %s has been registered for GroupVersionKind %v", gvk, fieldKey, fieldKey, gvk)
598+
for fieldKey := range requires {
599+
if len(indexes) == 0 || indexes[fieldKey] == nil {
600+
return nil, fmt.Errorf("List on GroupVersionKind %v specifies selector on field %s, but no "+
601+
"index with name %s has been registered for GroupVersionKind %v", gvk, fieldKey, fieldKey, gvk)
602+
}
603603
}
604604

605-
indexExtractor := indexes[fieldKey]
606605
filteredList := make([]runtime.Object, 0, len(list))
607606
for _, obj := range list {
608-
if c.objMatchesFieldSelector(obj, indexExtractor, fieldVal) {
607+
ok := true
608+
for fieldKey, fieldVal := range requires {
609+
indexExtractor := indexes[fieldKey]
610+
if !c.objMatchesFieldSelector(obj, indexExtractor, fieldVal) {
611+
ok = false
612+
break
613+
}
614+
}
615+
if ok {
609616
filteredList = append(filteredList, obj)
610617
}
611618
}

pkg/client/fake/client_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,14 +1333,15 @@ var _ = Describe("Fake client", func() {
13331333
Expect(list.Items).To(BeEmpty())
13341334
})
13351335

1336-
It("errors when field selector uses two requirements", func() {
1336+
It("no error when field selector uses two requirements", func() {
13371337
listOpts := &client.ListOptions{
13381338
FieldSelector: fields.AndSelectors(
13391339
fields.OneTermEqualSelector("spec.replicas", "1"),
13401340
fields.OneTermEqualSelector("spec.strategy.type", string(appsv1.RecreateDeploymentStrategyType)),
13411341
)}
1342-
err := cl.List(context.Background(), &appsv1.DeploymentList{}, listOpts)
1343-
Expect(err).To(HaveOccurred())
1342+
list := &appsv1.DeploymentList{}
1343+
Expect(cl.List(context.Background(), list, listOpts)).To(Succeed())
1344+
Expect(list.Items).To(ConsistOf(*dep))
13441345
})
13451346
})
13461347
})

pkg/internal/field/selector/utils.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,18 @@ import (
2222
)
2323

2424
// RequiresExactMatch checks if the given field selector is of the form `k=v` or `k==v`.
25-
func RequiresExactMatch(sel fields.Selector) (field, val string, required bool) {
25+
func RequiresExactMatch(sel fields.Selector) (requires map[string]string, required bool) {
2626
reqs := sel.Requirements()
27-
if len(reqs) != 1 {
28-
return "", "", false
27+
if len(reqs) == 0 {
28+
return nil, false
2929
}
30-
req := reqs[0]
31-
if req.Operator != selection.Equals && req.Operator != selection.DoubleEquals {
32-
return "", "", false
30+
31+
requires = make(map[string]string, len(reqs))
32+
for _, req := range reqs {
33+
if req.Operator != selection.Equals && req.Operator != selection.DoubleEquals {
34+
return nil, false
35+
}
36+
requires[req.Field] = req.Value
3337
}
34-
return req.Field, req.Value, true
38+
return requires, true
3539
}

pkg/internal/field/selector/utils_test.go

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,62 +27,57 @@ import (
2727
var _ = Describe("RequiresExactMatch function", func() {
2828

2929
It("Returns false when the selector matches everything", func() {
30-
_, _, requiresExactMatch := RequiresExactMatch(fields.Everything())
30+
_, requiresExactMatch := RequiresExactMatch(fields.Everything())
3131
Expect(requiresExactMatch).To(BeFalse())
3232
})
3333

3434
It("Returns false when the selector matches nothing", func() {
35-
_, _, requiresExactMatch := RequiresExactMatch(fields.Nothing())
35+
_, requiresExactMatch := RequiresExactMatch(fields.Nothing())
3636
Expect(requiresExactMatch).To(BeFalse())
3737
})
3838

3939
It("Returns false when the selector has the form key!=val", func() {
40-
_, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val"))
40+
_, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val"))
4141
Expect(requiresExactMatch).To(BeFalse())
4242
})
4343

44-
It("Returns false when the selector has the form key1==val1,key2==val2", func() {
45-
_, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key1==val1,key2==val2"))
46-
Expect(requiresExactMatch).To(BeFalse())
44+
It("Returns true when the selector has the form key1==val1,key2==val2", func() {
45+
_, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key1==val1,key2==val2"))
46+
Expect(requiresExactMatch).To(BeTrue())
4747
})
4848

4949
It("Returns true when the selector has the form key==val", func() {
50-
_, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key==val"))
50+
_, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key==val"))
5151
Expect(requiresExactMatch).To(BeTrue())
5252
})
5353

5454
It("Returns true when the selector has the form key=val", func() {
55-
_, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key=val"))
55+
_, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key=val"))
5656
Expect(requiresExactMatch).To(BeTrue())
5757
})
5858

5959
It("Returns empty key and value when the selector matches everything", func() {
60-
key, val, _ := RequiresExactMatch(fields.Everything())
61-
Expect(key).To(Equal(""))
62-
Expect(val).To(Equal(""))
60+
requires, _ := RequiresExactMatch(fields.Everything())
61+
Expect(requires).To(BeNil())
6362
})
6463

6564
It("Returns empty key and value when the selector matches nothing", func() {
66-
key, val, _ := RequiresExactMatch(fields.Nothing())
67-
Expect(key).To(Equal(""))
68-
Expect(val).To(Equal(""))
65+
requires, _ := RequiresExactMatch(fields.Nothing())
66+
Expect(requires).To(BeNil())
6967
})
7068

7169
It("Returns empty key and value when the selector has the form key!=val", func() {
72-
key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val"))
73-
Expect(key).To(Equal(""))
74-
Expect(val).To(Equal(""))
70+
requires, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val"))
71+
Expect(requires).To(BeNil())
7572
})
7673

7774
It("Returns key and value when the selector has the form key==val", func() {
78-
key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key==val"))
79-
Expect(key).To(Equal("key"))
80-
Expect(val).To(Equal("val"))
75+
requires, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key==val"))
76+
Expect(requires).To(HaveKeyWithValue("key", "val"))
8177
})
8278

8379
It("Returns key and value when the selector has the form key=val", func() {
84-
key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key=val"))
85-
Expect(key).To(Equal("key"))
86-
Expect(val).To(Equal("val"))
80+
requires, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key=val"))
81+
Expect(requires).To(HaveKeyWithValue("key", "val"))
8782
})
8883
})

0 commit comments

Comments
 (0)