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

[Helm Chart] Allow extraVolumes and extraVolumeMounts to be specified in values. #16359

Closed
cccs-tom opened this issue Aug 19, 2021 · 3 comments
Closed

Comments

@cccs-tom
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When using Superset as a subchart, it is impossible (known Helm limitation) to "inject" configuration or Python (or other) files using .Files.Get or similar. We would like to be able to define our own ConfigMaps / Secrets / other volumes and mount them into the pods. Other charts we use include extraVolumes and extraVolumeMounts in their templates for this purpose. I would like to add those to the Superset chart.

Describe the solution you'd like
What we have currently implemented (which I would gladly contribute) are extraVolumes and extraVolumeMounts objects at the root of the Superset values. Those are applied to the main containers of all 3 deployments as well as the init-job. The values looks like this:

extraVolumes:
  volume1:
    configMap:
      name: '{{ template "superset.fullname" . }}-custom-config'
  volume2:
    secret:
      secretName: my-secret
extraVolumeMounts:
  volume1:
    mountPath: /mnt/volume1
  volume2:
    mountPath: /mnt/volume2

The key for each object becomes the 'name' field (so the volume and volumeMount keys must match)

Describe alternatives you've considered
The above fits our use case well, but other use cases may not require all volumes to be mounted everywhere. One possible alternative would be to have extraVolumes and extraVolumeMounts objects under supersetNode, supersetWorker, etc. In our specific use case, it would mean some repetition and additional maintenance, but that would be an acceptable solution nevertheless.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 16, 2022
@rusackas
Copy link
Member

@cccs-tom Should we move this to an "Ideas" thread for further discussion, if it's something you want to keep getting feedback on? Or, would you want to open the PR you mention contributing? CC @craig-rueda for further opinion(s) here.

@stale stale bot removed the inactive Inactive for >= 30 days label Jan 27, 2023
@cccs-tom
Copy link
Contributor Author

@cccs-tom Should we move this to an "Ideas" thread for further discussion, if it's something you want to keep getting feedback on? Or, would you want to open the PR you mention contributing? CC @craig-rueda for further opinion(s) here.

@rusackas As far as I'm concerned, it is an oversight that this issue wasn't closed when my PR (#16361) was merged. This solution has been working great for us and our use case(s) require no further improvements at this time...

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

No branches or pull requests

4 participants