-
Notifications
You must be signed in to change notification settings - Fork 493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Specifically cached client #2940
Specifically cached client #2940
Conversation
/assign |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice PR. I don't have much to add except the nits I described in the comments.
872ae6f
to
1d84348
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Very nice PR!
How to categorize this PR?
/area cost networking
/kind enhancement
/priority normal
What this PR does / why we need it:
This PR add an implementation of the
client.{Reader,Client}
interfaces that can disable the cache or enable the cache only for a given set of GroupKinds.The problem with the cached controller-runtime client is, that one
Get
orList
call triggers the client to start a watch for the respective object and therefore caches all object of that kind. This leads to an unnecessarily increased memory footprint for controllers that start caches, although they don't need to.This implementation is useful for extension controllers that e.g. create ManagedResources and corresponding Secrets but don't want to cache all Secrets in the Seed cluster (which might be a lot).
For an example usage see gardener/gardener-extension-networking-calico#55.
This PR also enables configuring these cache settings for our clientsets that we use in g/g, if we see the necessity to do so in the future.
Which issue(s) this PR fixes:
Related to #2822 and #1953
Special notes for your reviewer:
/cc @timuthy @MartinWeindel
Release note: