Skip to content
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

feat: add pvc creation support #67

Merged
merged 9 commits into from
Aug 2, 2024
Merged

Conversation

mt-czi
Copy link
Contributor

@mt-czi mt-czi commented Aug 1, 2024

Add the option for users to create pvc claims

Co-authored-by: Hayden Spitzley <105455169+hspitzley-czi@users.noreply.github.com>
@mt-czi mt-czi requested a review from hspitzley-czi August 1, 2024 18:05
@mt-czi mt-czi requested a review from hspitzley-czi August 1, 2024 18:24
Comment on lines 21 to 23
{{- range .Values.persistence.accessModes }}
- {{ . | quote }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be simplified to use toYaml?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, maybe just allow the user to specify a spec and toYaml it. That way we don't have to keep adding prs to support other features and the configuration looks more like natural k8s configuration

mt-czi and others added 2 commits August 1, 2024 16:00
Co-authored-by: Hayden Spitzley <105455169+hspitzley-czi@users.noreply.github.com>
Co-authored-by: Hayden Spitzley <105455169+hspitzley-czi@users.noreply.github.com>
Copy link
Contributor

@jakeyheath jakeyheath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! thanks for your contribution!

small change to open up the PVC to allow all settings; also we don't have great testing and linting yet, so can you please do due diligence to make sure the helm template renders correctly and linted before merging.

@mt-czi
Copy link
Contributor Author

mt-czi commented Aug 2, 2024

@jakeyheath @hspitzley-czi Thank you for the feedback. I opened all the pvc settings and linted the chart

@mt-czi mt-czi merged commit 2269fbd into main Aug 2, 2024
2 checks passed
@mt-czi mt-czi deleted the GPU-271/pvc-creation-support branch August 2, 2024 16:12
@czi-github-helper czi-github-helper bot mentioned this pull request Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants