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

[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

[karuppiah7890]

Able to reproduce. Will check why this occurs

[karuppiah7890]

This is what I tried to debug:

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

The error occurs at this line

helm/pkg/action/upgrade.go

Line 361 in 7f7e90b

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

helm/pkg/chartutil/coalesce.go

Lines 179 to 185 in a758490

	// 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

helm/pkg/action/upgrade.go

Line 361 in 7f7e90b

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

which gets returned here

helm/pkg/action/upgrade.go

Line 365 in 7f7e90b

return newVals, nil

and then it comes here

helm/pkg/action/upgrade.go

Line 128 in 7f7e90b

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

helm/pkg/chartutil/coalesce.go

Lines 179 to 185 in a758490

	// 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 ?