-
Notifications
You must be signed in to change notification settings - Fork 3
add support for ephemeral volumes #133
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
Conversation
WalkthroughAdds a new core.v1.volume helper for ephemeral volumes and extends deployment/statefulSet volume merging to include ephemeral volumes when provided. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kubernetes/k.libsonnet(1 hunks)kubernetes/kubernetes.libsonnet(2 hunks)
🔇 Additional comments (2)
kubernetes/kubernetes.libsonnet (2)
245-248: Ephemeral volume wiring in deployments looks solidThe new branch cleanly mirrors the existing patterns and keeps the mixin dedupe logic intact. 👍
369-372: Nice parity for StatefulSetsThanks for keeping the StatefulSet path in sync with the deployment helper—makes downstream configs consistent.
| super.ephemeral.volumeClaimTemplate.spec.resources.withRequests(ephemeral.resources.requests) + | ||
| super.ephemeral.volumeClaimTemplate.spec.withAccessModes(if std.objectHas(ephemeral, 'accessModes') then ephemeral.accessModes else ['ReadWriteOnce']) + | ||
| super.ephemeral.volumeClaimTemplate.spec.withStorageClassName(if std.objectHas(ephemeral, 'storageClassName') then ephemeral.storageClassName else 'gp3'), |
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.
Guard ephemeral.resources.requests before dereferencing
ephemeral defaults to {}, so core.v1.volume.fromEphemeral('cache') (or any call that only overrides access modes / storage class) will throw Field does not exist: resources at evaluation time. Add a guard—or emit a clear assertion error—before touching ephemeral.resources.requests, so the helper is usable with the documented default.
Apply this diff:
- super.ephemeral.volumeClaimTemplate.spec.resources.withRequests(ephemeral.resources.requests) +
+ (
+ if std.objectHas(ephemeral, 'resources') && std.objectHas(ephemeral.resources, 'requests')
+ then super.ephemeral.volumeClaimTemplate.spec.resources.withRequests(ephemeral.resources.requests)
+ else error 'core.v1.volume.fromEphemeral requires ephemeral.resources.requests'
+ ) +📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| super.ephemeral.volumeClaimTemplate.spec.resources.withRequests(ephemeral.resources.requests) + | |
| super.ephemeral.volumeClaimTemplate.spec.withAccessModes(if std.objectHas(ephemeral, 'accessModes') then ephemeral.accessModes else ['ReadWriteOnce']) + | |
| super.ephemeral.volumeClaimTemplate.spec.withStorageClassName(if std.objectHas(ephemeral, 'storageClassName') then ephemeral.storageClassName else 'gp3'), | |
| ( | |
| if std.objectHas(ephemeral, 'resources') && std.objectHas(ephemeral.resources, 'requests') | |
| then super.ephemeral.volumeClaimTemplate.spec.resources.withRequests(ephemeral.resources.requests) | |
| else error 'core.v1.volume.fromEphemeral requires ephemeral.resources.requests' | |
| ) + | |
| super.ephemeral.volumeClaimTemplate.spec.withAccessModes( | |
| if std.objectHas(ephemeral, 'accessModes') | |
| then ephemeral.accessModes | |
| else ['ReadWriteOnce'] | |
| ) + | |
| super.ephemeral.volumeClaimTemplate.spec.withStorageClassName( | |
| if std.objectHas(ephemeral, 'storageClassName') | |
| then ephemeral.storageClassName | |
| else 'gp3' | |
| ), |
🤖 Prompt for AI Agents
In kubernetes/k.libsonnet around lines 7 to 9, the code dereferences
ephemeral.resources.requests although ephemeral may be {} causing a "Field does
not exist: resources" error; guard that access by checking for the presence of
resources (and requests) before using it—e.g. replace the direct
ephemeral.resources.requests with a conditional that uses
ephemeral.resources.requests only if std.objectHas(ephemeral, "resources") and
std.objectHas(ephemeral.resources, "requests"), otherwise supply an empty
requests object (or assert with a clear error message); keep the surrounding
.withRequests call unchanged but pass the guarded value.
No description provided.