Skip to content
This repository has been archived by the owner on Nov 7, 2019. It is now read-only.

Fix Helm Chart so extraArgs and extraEnv render correctly #151

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mattparkes
Copy link

@mattparkes mattparkes commented Dec 28, 2018

Issue: #150

Description of changes:
Fix Helm Chart so extraArgs and extraEnv render correctly.

Testing:
Can test/validate by using a values file (test.yaml) whose contents are exactly as per the default values.yaml, but with the addition of testing --key=value extraArgs items render properly:

extraEnv:
  - name: my_env
    value: my_value

extraArgs:
  - --help
  - --foo=bar

helm template ./aws-service-operator/ -f test.yaml

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mattparkes
Copy link
Author

I have not incremented the version in Chart.yaml.

Let me know if you think it is preferred/appropriate to do so and I'll increment it to 0.0.2.

@mattparkes
Copy link
Author

mattparkes commented Dec 28, 2018

A colleague of mine just pointed out that technically the existing template did work if you used:

extraEnv:
  my_env: my_value

So rather than changing the template, I could have just changed the default values.yaml file to something like the above.

extraArgs doesn't seem to work no matter the format though:

extraArgs:
 - help
 - foo: bar

renders as:

args:
- --0=help
- --1=map[foo:bar]

So a change is still needed to fix that.

I guess this totally a stylistic choice, so if anyone has a strong preference, I'm happy to adjust this PR to suit.

@christopherhein
Copy link
Contributor

So sounds like we can actually just change the Values.yaml to represent extraEnv.my_env: my_value instead and that will solve the first issue? Rather than changing the template?

And the second change lgtm. Would you mind just going back to using the $key $value reference and we'll be good to merge?
Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants