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

Upgrade to new chart version and --set a value at the same time with --reuse-values ignores changes from values.yaml #3957

Closed
andremarianiello opened this issue Apr 24, 2018 · 12 comments · Fixed by #9653

Comments

@andremarianiello
Copy link

andremarianiello commented Apr 24, 2018

Output of helm version:

Client: &version.Version{SemVer:"v2.8.2", GitCommit:"a80231648a1473929271764b920a8e346f6de844", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.8.2", GitCommit:"a80231648a1473929271764b920a8e346f6de844", GitTreeState:"clean"}

Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"9", GitVersion:"v1.9.6", GitCommit:"9f8ebd171479bec0ada837d7ee641dec2f8c6dd1", GitTreeState:"clean", BuildDate:"2018-03-21T15:21:50Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"8", GitVersion:"v1.8.2", GitCommit:"bdaeafa71f6c7c04636251031f93464384d54963", GitTreeState:"clean", BuildDate:"2017-10-24T19:38:10Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.):
Kubeadm cluster on vsphere

I can demonstrate the problem with the chart created with helm create, but it is not specific to that chart in any way

helm create temp
cd temp
helm upgrade temp . --install --set replicaCount=2
# at this point I have a release with custom values I want to preserve
#
# but let's say a new version of the chart becomes available that adds new defaults to values.yaml
echo "hello: world" >> values.yaml
echo "goodbye: world" >> values.yaml

# I want to use this new version, but the defaults for one of the new values doesn't work for me
# so I can try to upgrade
helm upgrade temp . --set goodbye=nobody --install --debug --dry-run
# whoops, I would lose my custom settings if I ran that
# let's reuse values (that's what it's for, right?)
helm upgrade temp . --install --set goodbye=nobody --reuse-values  --debug --dry-run
# now replicaCount is still set, but, weirdly, hello:world is missing from the computed values
# seems like --reuse-values doesn't use values introduced by the new version of the chart
#
# if I didn't need to --set any values, I could run
helm upgrade temp . --install --debug --dry-run
# replicaCount is still 2, but the setting I wanted to set is not set, forcing me to upgrade in two steps

This behavior is unexpected, and will be hard to people to debug if they are not very familiar with all the new values introduced by a new chart version.

@technosophos
Copy link
Member

--reuse-values does exactly what it says it does: It re-uses the values from the last release, not the ones supplied in the chart. And as the documentation states, it only merges in values that are explicitly passed in with --set or --values.

The design goal is to prevent changes from a new chart release's values from automatically being applied. This is a task that cannot be done any other way than by explicitly telling the server to do it. So it gets its own flag.

Your desired workflow can be accomplished by doing a helm get values foo > values.yaml && helm upgrade foo ./foochart -f values --set .... In that case, the merge will proceed like this:

  • --set is merged into values.yaml
  • values.yaml is sent to the server with the chart
  • the server merges chart values and your values
  • the template is rendered and the values are stored

@andremarianiello
Copy link
Author

Thanks for your response.

From the help:

--reuse-values         when upgrading, reuse the last release's values, and merge in any new values. If '--reset-values' is specified, this is ignored.

Here 'last release's values' means computed values not user-supplied values. It does exactly what it says, but what is says is not precise, and led to this misunderstanding. Perhaps this can be clarified somehow? I can make a PR to update the help text for this if you wish.

--reuse-values does exactly what it says it does: It re-uses the values from the last release, not the ones supplied in the chart.

I couldn't find anything in the docs that said that the values from the chart are ignored, though if one understands that "last release's values" means "computed values", then this fact can be deduced.

Your desired workflow can be accomplished by doing a helm get values foo > values.yaml && helm upgrade foo ./foochart -f values.yaml --set .... In that case, the merge will proceed like this:

From what I have seen, helm get values displays the user-supplied from from the last release only. This is a problem if I have been upgrading my release using --reuse-values to incrementally supply user values. Only the last change is stored as a user-supplied value. All user-supplied values from previous releases will get lost with that suggested workflow. This is another unintuitive behavioral quirk of how --reuse-values works. If I had my way I would make helm preserve and merge in command line values with the user-supplied values from the previous release to get the user-supplied values for this release, and apply those to the chart values to get the computed values for this release. --reuse-values would preserve and merge user-supplied values from the previous release. I assume you have given this much more thought than I, so I would be interested in hearing the reasoning. (If you already describe this somewhere a link works just as well.)

I know this isn't strictly on the topic of the issue, so if there is discussion about this elsewhere I would be happy to close this and discuss it there instead.

@zaro
Copy link

zaro commented Apr 30, 2020

I hit the same thing today.

@technosophos What you describe is fine workaround for CI and scripting, but consider the following case, where the user is simply upgrading to a new version of a chart, some values were added to the default values.yaml and then the deployment fails because the newly added values are not present.

Doesn't it make sense at least for the case where chart is different version of the currently deployed, to have the default values.yaml also used as a starting point ?

@Envek
Copy link

Envek commented May 18, 2020

Current behavior of helm upgrade --reuse-values is very counter-intuitive from user point of view (as of Helm 3.2.1). It is very annoying that when I add new templates and default values into application's chart, it uses new templates, but doesn't use new values which leads to templates errors at best and to partial upgrades at worst (e.g. when there is {{ if .Values.new.enabled }} in templates and default value for new.enabled is true, then these templates sections won't be included as expected).


So, for example, if I add following snippet to some deployment definition:

+           ports:
+             - containerPort: {{ .Values.metrics.port }}
+               name: metrics

And values for it:

+ monitoring:
+   port: 8001

And then execute helm upgrade ./helm-chart --reuse-values --wait --atomic

Then I get following error:

Error: UPGRADE FAILED: template: cool-app/templates/deployment.whatever.yaml:19:39: executing "cool-app/templates/deployment.whatever.yaml" at <.Values.metrics.port>: nil pointer evaluating interface {}.port

While expected result is to perform upgrade using new default values.

At the same time all testing commands like helm template and helm lint works perfectly.

@bacongobbler
Copy link
Member

bacongobbler commented Aug 14, 2020

when I add new templates and default values into application's chart, it uses new templates, but doesn't use new values

That is the EXACT design of --reuse-values. That was how it was written and it was intentionally designed that way as described earlier. I'm not sure how that can be argued differently here.

If you want to use both new templates and new values, don't use --reuse-values.

Perhaps we need to make it abundantly clear that --reuse-values takes precedence and ignores values from --set, --set-string, --values, and other value-related flags.

@bacongobbler
Copy link
Member

#8085 describes a new feature flag to handle this alternative use case, which seems reasonable.

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Jan 26, 2021
@jglick
Copy link

jglick commented Jan 26, 2021

I do not think this should be “stale”; wasted a couple hours trying to understand what was wrong with my chart, when the answer was nothing. I would like to see --reuse-values documentation clearly warn you that this will not do what you want, in the typical case that what you want is to upgrade your chart while keeping your existing overrides and honoring any newly introduced chart parameters, and referring you to a new option

helm upgrade --keep-values $release $chart

which would effectively do

helm get values -o yaml $release > /tmp/v.yaml
helm upgrade -f /tmp/v.yaml $release $chart

@github-actions github-actions bot removed the Stale label Jan 27, 2021
@danports
Copy link

Agreed. I ran into this issue today too. I don't think the behavior of the --reuse-values flag follows the principle of least surprise. #8085 seems like a reasonable way to handle this without breaking backwards compatibility.

@bshh bshh mentioned this issue Mar 20, 2021
Okhoshi added a commit to Okhoshi/helm that referenced this issue Apr 27, 2021
When '--reset-then-reuse-values' is used on 'helm upgrade', the chart's values will be
reset to the values of the deployed chart while the current release's values will be
reused and merged with the values passed as argument (is any). '--reset-values' and
'--reuse-values' flags take precedence over `--reset-then-reuse-values', making it
ignored if one or the other is also used.

Closes helm#8085, helm#3957

Signed-off-by: Quentin Devos <quentin@devos.pm>
Okhoshi added a commit to Okhoshi/helm that referenced this issue Apr 27, 2021
When '--reset-then-reuse-values' is used on 'helm upgrade', the chart's values will be
reset to the values of the deployed chart while the current release's values will be
reused and merged with the values passed as argument (is any). '--reset-values' and
'--reuse-values' flags take precedence over `--reset-then-reuse-values', making it
ignored if one or the other is also used.

Closes helm#8085, helm#3957

Signed-off-by: Quentin Devos <quentin@devos.pm>
@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label May 19, 2021
@Envek
Copy link

Envek commented May 19, 2021

Not stale. Also there is pull request to fix this: #9653

@github-actions github-actions bot removed the Stale label May 20, 2021
@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants