helm: Values in library subcharts are ignored
Output of helm version:
version.BuildInfo{Version:"v3.8.2", GitCommit:"6e3701edea09e5d55a8ca2aae03a68917630e91b", GitTreeState:"clean", GoVersion:"go1.17.5"}
Output of kubectl version:
Client Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.1", GitCommit:"86ec240af8cbd1b60bcc4c03c20da9b98005b92e", GitTreeState:"clean", BuildDate:"2021-12-16T11:41:01Z", GoVersion:"go1.17.5", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.7+k3s1", GitCommit:"8432d7f239676dfe8f748c0c2a3fabf8cf40a826", GitTreeState:"clean", BuildDate:"2022-02-24T23:03:47Z", GoVersion:"go1.16.10", Compiler:"gc", Platform:"linux/amd64"}
Cloud Provider/Platform (AKS, GKE, Minikube etc.): N/A
Description
Starting from helm v3.8.2 a NBC change has been included, changing the precedence of imported values from library charts via imported-values in Chart.yaml. The following commit introduced that change: https://github.com/helm/helm/commit/5d017e11f1f47345a3559bf70f63d81a7edc981a.
An illustration for this behavior change can be seen in the following repository https://github.com/albgp22/helm-values-issue but can be stated simply as follows: when we import a value from a given library via imported-values in Chart.yaml file in our application chart, the value gets ignored if it is already present in the values.yaml file.
As this precedence change potentially breaks compatibility of existing charts with newer helm versions, I wonder if it will be reverted in future helm versions. As I see it, imported values from libraries overriding the ones in the application values.yaml file was the expected and logical behavior.
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 7
- Comments: 23 (2 by maintainers)
Commits related to this issue
- Fix multiple bugs in values handling First, some notes about priority and how some code flow works. For Helm handling values, the expected order of precidence is: 1. User specified values (e.g CLI) ... — committed to mattfarina/helm by mattfarina a year ago
- Fix multiple bugs in values handling First, some notes about priority and how some code flow works. For Helm handling values, the expected order of precidence is: 1. User specified values (e.g CLI) ... — committed to mattfarina/helm by mattfarina a year ago
- Fix multiple bugs in values handling First, some notes about priority and how some code flow works. For Helm handling values, the expected order of precidence is: 1. User specified values (e.g CLI) ... — committed to mattfarina/helm by mattfarina a year ago
- Fix multiple bugs in values handling First, some notes about priority and how some code flow works. For Helm handling values, the expected order of precidence is: 1. User specified values (e.g CLI) ... — committed to mattfarina/helm by mattfarina a year ago
- Fix multiple bugs in values handling First, some notes about priority and how some code flow works. For Helm handling values, the expected order of precidence is: 1. User specified values (e.g CLI) ... — committed to wahabmk/helm by mattfarina a year ago
@bacongobbler , @mattfarina could you comment? I think we should revert the non-backward-compatible (NBC) change introduced in Helm behavior from 3.8.2 onwards (because of the merged PR closing #9940) as soon as possible. The proposed PR #1162 for that is still providing a solution for the people asking for a different behavior, so when this PR being merged we will
We (and some others) are blocking the upgrade (in our deployment pipelines) to newer Helm versions because of this introduced NBC from 3.8.2
Note, I have been working on this. I just tested my fix against the chart from the initial description and the bug is fixed. Note, there are tests in the coming PR for this case so a change should be caught in the future.
There are so many cases in which a strange/odd behavior has not been considered a bug as if doing like that the new Helm release would introduce a non-backward compatible change (so breaking those deployment workflows expecting the current behavior) and so, in those cases, it was proposed to solve it as a new feature (e.g. proposing to add some new input param to the helm command in order to get the new desired behavior without breaking compatibility).
I think not following this approach for #9440 was a mistake and so it would be good to solve it in a coming release, doing as @Cybermaxke comments above (i.e., to get the behavior implemented by #9440 as a new feature to be triggered by some additional input param passed to the helm install/upgrade command and/or additional Chart.yaml field asking for it).
Sadly enough, in our case it’s not that easy to change to the latest version because of this, and would require some big workarounds.
I don’t think that the new default behavior should be changed at this point, because that would break charts that already depend on this change, but I would like to see the import strategy being added. And that these changes are properly reflected in the documentation, because they are still not up-to-date.
Some input from the devs about this would be appreciated, because I don’t know how to proceed at this point.
Hi Guys is it really solved? I Observe the same behaviour as stated here in v3.12.3: version.BuildInfo{Version:“v3.12.3”, GitCommit:“3a31588ad33fe3b89af5a2a54ee1d25bfe6eaa5e”, GitTreeState:“clean”, GoVersion:“go1.20.7”}
Yep I can see now that it is for Milestone 3.13.0 😉
Hope this fixes soon via either PR. Just wanted to clarify something: there are things which cannot be done at all if the old behavior is changed (Although, those are themselves workarounds for other limitations of Helm), because you’ll lose the possibility of overriding some existing values based on a condition/tag.
Thanks!