Skip to content

Helm upgrade --reuse-values on clean chart doesn't apply #6899

Closed
@technicool

Description

@technicool

Output of helm version: version.BuildInfo{Version:"v3.0.0-rc.3", GitCommit:"2ed206799b451830c68bff30af2a52879b8b937a", GitTreeState:"clean", GoVersion:"go1.13.4"}

Output of kubectl version:
Client Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.1", GitCommit:"4485c6f18cee9a5d3c3b4e523bd27972b1b53892", GitTreeState:"clean", BuildDate:"2019-07-18T09:18:22Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.3", GitCommit:"2d3c76f9091b6bec110a5e63777c332469e0cba2", GitTreeState:"clean", BuildDate:"2019-08-19T11:05:50Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.):
DigitalOcean

Premise

When creating a new chart, and then attempting to apply values using either --set or via a file (-f values.yaml), the new values will not apply if there have been no user supplied values.

Detailed logs

These have been reproducable for the at least rc-2 and rc-3. Multiple times, and in both the default and other namespaces. For example, I am using the stable/redis chart; although, any chart appears to have this issue, including file-based charts.

$ helm install redis stable/redis
....

$ helm upgrade redis stable/redis --reuse-values --set a=a
Release "redis" has been upgraded. Happy Helming!
NAME: redis
LAST DEPLOYED: Wed Nov  6 21:54:07 2019
NAMESPACE: default
STATUS: deployed
REVISION: 2
TEST SUITE: None
NOTES:
...

$ helm get values redis
USER-SUPPLIED VALUES:
null

Notice how there is nothing coming back, when we expected

USER-SUPPLIED VALUES:
a: a

Instead, it appears that we must first omit --reuse-values the first time we modify the chart.

$ helm upgrade redis stable/redis --set a=a

$ helm get values redis
USER-SUPPLIED VALUES:
a: a

$ helm upgrade redis stable/redis --reuse-values --set b=b
...

$ helm get values redis
USER-SUPPLIED VALUES:
a: a
b: b

$ helm upgrade redis stable/redis --reuse-values --set a=c
...

$ helm get values redis
USER-SUPPLIED VALUES:
a: c
b: b

Activity

added
bugCategorizes issue or PR as related to a bug.
on Nov 7, 2019
karuppiah7890

karuppiah7890 commented on Nov 7, 2019

@karuppiah7890
Contributor

Able to reproduce. Will check why this occurs

karuppiah7890

karuppiah7890 commented on Nov 7, 2019

@karuppiah7890
Contributor

This is what I tried to debug:

$ helm upgrade test test --reuse-values --set "something=blah"

The error occurs at this line

newVals = chartutil.CoalesceTables(newVals, current.Config)

where current.Config refers to the current release's values, which is nil and the newValues is a map[string]string containing "something":"blah" key value pair, according to my example

And then here

// CoalesceTables merges a source map into a destination map.
//
// dest is considered authoritative.
func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} {
if dst == nil || src == nil {
return src
}

nil is returned. And that's what is stored in the newValues here

newVals = chartutil.CoalesceTables(newVals, current.Config)

which gets returned here

return newVals, nil

and then it comes here

vals, err = u.reuseValues(chart, currentRelease, vals)

and then it gets stored in the release object here, which then gets stored in the store (secrets/config maps)

helm/pkg/action/upgrade.go

Lines 169 to 180 in 7f7e90b

upgradedRelease := &release.Release{
Name: name,
Namespace: currentRelease.Namespace,
Chart: chart,
Config: vals,
Info: &release.Info{
FirstDeployed: currentRelease.Info.FirstDeployed,
LastDeployed: Timestamper(),
Status: release.StatusPendingUpgrade,
Description: "Preparing upgrade", // This should be overwritten later.
},
Version: revision,

Looks like the if condition here is the issue

// CoalesceTables merges a source map into a destination map.
//
// dest is considered authoritative.
func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} {
if dst == nil || src == nil {
return src
}

Not sure what's the idea behind the if condition. I would expect dst to be returned if dst has a value and also since it has more precedence. Any thoughts on this @bacongobbler ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugCategorizes issue or PR as related to a bug.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @technicool@bacongobbler@karuppiah7890

      Issue actions

        Helm upgrade --reuse-values on clean chart doesn't apply · Issue #6899 · helm/helm