diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index bff0c87083..16ebd25736 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -1941,13 +1941,13 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca Expect(err).NotTo(HaveOccurred()) By("indexing the Namespace objects with fixed values before starting") - pod := &corev1.Namespace{} + ns := &corev1.Namespace{} indexerValues := []string{"a", "b", "c"} fieldName := "fixedValues" indexFunc := func(obj client.Object) []string { return indexerValues } - Expect(informer.IndexField(context.TODO(), pod, fieldName, indexFunc)).To(Succeed()) + Expect(informer.IndexField(context.TODO(), ns, fieldName, indexFunc)).To(Succeed()) By("running the cache and waiting for it to sync") go func() { @@ -1968,6 +1968,51 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca Expect(indexerValues[1]).To(Equal("b")) Expect(indexerValues[2]).To(Equal("c")) }) + + It("should be able to matching fields with multiple indexes", func() { + By("creating the cache") + informer, err := cache.New(cfg, cache.Options{}) + Expect(err).NotTo(HaveOccurred()) + + pod := &corev1.Pod{} + By("indexing pods with namespace before starting") + fieldName1 := "indexByNamespace" + indexFunc1 := func(obj client.Object) []string { + return []string{obj.(*corev1.Pod).Namespace} + } + Expect(informer.IndexField(context.TODO(), pod, fieldName1, indexFunc1)).To(Succeed()) + By("indexing pods with restart policy before starting") + fieldName2 := "indexByPolicy" + indexFunc2 := func(obj client.Object) []string { + return []string{string(obj.(*corev1.Pod).Spec.RestartPolicy)} + } + Expect(informer.IndexField(context.TODO(), pod, fieldName2, indexFunc2)).To(Succeed()) + + By("running the cache and waiting for it to sync") + go func() { + defer GinkgoRecover() + Expect(informer.Start(informerCacheCtx)).To(Succeed()) + }() + Expect(informer.WaitForCacheSync(informerCacheCtx)).To(BeTrue()) + + By("listing pods with namespace index") + listObj := &corev1.PodList{} + Expect(informer.List(context.Background(), listObj, + client.MatchingFields{fieldName1: testNamespaceTwo})).To(Succeed()) + Expect(listObj.Items).To(HaveLen(3)) + + By("listing pods with restart policy index") + listObj = &corev1.PodList{} + Expect(informer.List(context.Background(), listObj, + client.MatchingFields{fieldName2: string(corev1.RestartPolicyAlways)})).To(Succeed()) + Expect(listObj.Items).To(HaveLen(2)) + + By("listing Namespaces with both fixed indexers 1 and 2") + listObj = &corev1.PodList{} + Expect(informer.List(context.Background(), listObj, + client.MatchingFields{fieldName1: testNamespaceTwo, fieldName2: string(corev1.RestartPolicyAlways)})).To(Succeed()) + Expect(listObj.Items).To(HaveLen(2)) + }) }) Context("with unstructured objects", func() { It("should be able to get informer for the object", func() { diff --git a/pkg/cache/internal/cache_reader.go b/pkg/cache/internal/cache_reader.go index eb941f034e..9a586c7d6b 100644 --- a/pkg/cache/internal/cache_reader.go +++ b/pkg/cache/internal/cache_reader.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/cache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -117,16 +118,14 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli switch { case listOpts.FieldSelector != nil: - // TODO(directxman12): support more complicated field selectors by - // combining multiple indices, GetIndexers, etc - field, val, requiresExact := selector.RequiresExactMatch(listOpts.FieldSelector) + requires, requiresExact := selector.RequiresExactMatch(listOpts.FieldSelector) if !requiresExact { return fmt.Errorf("non-exact field matches are not supported by the cache") } // list all objects by the field selector. If this is namespaced and we have one, ask for the // namespaced index key. Otherwise, ask for the non-namespaced variant by using the fake "all namespaces" // namespace. - objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(listOpts.Namespace, val)) + objs, err = byIndexes(c.indexer, requires, listOpts.Namespace) case listOpts.Namespace != "": objs, err = c.indexer.ByIndex(cache.NamespaceIndex, listOpts.Namespace) default: @@ -178,6 +177,42 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli return apimeta.SetList(out, runtimeObjs) } +func byIndexes(indexer cache.Indexer, requires map[string]string, namespace string) ([]interface{}, error) { + var ( + keysSet = sets.NewString() + keys []string + err error + ) + for fieldKey, fieldVal := range requires { + keys, err = indexer.IndexKeys(FieldIndexName(fieldKey), KeyToNamespacedKey(namespace, fieldVal)) + if err != nil { + return nil, err + } + if len(keys) == 0 { + return nil, nil + } + if keysSet.Len() == 0 { + keysSet = keysSet.Insert(keys...) + } else { + keysSet = keysSet.Intersection(sets.NewString(keys...)) + if keysSet.Len() == 0 { + return nil, nil + } + } + } + objs := make([]interface{}, 0, keysSet.Len()) + for key := range keysSet { + obj, exist, err := indexer.GetByKey(key) + if err != nil { + return nil, err + } + if exist { + objs = append(objs, obj) + } + } + return objs, nil +} + // objectKeyToStorageKey converts an object key to store key. // It's akin to MetaNamespaceKeyFunc. It's separate from // String to allow keeping the key format easily in sync with diff --git a/pkg/client/example_test.go b/pkg/client/example_test.go index 7d4cb8c616..db002a4c69 100644 --- a/pkg/client/example_test.go +++ b/pkg/client/example_test.go @@ -247,7 +247,7 @@ func ExampleClient_deleteAllOf() { } // This example shows how to set up and consume a field selector over a pod's volumes' secretName field. -func ExampleFieldIndexer_secretName() { +func ExampleFieldIndexer_secretNameNode() { // someIndexer is a FieldIndexer over a Cache _ = someIndexer.IndexField(context.TODO(), &corev1.Pod{}, "spec.volumes.secret.secretName", func(o client.Object) []string { var res []string @@ -261,8 +261,20 @@ func ExampleFieldIndexer_secretName() { return res }) + _ = someIndexer.IndexField(context.TODO(), &corev1.Pod{}, "spec.NodeName", func(o client.Object) []string { + nodeName := o.(*corev1.Pod).Spec.NodeName + if nodeName != "" { + return []string{nodeName} + } + return nil + }) + // elsewhere (e.g. in your reconciler) mySecretName := "someSecret" // derived from the reconcile.Request, for instance + myNode := "master-0" var podsWithSecrets corev1.PodList - _ = c.List(context.Background(), &podsWithSecrets, client.MatchingFields{"spec.volumes.secret.secretName": mySecretName}) + _ = c.List(context.Background(), &podsWithSecrets, client.MatchingFields{ + "spec.volumes.secret.secretName": mySecretName, + "spec.NodeName": myNode, + }) } diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index b3819926c0..3db2d83600 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -586,9 +586,7 @@ func (c *fakeClient) filterList(list []runtime.Object, gvk schema.GroupVersionKi } func (c *fakeClient) filterWithFields(list []runtime.Object, gvk schema.GroupVersionKind, fs fields.Selector) ([]runtime.Object, error) { - // We only allow filtering on the basis of a single field to ensure consistency with the - // behavior of the cache reader (which we're faking here). - fieldKey, fieldVal, requiresExact := selector.RequiresExactMatch(fs) + requires, requiresExact := selector.RequiresExactMatch(fs) if !requiresExact { return nil, fmt.Errorf("field selector %s is not in one of the two supported forms \"key==val\" or \"key=val\"", fs) @@ -597,15 +595,24 @@ func (c *fakeClient) filterWithFields(list []runtime.Object, gvk schema.GroupVer // Field selection is mimicked via indexes, so there's no sane answer this function can give // if there are no indexes registered for the GroupVersionKind of the objects in the list. indexes := c.indexes[gvk] - if len(indexes) == 0 || indexes[fieldKey] == nil { - return nil, fmt.Errorf("List on GroupVersionKind %v specifies selector on field %s, but no "+ - "index with name %s has been registered for GroupVersionKind %v", gvk, fieldKey, fieldKey, gvk) + for fieldKey := range requires { + if len(indexes) == 0 || indexes[fieldKey] == nil { + return nil, fmt.Errorf("List on GroupVersionKind %v specifies selector on field %s, but no "+ + "index with name %s has been registered for GroupVersionKind %v", gvk, fieldKey, fieldKey, gvk) + } } - indexExtractor := indexes[fieldKey] filteredList := make([]runtime.Object, 0, len(list)) for _, obj := range list { - if c.objMatchesFieldSelector(obj, indexExtractor, fieldVal) { + ok := true + for fieldKey, fieldVal := range requires { + indexExtractor := indexes[fieldKey] + if !c.objMatchesFieldSelector(obj, indexExtractor, fieldVal) { + ok = false + break + } + } + if ok { filteredList = append(filteredList, obj) } } diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index 810cf5499c..2cec470e49 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -1333,14 +1333,15 @@ var _ = Describe("Fake client", func() { Expect(list.Items).To(BeEmpty()) }) - It("errors when field selector uses two requirements", func() { + It("no error when field selector uses two requirements", func() { listOpts := &client.ListOptions{ FieldSelector: fields.AndSelectors( fields.OneTermEqualSelector("spec.replicas", "1"), fields.OneTermEqualSelector("spec.strategy.type", string(appsv1.RecreateDeploymentStrategyType)), )} - err := cl.List(context.Background(), &appsv1.DeploymentList{}, listOpts) - Expect(err).To(HaveOccurred()) + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(ConsistOf(*dep)) }) }) }) diff --git a/pkg/internal/field/selector/utils.go b/pkg/internal/field/selector/utils.go index 4f6d084318..2140649bea 100644 --- a/pkg/internal/field/selector/utils.go +++ b/pkg/internal/field/selector/utils.go @@ -22,14 +22,18 @@ import ( ) // RequiresExactMatch checks if the given field selector is of the form `k=v` or `k==v`. -func RequiresExactMatch(sel fields.Selector) (field, val string, required bool) { +func RequiresExactMatch(sel fields.Selector) (requires map[string]string, required bool) { reqs := sel.Requirements() - if len(reqs) != 1 { - return "", "", false + if len(reqs) == 0 { + return nil, false } - req := reqs[0] - if req.Operator != selection.Equals && req.Operator != selection.DoubleEquals { - return "", "", false + + requires = make(map[string]string, len(reqs)) + for _, req := range reqs { + if req.Operator != selection.Equals && req.Operator != selection.DoubleEquals { + return nil, false + } + requires[req.Field] = req.Value } - return req.Field, req.Value, true + return requires, true } diff --git a/pkg/internal/field/selector/utils_test.go b/pkg/internal/field/selector/utils_test.go index fba214ff16..e9ae60e9dc 100644 --- a/pkg/internal/field/selector/utils_test.go +++ b/pkg/internal/field/selector/utils_test.go @@ -27,62 +27,57 @@ import ( var _ = Describe("RequiresExactMatch function", func() { It("Returns false when the selector matches everything", func() { - _, _, requiresExactMatch := RequiresExactMatch(fields.Everything()) + _, requiresExactMatch := RequiresExactMatch(fields.Everything()) Expect(requiresExactMatch).To(BeFalse()) }) It("Returns false when the selector matches nothing", func() { - _, _, requiresExactMatch := RequiresExactMatch(fields.Nothing()) + _, requiresExactMatch := RequiresExactMatch(fields.Nothing()) Expect(requiresExactMatch).To(BeFalse()) }) It("Returns false when the selector has the form key!=val", func() { - _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val")) + _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val")) Expect(requiresExactMatch).To(BeFalse()) }) - It("Returns false when the selector has the form key1==val1,key2==val2", func() { - _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key1==val1,key2==val2")) - Expect(requiresExactMatch).To(BeFalse()) + It("Returns true when the selector has the form key1==val1,key2==val2", func() { + _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key1==val1,key2==val2")) + Expect(requiresExactMatch).To(BeTrue()) }) It("Returns true when the selector has the form key==val", func() { - _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key==val")) + _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key==val")) Expect(requiresExactMatch).To(BeTrue()) }) It("Returns true when the selector has the form key=val", func() { - _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key=val")) + _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key=val")) Expect(requiresExactMatch).To(BeTrue()) }) It("Returns empty key and value when the selector matches everything", func() { - key, val, _ := RequiresExactMatch(fields.Everything()) - Expect(key).To(Equal("")) - Expect(val).To(Equal("")) + requires, _ := RequiresExactMatch(fields.Everything()) + Expect(requires).To(BeNil()) }) It("Returns empty key and value when the selector matches nothing", func() { - key, val, _ := RequiresExactMatch(fields.Nothing()) - Expect(key).To(Equal("")) - Expect(val).To(Equal("")) + requires, _ := RequiresExactMatch(fields.Nothing()) + Expect(requires).To(BeNil()) }) It("Returns empty key and value when the selector has the form key!=val", func() { - key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val")) - Expect(key).To(Equal("")) - Expect(val).To(Equal("")) + requires, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val")) + Expect(requires).To(BeNil()) }) It("Returns key and value when the selector has the form key==val", func() { - key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key==val")) - Expect(key).To(Equal("key")) - Expect(val).To(Equal("val")) + requires, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key==val")) + Expect(requires).To(HaveKeyWithValue("key", "val")) }) It("Returns key and value when the selector has the form key=val", func() { - key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key=val")) - Expect(key).To(Equal("key")) - Expect(val).To(Equal("val")) + requires, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key=val")) + Expect(requires).To(HaveKeyWithValue("key", "val")) }) })