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.

Sign up for GitHub

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

Jump to bottom

add support for more flexible volume mounts e.g. for use with init containers #314

Merged
merged 1 commit into from
Jul 14, 2020
Merged

add support for more flexible volume mounts e.g. for use with init containers #314

merged 1 commit into from
Jul 14, 2020

Conversation

georgekaz
Copy link
Contributor

The existing volume mounts implementation via extraVolumes is too opinionated to allow mounting volumes for custom purposes, such as installing a plugin via an init container. Personally I don't like it and would remove it completely, but for backwards compatibility I've left it and added additional server.volumes and server.volumeMounts. Examples in the values files show how this can be used.

Please consider this as it's an easy way to customise your Vault installation.

jasonodonnell
jasonodonnell suggested changes May 27, 2020
View reviewed changes
Copy link
Contributor

@jasonodonnell jasonodonnell left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks useful but this will need unit tests. You can find them in test/unit. Let me know if you have any questions about implementing them!

@georgekaz
Copy link
Contributor Author

Hi Jason. I'll try, I've not used these before so might have questions. I've tested the output is as expected using helm template and helm lint and am successfully running the output in a cluster at the moment. Happy to learn the unit tests though, will be useful for me.

@allan-degnan-rft
Copy link

If this gets merged you can kill the need for #303 and #302.

@georgekaz
add support for more flexible volume mounts. Include example values f…
060ea1f 
…or init container

add unit tests

add missing braces
@sssd-dev
Copy link

@jasonodonnell Can this be merged, please?

@tvoran tvoran added chart Area: helm chart enhancement New feature or request labels Jul 2, 2020
@tvoran tvoran mentioned this pull request Jul 2, 2020
Closed
jasonodonnell
jasonodonnell approved these changes Jul 14, 2020
View reviewed changes
Copy link
Contributor

@jasonodonnell jasonodonnell left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I really like the approach for adding volumes in this PR. I agree that extraVolumes is opinionated, so I'm going to mark that as deprecated (and begin phase out) in support of this approach.

@jasonodonnell jasonodonnell merged commit 8c741f6 into hashicorp:master Jul 14, 2020
This was referenced Jul 14, 2020
Closed
Closed
Closed
@jasonodonnell jasonodonnell mentioned this pull request Aug 24, 2020
Merged
@georgekaz georgekaz deleted the support-init-containers branch December 21, 2020 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonodonnell jasonodonnell jasonodonnell approved these changes

Assignees
No one assigned
Labels
chart Area: helm chart enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@georgekaz @allan-degnan-rft @sssd-dev @jasonodonnell @tvoran