Skip to content

fix(service): switch Endpoints() to indexer-based filtering#6344

Open
ivankatliarchuk wants to merge 7 commits intokubernetes-sigs:masterfrom
gofogo:feat/service-lister-switch-to-indexer
Open

fix(service): switch Endpoints() to indexer-based filtering#6344
ivankatliarchuk wants to merge 7 commits intokubernetes-sigs:masterfrom
gofogo:feat/service-lister-switch-to-indexer

Conversation

@ivankatliarchuk
Copy link
Copy Markdown
Member

@ivankatliarchuk ivankatliarchuk commented Apr 3, 2026

What does it do ?

  • service.go: push annotation filter, label selector, service type, and controller-mismatch check into the indexer at construction time; switch Endpoints() from Lister().List() to GetIndexer().ListIndexFuncValues; remove filterByServiceType, annotationFilter, labelSelector from serviceSource; add predicate method on serviceTypes
  • informers/indexers.go: add IndexSelectorWithConditions[T] - typed predicate support in IndexerWithOptions, so sources can push arbitrary func(T) bool logic into the index
  • annotations/processors.go: add IsControllerMatch[T] - inverse of IsControllerMismatch, usable directly as a predicate

Motivation

Moving all filtering into the indexer means only matching services are ever stored or iterated.

The key benefit is filtering happens once at write time to the Informer cache, not on every read.

With the old Lister().List() approach:

  • Every Endpoints() call fetches all services, then walks the full list applying annotation filter, label selector, service type check, and controller mismatch check in sequence
  • O(n) work on every reconciliation loop, even when nothing changed

With the indexer:

  • Filtering runs when a service is added/updated in the cache - services that don't match are simply never added to the withSelectors index
  • Endpoints() only iterates the already-filtered set - no per-call filtering work
  • The larger the cluster (more services), the bigger the win: most services are irrelevant noise that the old approach re-scanned every loop

Secondary benefits:

  • Less memory pressure - unmatched services do not exist in the index cache, so ListIndexFuncValues returns a smaller working set
  • Correctness - filters are consistently applied at a single point rather than scattered across Endpoints() with different code paths (the old annotations.Filter error path is gone, controller mismatch check is gone from the hot path)
  • Extensibility - IndexSelectorWithConditions lets any source push typed, source-specific predicates into the indexer without touching IndexSelectorOptions

Previous releted PRs

More

  • Yes, this PR title follows Conventional Commits
  • Yes, I added unit tests
  • Yes, I updated end user documentation accordingly
┌──────────┬───────┐
│ Category │ Lines │
├──────────┼───────┤
│ Non-test │ +63   │
├──────────┼───────┤
│ Test     │ +326  │
├──────────┼───────┤
│ Docs     │ +0    │
├──────────┼───────┤
│ Ratio    │ 1:5.1 │
└──────────┴───────┘

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mloiseleur for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 3, 2026
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 3, 2026

Pull Request Test Coverage Report for Build 23954439297

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 35 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.007%) to 80.521%

Files with Coverage Reduction New Missed Lines %
openshift_route.go 1 82.93%
annotations/processors.go 2 97.37%
informers/indexers.go 7 90.28%
service.go 25 95.12%
Totals Coverage Status
Change from base Build 23940145743: -0.007%
Covered Lines: 17151
Relevant Lines: 21300

💛 - Coveralls

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
@ivankatliarchuk
Copy link
Copy Markdown
Member Author

/test pull-external-dns-unit-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants