Repository navigation
[bootbox] Add resources requests/limits to matchbox container - #2625
Aleksei Sviridkin (lexfrei) merged 1 commit into
Conversation
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 introduces explicit resource requests and limits to the matchbox container within the bootbox deployment. By defining these constraints, the change ensures the service adheres to cluster resource management standards and eliminates existing conformance warnings. 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
|
|
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)
📝 WalkthroughWalkthroughThis PR adds Kubernetes resource constraints to the matchbox container within the bootbox-matchbox Deployment. The container now declares CPU and memory limits as well as resource requests, ensuring predictable scheduling and preventing resource exhaustion on the cluster. ChangesMatchbox Deployment Resource Constraints
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 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 |
There was a problem hiding this comment.
Code Review
This pull request adds resource requests and limits to the matchbox deployment in the bootbox package. Feedback indicates that a securityContext should be added to the container for better security, and the commit metadata needs to be updated to follow the Conventional Commits format and include a sign-off trailer as per the repository guidelines.
| - name: http | ||
| containerPort: 8080 | ||
| protocol: TCP | ||
| resources: |
There was a problem hiding this comment.
The container is missing a securityContext. According to the repository guidelines (Line 82), containers should have a securityContext defined and should not run as root without a clear rationale. Adding a restricted security context improves the security posture of the workload.
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
runAsNonRoot: true
seccompProfile:
type: RuntimeDefault
resources:References
- Flag missing securityContext and containers running as root without an explicit reason. (link)
| - name: http | ||
| containerPort: 8080 | ||
| protocol: TCP | ||
| resources: |
There was a problem hiding this comment.
The commit message and PR title do not follow the Conventional Commits format required by the repository guidelines (Line 36). It should follow the type(scope): description format (e.g., feat(bootbox): add resource requests/limits to matchbox). Additionally, ensure each commit includes a Signed-off-by: trailer.
References
- Each commit must follow Conventional Commits format and have a Signed-off-by: trailer. (link)
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Straightforward and correct. The resources block is indented as a sibling of ports on the matchbox container, and 200m/256Mi limits over 50m/128Mi requests fit a lightweight HTTP server that just serves iPXE profiles and boot assets from ConfigMaps. This is a template edit, not a values change, so the chart's values.schema.json is unaffected and no regeneration is needed. Sign-off present.
The matchbox Deployment in the bootbox package had no resources block, triggering a 'no CPU limit' conformance warning. Set limits 200m/256Mi and requests 50m/128Mi — modest sizing since matchbox is a lightweight HTTP server serving iPXE profiles and boot assets from ConfigMaps. Bootbox is a standalone chart (no vendored upstream), so the values are set directly in the template, matching the existing pattern in csi-resizer and s3manager. Signed-off-by: Matthieu <[email protected]>
486742d to
0ea94e7
Compare
24e3bf9
into
cozystack:main
Summary
The
matchboxcontainer inpackages/extra/bootbox/templates/matchbox/deployment.yamlhad noresourcesblock, triggering ano CPU limitconformance warning on thebootbox-matchboxDeployment (PXE/iPXE provisioning service).Bootbox is a standalone chart (no vendored upstream), so the values are set directly in the template — same pattern as #2614 (csi-resizer) and #2624 (s3manager).
limits:cpu: 200m,memory: 256Mirequests:cpu: 50m,memory: 128MiModest sizing since matchbox is a lightweight HTTP server serving iPXE profiles and boot assets from ConfigMaps.
Test plan
helm template bootbox . --namespace tenant-root --show-only templates/matchbox/deployment.yaml --set _namespace.host=example.com --set _cluster.solver=letsencryptfrompackages/extra/bootbox/renders theresourcesblock on thematchboxcontainer (verified locally)matchbox has no CPU limitwarning should be goneRelease note
```release-note
Add CPU and memory requests/limits to the matchbox container of the bootbox Deployment to clear the "no CPU limit" conformance warning.
```
Summary by CodeRabbit