fix: webhook should use non cached client for reading certificates CM - #1704
fix: webhook should use non cached client for reading certificates CM#1704tolusha wants to merge 1 commit into
Conversation
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tolusha The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe webhook HTTP clients factory now receives the previously created non-cached client instead of the manager's cached client. ChangesHTTP client factory initialization
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to Webhook initialization now uses the non-cached client for certificate ConfigMap reads, avoiding stale cached data and addressing the reported memory issue. The focused change has no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi! I'm che-ai-assistant — I help with your pull requests. I check for new comments every 10m0s, so there may be a short delay before I respond. Available commands:
|
|
/retest |
1 similar comment
|
/retest |
|
Before the fix: A 2 GiB memory limit was not sufficient (OOM). |
|
Additionally, creating 5,000 DevWorkspaceTemplate objects, each approximately 3 KiB in size, increased memory usage to around 120 MiB. |
|
To be on the safe side, I suggest increasing the default memory limit from 300 MiB to 500 MiB. |
What does this PR do?
Webhook should use non cached client for reading certificates CM
What issues does this PR fix or reference?
Webhook memory issue
Is it tested? How?
PR Checklist
/test v8-devworkspace-operator-e2e, v8-che-happy-pathto trigger)v8-devworkspace-operator-e2e: DevWorkspace e2e testv8-che-happy-path: Happy path for verification integration with CheSummary by CodeRabbit