-
Notifications
You must be signed in to change notification settings - Fork 579
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
🌱 Update to controller runtime 0.5.11 #1950
Conversation
Signed-off-by: Naadir Jeewa <jeewan@vmware.com>
/assign @vincepri |
FWIW, the memory leak we fixed in controller-runtime only really occurs if you're dynamically adding a watch to a controller, and then you close/stop the cache (shared informers) associated with the that watch (e.g. when watching something from a workload cluster, not from the Manager's client/cache). It's fine to upgrade here, but it's not strictly required for CAPA from a memory leak perspective, I don't think. |
+1 to what Andy said, although we can merge this now if we want to ship CAPA with the fix earlier than CAPI /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
should we be doing this in CAPZ as well? |
Won't hurt. |
/lgtm |
Signed-off-by: Naadir Jeewa jeewan@vmware.com
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #