-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
🐛 pkg/client: optionally allow caching of unstructured objects #1332
🐛 pkg/client: optionally allow caching of unstructured objects #1332
Conversation
/retest |
c3ee29b
to
eb0f1d4
Compare
/hold For discussion. |
Looks like #1058 removed the exported |
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.
Agreed that this is a regression.
/lgtm
I'm +1 having the option to cache unstructured objects as well. IIRC, the earlier version of the DelegatingClient was meant to be used to always go to a live client for unstructured objects https://github.com/kubernetes-sigs/controller-runtime/blob/release-0.5/pkg/client/split.go#L45-L61, we kept that behavior. That said, as you mentioned the /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: estroz, joelanford, vincepri 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:
Approvers can indicate their approval by writing |
Might also be good to set this up as a manager option too? |
/hold cancel I'll add a manager option in a follow-up. |
/cherry-pick release-0.7 |
Based on #615, it seems like there is intent to allow the DelegatingClient to cache unstructured objects. This is behavior that was possible to use before v0.7.0 by using the
NewClientFunc
manager option and creating a custom DelegatingClient.IMO, this could be considered a regression, but since a large chunk of this code was restructured in v0.7.0, I'm not sure if dropping support for unstructured caching was intentional.
This PR adds a new boolean field to the NewDelegatingClientInput struct that allows users to opt-in to caching unstructured objects.