[cert-manager] Set resources requests/limits on cert-manager pods - #2616
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the cert-manager configuration to include explicit resource requests and limits for its various workloads. By defining these values, the deployment now adheres to cluster conformance standards, preventing warnings related to unbounded resource usage while ensuring each component is appropriately sized for its specific role. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
github-actions
Bot
added
the
area/uncategorized
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new Helm values configuration file for ChangesCert-Manager Resource Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
github-actions
Bot
added
the
size/M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces resource requests and limits for the cert-manager, webhook, cainjector, and startupapicheck components. The reviewer noted that the commit messages and PR title must follow the Conventional Commits format and include a Signed-off-by trailer as required by the style guide. Furthermore, it is suggested to increase the memory request for the startupapicheck component from 16Mi to 32Mi to prevent potential OOM issues and align with upstream standards.
| @@ -0,0 +1,32 @@ | |||
| cert-manager: | |||
There was a problem hiding this comment.
The pull request title and commit messages do not follow the Conventional Commits format required by the repository style guide. It should follow the pattern type(scope): description (e.g., feat(cert-manager): set resource requests and limits). Additionally, ensure that each commit includes a Signed-off-by: trailer.
References
- Commits must follow the Conventional Commits format (type(scope): description) and include a Signed-off-by trailer. (link)
| memory: 64Mi | ||
| requests: | ||
| cpu: 10m | ||
| memory: 16Mi |
There was a problem hiding this comment.
The memory request for startupapicheck is set to 16Mi, which is lower than the 32Mi suggested in the upstream cert-manager examples and used for other helper components like the webhook and cainjector in this PR. Increasing this to 32Mi provides a safer margin for the Go runtime and avoids potential OOM kills during the initial API checks.
memory: 32MiReferences
- Ensure appropriate resource requests and limits are set for all workloads to maintain stability and avoid conformance warnings.
Aleksei Sviridkin (lexfrei)
added
kind/cleanup
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
The four value paths map correctly onto the vendored cert-manager subchart — controller at top-level resources, plus webhook.resources, cainjector.resources, and startupapicheck.resources (which is enabled by default, so the block takes effect). Sizing is sane for each role and the requests stay modest. Nit: the title isn't in Conventional Commits form; and the earlier bot note about a missing Signed-off-by is stale — the head commit is signed off.
Non-blocking: startupapicheck memory request 16Mi is fine for a one-shot check job, but 32Mi would give more margin if the API bundle grows.
The upstream cert-manager chart ships with resources: {} for all four
workloads (controller, webhook, cainjector, startupapicheck), which
triggers 'no CPU limit' conformance warnings on every pod. Override the
package values.yaml to set sane requests and limits sized to each
component's role: controller is the most loaded, webhook and cainjector
handle short-lived admission/injection work, and startupapicheck is a
one-shot Job.
Signed-off-by: Matthieu <[email protected]>
afab347 to
cd95040
Compare
github-actions
Bot
added
the
area/uncategorized
1aa6436
into
cozystack:main
2 participants
Summary
The upstream cert-manager chart ships with
resources: {}for all four workloads, which triggersno CPU limit(andno memory limit) conformance warnings on every pod.Override
packages/system/cert-manager/values.yaml(previously empty) to set sane requests/limits sized to each component's role:Requests for
cainjector(10mcpu /32Mimem) match the example in the upstream values.yaml comment.Test plan
helm template . --namespace cert-managerfrompackages/system/cert-manager/renders aresourcesblock on all four pods (verified locally for controller, webhook, cainjector and the startupapicheck Job)cert-manager-cainjector— should be clean on the other three pods tooRelease note
```release-note
Set CPU/memory requests and limits on cert-manager controller, webhook, cainjector and startupapicheck pods to clear "no CPU limit" conformance warnings.
```
Summary by CodeRabbit