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 multiple helm values files #985

Merged

Conversation

GeertJohan
Copy link
Contributor

@GeertJohan GeertJohan commented Sep 17, 2018

This PR adds support for multiple helm values files in the skaffold config.

The helm deploy config the field valuesFilePath has been renamed to valuesFiles and now accepts a list of strings, instead of a string.
All valuesFiles are added to helm deploy using the -f option.

This PR was based on the changes and discussion in #677.

@GeertJohan GeertJohan force-pushed the feature/multiple-values-files branch 2 times, most recently from b2a5764 to ec43045 Compare September 17, 2018 10:20
@GeertJohan
Copy link
Contributor Author

Closes #672

@nkubala
Copy link
Contributor

nkubala commented Sep 18, 2018

Hey @GeertJohan, thanks for keeping up with this one. Unfortunately this won't work; since we already rolled out the v1alpha3 config change, anybody who's already used skaffold fix to update their configs will be broken by this. You'll need to make a v1alpha4 config and a new transform function instead of updating the existing one. The rest of the change looks fine though

@r2d4
Copy link
Contributor

r2d4 commented Sep 18, 2018

@nkubala we haven't actually shipped v1alpha3 yet, so this might be fine

@nkubala
Copy link
Contributor

nkubala commented Sep 18, 2018

Oh wow my bad, somehow I thought we got that in for the release last week...I should have looked 😨 @GeertJohan disregard my last comment

@GeertJohan
Copy link
Contributor Author

Alright, is there anything else needed or can this be merged now? How to make kokoro happy?

@dgageot dgageot merged commit e7108c6 into GoogleContainerTools:master Sep 21, 2018
@GeertJohan GeertJohan deleted the feature/multiple-values-files branch September 21, 2018 08:11
@GeertJohan
Copy link
Contributor Author

Thanks @dgageot 🎉

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.

4 participants