From 4dd21780bb6debf2eccfef2c37bdcb52bf649abd Mon Sep 17 00:00:00 2001
From: Michelle Noorali <michellemolu@gmail.com>
Date: Wed, 4 Apr 2018 15:44:28 -0400
Subject: [PATCH] fix(pkg/tiller): reuseValues combines all prev val
Resolves #3655
We were seeing that when running helm upgrade with the reuse-values
flag enabled that you could end up in the position where overrides
a.k.a computed values from previous revisions were not being saved on
the updated revision. This left us in a weird position where some
computed values would disappear mysteriously in the abyss. That
happened because computed values from previous revisions weren't merged
with the new computed values every time the reuse-values flag was used.
This PR merges computed values from the previous revisions so you don't
end up in that kind of conundrum.
(cherry picked from commit 9266731dc45aac2aa6a726e5a887ee1078aaf35f)
---
pkg/tiller/release_server.go | 16 +++++
pkg/tiller/release_update_test.go | 106 +++++++++++++++++++++++++++++-
2 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go
index eaa6515bb..7c4bc62cf 100644
--- a/pkg/tiller/release_server.go
+++ b/pkg/tiller/release_server.go
@@ -25,6 +25,7 @@ import (
"strings"
"github.com/technosophos/moniker"
+ "gopkg.in/yaml.v2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/discovery"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
@@ -135,7 +136,22 @@ func (s *ReleaseServer) reuseValues(req *services.UpdateReleaseRequest, current
if err != nil {
return err
}
+
+ // merge new values with current
+ req.Values.Raw = current.Config.Raw + "\n" + req.Values.Raw
req.Chart.Values = &chart.Config{Raw: nv}
+
+ // yaml unmarshal and marshal to remove duplicate keys
+ y := map[string]interface{}{}
+ if err := yaml.Unmarshal([]byte(req.Values.Raw), &y); err != nil {
+ return err
+ }
+ data, err := yaml.Marshal(y)
+ if err != nil {
+ return err
+ }
+
+ req.Values.Raw = string(data)
return nil
}
diff --git a/pkg/tiller/release_update_test.go b/pkg/tiller/release_update_test.go
index 642952f19..0189201d1 100644
--- a/pkg/tiller/release_update_test.go
+++ b/pkg/tiller/release_update_test.go
@@ -17,6 +17,7 @@ limitations under the License.
package tiller
import (
+ "fmt"
"strings"
"testing"
@@ -128,6 +129,107 @@ func TestUpdateRelease_ResetValues(t *testing.T) {
}
}
+// This is a regression test for bug found in issue #3655
+func TestUpdateRelease_ComplexReuseValues(t *testing.T) {
+ c := helm.NewContext()
+ rs := rsFixture()
+
+ installReq := &services.InstallReleaseRequest{
+ Namespace: "spaced",
+ Chart: &chart.Chart{
+ Metadata: &chart.Metadata{Name: "hello"},
+ Templates: []*chart.Template{
+ {Name: "templates/hello", Data: []byte("hello: world")},
+ {Name: "templates/hooks", Data: []byte(manifestWithHook)},
+ },
+ Values: &chart.Config{Raw: "defaultFoo: defaultBar"},
+ },
+ Values: &chart.Config{Raw: "foo: bar"},
+ }
+
+ fmt.Println("Running Install release with foo: bar override")
+ installResp, err := rs.InstallRelease(c, installReq)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ rel := installResp.Release
+ req := &services.UpdateReleaseRequest{
+ Name: rel.Name,
+ Chart: &chart.Chart{
+ Metadata: &chart.Metadata{Name: "hello"},
+ Templates: []*chart.Template{
+ {Name: "templates/hello", Data: []byte("hello: world")},
+ {Name: "templates/hooks", Data: []byte(manifestWithUpgradeHooks)},
+ },
+ Values: &chart.Config{Raw: "defaultFoo: defaultBar"},
+ },
+ }
+
+ fmt.Println("Running Update release with no overrides and no reuse-values flag")
+ res, err := rs.UpdateRelease(c, req)
+ if err != nil {
+ t.Fatalf("Failed updated: %s", err)
+ }
+
+ expect := "foo: bar"
+ if res.Release.Config != nil && res.Release.Config.Raw != expect {
+ t.Errorf("Expected chart values to be %q, got %q", expect, res.Release.Config.Raw)
+ }
+
+ rel = res.Release
+ req = &services.UpdateReleaseRequest{
+ Name: rel.Name,
+ Chart: &chart.Chart{
+ Metadata: &chart.Metadata{Name: "hello"},
+ Templates: []*chart.Template{
+ {Name: "templates/hello", Data: []byte("hello: world")},
+ {Name: "templates/hooks", Data: []byte(manifestWithUpgradeHooks)},
+ },
+ Values: &chart.Config{Raw: "defaultFoo: defaultBar"},
+ },
+ Values: &chart.Config{Raw: "foo2: bar2"},
+ ReuseValues: true,
+ }
+
+ fmt.Println("Running Update release with foo2: bar2 override and reuse-values")
+ res, err = rs.UpdateRelease(c, req)
+ if err != nil {
+ t.Fatalf("Failed updated: %s", err)
+ }
+
+ // This should have the newly-passed overrides.
+ expect = "foo: bar\nfoo2: bar2\n"
+ if res.Release.Config != nil && res.Release.Config.Raw != expect {
+ t.Errorf("Expected request config to be %q, got %q", expect, res.Release.Config.Raw)
+ }
+
+ rel = res.Release
+ req = &services.UpdateReleaseRequest{
+ Name: rel.Name,
+ Chart: &chart.Chart{
+ Metadata: &chart.Metadata{Name: "hello"},
+ Templates: []*chart.Template{
+ {Name: "templates/hello", Data: []byte("hello: world")},
+ {Name: "templates/hooks", Data: []byte(manifestWithUpgradeHooks)},
+ },
+ Values: &chart.Config{Raw: "defaultFoo: defaultBar"},
+ },
+ Values: &chart.Config{Raw: "foo: baz"},
+ ReuseValues: true,
+ }
+
+ fmt.Println("Running Update release with foo=baz override with reuse-values flag")
+ res, err = rs.UpdateRelease(c, req)
+ if err != nil {
+ t.Fatalf("Failed updated: %s", err)
+ }
+ expect = "foo: baz\nfoo2: bar2\n"
+ if res.Release.Config != nil && res.Release.Config.Raw != expect {
+ t.Errorf("Expected chart values to be %q, got %q", expect, res.Release.Config.Raw)
+ }
+}
+
func TestUpdateRelease_ReuseValues(t *testing.T) {
c := helm.NewContext()
rs := rsFixture()
@@ -157,8 +259,8 @@ func TestUpdateRelease_ReuseValues(t *testing.T) {
if res.Release.Chart.Values != nil && res.Release.Chart.Values.Raw != expect {
t.Errorf("Expected chart values to be %q, got %q", expect, res.Release.Chart.Values.Raw)
}
- // This should have the newly-passed overrides.
- expect = "name2: val2"
+ // This should have the newly-passed overrides and any other computed values. `name: value` comes from release Config via releaseStub()
+ expect = "name: value\nname2: val2"
if res.Release.Config != nil && res.Release.Config.Raw != expect {
t.Errorf("Expected request config to be %q, got %q", expect, res.Release.Config.Raw)
}
--
GitLab