Enable configurable storageClass and size for H2 PVC#18
Conversation
Co-authored-by: kthoms <265597+kthoms@users.noreply.github.com>
| # If empty, no storageClassName is set (cluster default is used). | ||
| storageClass: "" | ||
| # Size of the H2 database PVC. | ||
| size: 1Gi |
There was a problem hiding this comment.
@copilot Although it was the previous default, isn't 1Gi too much for the default? Especially when using h2?
There was a problem hiding this comment.
Reduced the default to 256Mi in ad622b7, which is more appropriate for H2 in demo/test scenarios.
| extraContainers: [] | ||
|
|
||
| persistence: | ||
| # Storage class for the H2 database PVC. |
There was a problem hiding this comment.
@copilot Aren't the persistence settings also valid for other DB kinds than h2? Then the comment should be db agnostic.
There was a problem hiding this comment.
Updated in ad622b7 — comments now say "Storage class for the PVC" and "Size of the PVC" without referencing H2 specifically.
Co-authored-by: kthoms <265597+kthoms@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Helm chart configurability for the H2 PersistentVolumeClaim (PVC) by introducing new persistence.storageClass and persistence.size values, wiring them into the PVC template, and validating them via the chart’s JSON schema.
Changes:
- Added
persistence.storageClassandpersistence.sizetovalues.yaml(defaulting size to256Mi). - Updated the H2 PVC template to optionally set
storageClassName(including"-"→""behavior) and to source storage size from values. - Extended
values.schema.jsonto include the newpersistenceobject and require it at the chart root.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/operaton/values.yaml | Introduces persistence values and sets a smaller default size. |
| charts/operaton/values.schema.json | Adds schema definitions/requirements for persistence.storageClass and persistence.size. |
| charts/operaton/templates/persistentvolumeclaim.yaml | Uses the new persistence values when rendering the H2 PVC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- if .Values.persistence.size }} | ||
| storage: {{ .Values.persistence.size }} | ||
| {{- end }} |
There was a problem hiding this comment.
resources.requests.storage is currently rendered only when persistence.size is truthy. A PVC without a storage request is invalid and will be rejected by the Kubernetes API if a user sets persistence.size: "" (or otherwise makes it empty). Prefer always rendering storage (using default/chart default) or failing the render with required when persistence.size is empty, rather than silently omitting it.
| {{- if .Values.persistence.size }} | |
| storage: {{ .Values.persistence.size }} | |
| {{- end }} | |
| storage: {{ required "persistence.size is required and cannot be empty" .Values.persistence.size }} |
| persistence: | ||
| # Storage class for the PVC. | ||
| # If set to "-", storageClassName will be set to "", disabling dynamic provisioning. | ||
| # If empty, no storageClassName is set (cluster default is used). | ||
| storageClass: "" |
There was a problem hiding this comment.
These persistence.* values are defined at the chart root, but they only affect the H2 PersistentVolumeClaim (this PVC template is gated by operaton.h2DatabaseIsUsed). Consider clarifying that in the comments so users running an external DB don’t assume these settings have an effect.
persistence.storageClassandpersistence.sizefields tovalues.yamlpersistentvolumeclaim.yamltemplate to use the new valuesvalues.schema.jsonto include the newpersistencefieldspersistence.sizeconditional in the PVC template (consistent withstorageClass)1Gito256Mi(more appropriate for H2 demo/test usage)Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.