search

kubernetes-sigs / cli-utils

This repo contains binaries that built from libraries in cli-runtime.
Apache License 2.0
155 stars 77 forks source link

Add status watch label filter #639

Closed karlkfi closed 3 months ago

karlkfi commented 3 months ago

Using a label filter significantly cuts down on watch events and memory used by the informer's watch cache. But you'll need to set the labels on the objects yourself before providing them to the applier.

I've tested it with Config Sync end-to-end tests and shown that it significantly reduces memory usage when there are other objects on the cluster with resource types and namespaces that overlap with the apply set being watched. This is because the informer caches objects it receives events for, even if they're not in the apply set.

Unfortunately, FakeDynamicClient (client-go) only supports list label selector, not watch label selectors. So the unit tests added here aren't particularly useful. We will need to upstream changes to client-go to make the unit tests more functional. But that shouldn't be a blocker.

I added field selectors as well, but these are even harder to test than label selectors, because the FakeDynamicClient doesn't support them at all. It's also not super clear how indexers relate to label filters, since the indexer is only used client-side and the label filter is server-side (i think).

k8s-ci-robot commented 3 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/cli-utils/blob/master/OWNERS)~~ [karlkfi] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
karlkfi commented 3 months ago

I've started a rather large upstream refactor to make field selection reproducible in the upstream fake clients: https://github.com/kubernetes/kubernetes/pull/124801

However, even if that merges, we won't be able to update the k8s version here until it's published and has a chance to get into at least the rapid and maybe the standard channels of GKE. So I'd like to merge this PR without ideal coverage of the field selectors. This should be good enough, since we're only really planning to use the label selectors in Config Sync. Then I can circle back later and update the tests to better validate the field selectors after the upstream PR merges.

mortent commented 3 months ago

/lgtm