-
Couldn't load subscription status.
- Fork 5
Edit Helm Chart #101
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
base: dev
Are you sure you want to change the base?
Edit Helm Chart #101
Conversation
- Add resources - Add a key to change the storage mode - if RWO, the prowes pods will be deployed on the same nodes
Reviewer's GuideThis PR enhances the Helm chart by introducing a configurable storageAccessMode for dynamic volume access modes, centralizing resource requests and limits in values.yaml and propagating them via templated toYaml blocks, and adding conditional pod affinity to co-locate prowes pods under ReadWriteOnce volumes. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `deployment/templates/prowes/prowes-deployment.yaml:32` </location>
<code_context>
image: busybox
command: [ 'mkdir' ]
args: [ '-p', '/data/db', '/data/specs' ]
+ resources: {{- toYaml .Values.prowes.resources | nindent 10 }}
volumeMounts:
- mountPath: /data
</code_context>
<issue_to_address>
**suggestion:** Resource specification for initContainers may be excessive.
Consider lowering resource requests/limits for initContainers if their workload is minimal to avoid unnecessary over-provisioning.
Suggested implementation:
```
resources:
limits:
cpu: 10m
memory: 16Mi
requests:
cpu: 1m
memory: 4Mi
```
If you want to make the resource values configurable via Helm, you should add a new key (e.g., `initResources`) to your `values.yaml` and reference it here instead of hardcoding. For example:
```
resources: {{- toYaml .Values.prowes.initResources | nindent 10 }}
```
And in your `values.yaml`:
```
prowes:
initResources:
limits:
cpu: 10m
memory: 16Mi
requests:
cpu: 1m
memory: 4Mi
```
</issue_to_address>
### Comment 2
<location> `deployment/templates/prowes/celery-deployment.yaml:19-22` </location>
<code_context>
periodSeconds: 10
successThreshold: 1
timeoutSeconds: 1
- resources:
- limits:
- memory: 512Mi
</code_context>
<issue_to_address>
**suggestion (performance):** Celery initContainer resource limits may be unnecessarily high.
The current allocation of 256Mi memory and 100m CPU for the celery initContainer may exceed the needs for basic directory creation. Reducing these limits could improve resource efficiency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
IMPORTANT: Please create an issue before filing a pull request! Changes need to be discussed before proceeding. Please read the contribution guidelines.
Details
Please provide enough information so that others can review your pull request. Give a brief summary of the motivation. Refer to the corresponding issue/s with
#XXXXfor more information.Testing
Write the appropriate unit and integration tests, if applicable. Make sure these and all other tests pass.
Documentation
Please document your changes and test cases in the appropriate places, if applicable.
Style
Make sure your changes adhere to the coding/documentation style used throughout the project.
Closing issues
If your changes fix any issue/s, put
closes #XXXXin your comment to auto-close it/them.Credit
Add your credentials to the list of contributors once your pull request was merged.
Summary by Sourcery
Add storageAccessMode option for PVCs, configure resource requests and limits across all services, and implement pod affinity for ReadWriteOnce deployments
New Features:
Enhancements: