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(helm): Add extraVolumes and extraVolumeMounts #432

Conversation

duynguyenhoang
Copy link

I would like to use json file in secret (Bigquery credential for example) and mount it to container. This PR will allow us to do that.

@burningalchemist
Copy link
Owner

Hey @duynguyenhoang thanks for your contribution! 👍

@allanger could you please review this PR whenever you have some time? 🙂

@allanger
Copy link
Contributor

Will do it tomorrow

## - spec for VolumeMount:
## https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.26/#volumemount-v1-core
##
extraVolumeMounts: []
Copy link
Contributor

Choose a reason for hiding this comment

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

Just as a suggestion. If these Volumes and Mounts are not supposed to exist one without another, maybe it's better to add one property to values, and then use it in templates, to make sure that every volume is always mounted?

Copy link
Contributor

@allanger allanger left a comment

Choose a reason for hiding this comment

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

I've added a wee comment about the chart part.

But I think I have a questing regarding the exporter part. Is it not the same thing that's done by: #397? This (for example json) config that is mounted in this repo is supposed to server the same purpose as the secret/config mounted there?

If yes, maybe we can combine these 2 PRs, because I think that the from approach that is used there is easier for chart users than defining the whole volumes/mounts thingy

@burningalchemist
Copy link
Owner

Closed in favour of #446.

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