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: rename values.yaml.tmpl to values.yaml #3701

Merged
merged 1 commit into from
Aug 27, 2019
Merged

Conversation

k0da
Copy link
Contributor

@k0da k0da commented Aug 26, 2019

version can be set over values.yaml directly, there is no reason
to copy over a file, while lack of values.yaml file makes git tree
helm incompatible.

This is low effort fix for #3217

Signed-off-by: Dinar Valeev k0da@opensuse.org

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #3217

Checklist:

  • Reviewed the developer guide on Submitting a Pull Request
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.
  • Pending release notes updated with breaking and/or notable changes, if necessary.
  • Upgrade from previous release is tested and upgrade user guide is updated, if necessary.
  • Code generation (make codegen) has been run to update object specifications, if necessary.
  • Comments have been added or updated based on the standards set in CONTRIBUTING.md
  • Add the flag for skipping the CI if this PR does not require a build. See here for more details.

@k0da
Copy link
Contributor Author

k0da commented Aug 26, 2019

replicatedhq/ship#930

version can be set over values.yaml directly, there is no reason
to copy over a file, while lack of values.yaml file makes git tree
helm incompatible.

This is low effort fix for rook#3217

Signed-off-by: Dinar Valeev <k0da@opensuse.org>
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

LGTM, the build is passing so it seems a minor thing that the .tmpl file wasn't needed. Tools like ship won't care that the values.yaml is unusable until the variable is replaced?

@k0da
Copy link
Contributor Author

k0da commented Aug 27, 2019

@travisn ship just runs some sort of helm template, to get you initial objects. A user still can set a tag to its own value. For helm/ship %%version%% is till valid value :)
Yes it is a little bit inconvenient, that you have to set tag, but at least that enable use rook chart through a gitpath and leverage ship/kustomize to maintain rook setup.

@travisn travisn merged commit d11481e into rook:master Aug 27, 2019
krig pushed a commit to krig/rook that referenced this pull request Sep 3, 2019
Since values.yaml is no longer generated from a template
thanks to rook#3701, the .gitignore file shouldn't be needed.

Signed-off-by: Kristoffer Grönlund <kgronlund@suse.com>
BlaineEXE pushed a commit to SUSE/rook that referenced this pull request Sep 6, 2019
Since values.yaml is no longer generated from a template
thanks to rook#3701, the .gitignore file shouldn't be needed.

Signed-off-by: Kristoffer Grönlund <kgronlund@suse.com>
leseb pushed a commit to leseb/rook that referenced this pull request Oct 3, 2019
Since values.yaml is no longer generated from a template
thanks to rook#3701, the .gitignore file shouldn't be needed.

Signed-off-by: Kristoffer Grönlund <kgronlund@suse.com>
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.

Non-standard Helm Chart prevents usage with Ship
2 participants