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

Add support for --extra-create-metadata for CSI provisioner in Helm chart #565

Closed
jwesolowski-rms opened this issue Sep 18, 2020 · 2 comments

Comments

@jwesolowski-rms
Copy link

Is your feature request related to a problem?/Why is this needed
When using the Helm chart, I am unable to set --extra-create-metadata=true for the CSI provisioner. The only options can be seen here:

- name: csi-provisioner
   image: {{ printf "%s:%s" .Values.sidecars.provisionerImage.repository .Values.sidecars.provisionerImage.tag }}
   args:
     - --csi-address=$(ADDRESS)
     - --v=5
     {{- if .Values.enableVolumeScheduling }}
     - --feature-gates=Topology=true
     {{- end}}
     - --enable-leader-election
     - --leader-election-type=leases

/feature

Describe the solution you'd like in detail
It would be nice if we added this as a flag similar to the .Values.enableVolumeScheduling that would conditionally add the --extra-create-metadata=true to the provisioner.

Describe alternatives you've considered
Another alternative I thought of would be to have a list value that would allow us to add arbitrary args to each container. This would make it so we didn't need to do a code change to the Helm chart every time we want to add a new feature that requires an arg chance. For example, we could have a .Values.additionalArgsCSIProvisioner that is a list of additional arguments to set for the provisioner container. This could be expanded to support the other containers as well.

Rather than immediately open a pull request, I wanted to get some feedback on these possible approaches and see what the maintainers and community felt was the right approach.

@kcking
Copy link
Contributor

kcking commented Oct 7, 2020

I went ahead and made a first attempt at this here: #577

It's working well in our EKS 1.17 cluster

@jwesolowski-rms
Copy link
Author

Closed by #577

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

2 participants