diff --git a/internal/ingress/controller/store/endpointslice.go b/internal/ingress/controller/store/endpointslice.go index 61bc63ae7b..8a63a14432 100644 --- a/internal/ingress/controller/store/endpointslice.go +++ b/internal/ingress/controller/store/endpointslice.go @@ -18,51 +18,72 @@ package store import ( "fmt" - "strings" discoveryv1 "k8s.io/api/discovery/v1" - apiNames "k8s.io/apiserver/pkg/storage/names" "k8s.io/client-go/tools/cache" ) +type getEppsForServiceFunc = func(key string) ([]*discoveryv1.EndpointSlice, error) + // EndpointSliceLister makes a Store that lists Endpoints. type EndpointSliceLister struct { cache.Store + endpointSliceIndex getEppsForServiceFunc } // MatchByKey returns the EndpointsSlices of the Service matching key in the local Endpoint Store. func (s *EndpointSliceLister) MatchByKey(key string) ([]*discoveryv1.EndpointSlice, error) { - var eps []*discoveryv1.EndpointSlice - keyNsLen := strings.Index(key, "/") - if keyNsLen < -1 { - keyNsLen = 0 - } else { - // count '/' char - keyNsLen++ + epss, err := s.endpointSliceIndex(key) + if err != nil { + return nil, err } - // filter endpointSlices owned by svc - for _, listKey := range s.ListKeys() { - if len(key) < (apiNames.MaxGeneratedNameLength+keyNsLen) && !strings.HasPrefix(listKey, key) { - continue - } - // generated endpointslices names has truncated svc name as prefix when svc name is too long, we compare only non truncated part - // https://github.com/kubernetes/ingress-nginx/issues/9240 - if len(key) >= (apiNames.MaxGeneratedNameLength+keyNsLen) && !strings.HasPrefix(listKey, key[:apiNames.MaxGeneratedNameLength+keyNsLen-1]) { - continue + + if len(epss) == 0 { + return nil, NotExistsError(key) + } + return epss, nil +} + +func epssIndexer() cache.Indexers { + return cache.Indexers{ + discoveryv1.LabelServiceName: func(obj interface{}) ([]string, error) { + eps, ok := obj.(*discoveryv1.EndpointSlice) + if !ok { + // Skip object as it is not an endpointslice + return nil, nil + } + + parentService, ok := eps.Labels[discoveryv1.LabelServiceName] + if !ok { + // There is no parent service and thus we cannot match this endpointslice to any service + // As far as i'm aware, this is only possible if you create epps objects by hand + return nil, nil + } + + key := fmt.Sprintf("%s/%s", eps.Namespace, parentService) + + return []string{key}, nil + }, + } +} + +func eppsForServiceFuncFromIndexer(indexer cache.Indexer) getEppsForServiceFunc { + return func(key string) ([]*discoveryv1.EndpointSlice, error) { + objs, err := indexer.ByIndex(discoveryv1.LabelServiceName, key) + if err != nil { + return nil, err } - epss, exists, err := s.GetByKey(listKey) - if exists && err == nil { - // check for svc owner label - if svcName, ok := epss.(*discoveryv1.EndpointSlice).ObjectMeta.GetLabels()[discoveryv1.LabelServiceName]; ok { - namespace := epss.(*discoveryv1.EndpointSlice).ObjectMeta.GetNamespace() - if key == fmt.Sprintf("%s/%s", namespace, svcName) { - eps = append(eps, epss.(*discoveryv1.EndpointSlice)) - } + + epss := make([]*discoveryv1.EndpointSlice, 0, len(objs)) + for _, obj := range objs { + eps, ok := obj.(*discoveryv1.EndpointSlice) + if !ok { + continue } + + epss = append(epss, eps) } + + return epss, nil } - if len(eps) == 0 { - return nil, NotExistsError(key) - } - return eps, nil } diff --git a/internal/ingress/controller/store/endpointslice_test.go b/internal/ingress/controller/store/endpointslice_test.go index 5d423a3a6f..d1ce0612f1 100644 --- a/internal/ingress/controller/store/endpointslice_test.go +++ b/internal/ingress/controller/store/endpointslice_test.go @@ -25,15 +25,21 @@ import ( "k8s.io/client-go/tools/cache" ) -func newEndpointSliceLister(t *testing.T) *EndpointSliceLister { +func newEndpointSliceLister(t *testing.T) (*EndpointSliceLister, cache.Indexer) { t.Helper() - return &EndpointSliceLister{Store: cache.NewStore(cache.MetaNamespaceKeyFunc)} + store := cache.NewStore(cache.MetaNamespaceKeyFunc) + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, epssIndexer()) + + return &EndpointSliceLister{ + Store: store, + endpointSliceIndex: eppsForServiceFuncFromIndexer(indexer), + }, indexer } func TestEndpointSliceLister(t *testing.T) { t.Run("the key does not exist", func(t *testing.T) { - el := newEndpointSliceLister(t) + el, _ := newEndpointSliceLister(t) key := "namespace/svcname" _, err := el.MatchByKey(key) @@ -47,58 +53,60 @@ func TestEndpointSliceLister(t *testing.T) { } }) t.Run("the key exists", func(t *testing.T) { - el := newEndpointSliceLister(t) + el, indexer := newEndpointSliceLister(t) key := "namespace/svcname" - endpointSlice := &discoveryv1.EndpointSlice{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "namespace", - Name: "anothername-foo", - Labels: map[string]string{ - discoveryv1.LabelServiceName: "svcname", + epss := []*discoveryv1.EndpointSlice{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace", + Name: "anothername-foo", + Labels: map[string]string{ + discoveryv1.LabelServiceName: "svcname", + }, }, }, - } - if err := el.Add(endpointSlice); err != nil { - t.Errorf("unexpected error %v", err) - } - endpointSlice = &discoveryv1.EndpointSlice{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "namespace", - Name: "svcname-bar", - Labels: map[string]string{ - discoveryv1.LabelServiceName: "othersvc", + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace", + Name: "svcname-bar", + Labels: map[string]string{ + discoveryv1.LabelServiceName: "othersvc", + }, }, }, - } - if err := el.Add(endpointSlice); err != nil { - t.Errorf("unexpected error %v", err) - } - endpointSlice = &discoveryv1.EndpointSlice{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "namespace", - Name: "svcname-buz", - Labels: map[string]string{ - discoveryv1.LabelServiceName: "svcname", + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace", + Name: "svcname-buz", + Labels: map[string]string{ + discoveryv1.LabelServiceName: "svcname2", + }, }, }, } - if err := el.Add(endpointSlice); err != nil { - t.Errorf("unexpected error %v", err) + for _, eps := range epss { + if err := el.Add(eps); err != nil { + t.Errorf("unexpected error %v", err) + } + if err := indexer.Add(eps); err != nil { + t.Errorf("unexpected error %v", err) + } } + eps, err := el.MatchByKey(key) if err != nil { t.Errorf("unexpeted error %v", err) } if err == nil && len(eps) != 1 { - t.Errorf("expected one slice %v, error, got %d slices", endpointSlice, len(eps)) + t.Errorf("expected one slice %v, error, got %d slices, keys stored in indexer: %v, eps returned by storer: %v", epss[0], len(eps), indexer.ListKeys(), eps) } - if len(eps) > 0 && eps[0].GetName() != endpointSlice.GetName() { - t.Errorf("expected %v, error, got %v", endpointSlice.GetName(), eps[0].GetName()) + if len(eps) > 0 && eps[0].GetName() != epss[0].GetName() { + t.Errorf("expected %v, error, got %v", epss[0].GetName(), eps[0].GetName()) } }) t.Run("svc long name", func(t *testing.T) { - el := newEndpointSliceLister(t) + el, indexer := newEndpointSliceLister(t) ns := "namespace" ns2 := "another-ns" svcName := "test-backend-http-test-http-test-http-test-http-test-http-truncated" @@ -116,6 +124,9 @@ func TestEndpointSliceLister(t *testing.T) { if err := el.Add(endpointSlice); err != nil { t.Errorf("unexpected error %v", err) } + if err := indexer.Add(endpointSlice); err != nil { + t.Errorf("unexpected error %v", err) + } endpointSlice2 := &discoveryv1.EndpointSlice{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns2, @@ -128,9 +139,12 @@ func TestEndpointSliceLister(t *testing.T) { if err := el.Add(endpointSlice2); err != nil { t.Errorf("unexpected error %v", err) } + if err := indexer.Add(endpointSlice); err != nil { + t.Errorf("unexpected error %v", err) + } eps, err := el.MatchByKey(key) if err != nil { - t.Errorf("unexpeted error %v", err) + t.Errorf("unexpected error %v", err) } if len(eps) != 1 { t.Errorf("expected one slice %v, error, got %d slices", endpointSlice, len(eps)) diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 284f532090..775739a900 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -338,7 +338,15 @@ func New( } store.informers.EndpointSlice = infFactory.Discovery().V1().EndpointSlices().Informer() + // Add new endpointslices indexer to markup epps upfront for fast pinpoint retrieval + if err := store.informers.EndpointSlice.AddIndexers(epssIndexer()); err != nil { + // This error only occurs due to errors in code, this panic is not possible in runtime + // if the underlying code is correct. Typically, this error signals conflicts in indexer + panic(fmt.Sprintf("failed to add new index for endpointslices: %v", err)) + } + store.listers.EndpointSlice.Store = store.informers.EndpointSlice.GetStore() + store.listers.EndpointSlice.endpointSliceIndex = eppsForServiceFuncFromIndexer(store.informers.EndpointSlice.GetIndexer()) store.informers.Secret = infFactorySecrets.Core().V1().Secrets().Informer() store.listers.Secret.Store = store.informers.Secret.GetStore()