diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index 1ea7edd8c..a47073b67 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -94,6 +94,22 @@ func (v Values) Encode(w io.Writer) error { return err } +// MergeInto takes the properties in src and merges them into Values. Maps +// are merged while values and arrays are replaced. +func (v Values) MergeInto(src Values) { + for key, srcVal := range src { + destVal, found := v[key] + + if found && istable(srcVal) && istable(destVal) { + destMap := destVal.(map[string]interface{}) + srcMap := srcVal.(map[string]interface{}) + Values(destMap).MergeInto(Values(srcMap)) + } else { + v[key] = srcVal + } + } +} + func tableLookup(v Values, simple string) (Values, error) { v2, ok := v[simple] if !ok { diff --git a/pkg/chartutil/values_test.go b/pkg/chartutil/values_test.go index f54b25827..f38afaf95 100644 --- a/pkg/chartutil/values_test.go +++ b/pkg/chartutil/values_test.go @@ -20,6 +20,7 @@ import ( "bytes" "encoding/json" "fmt" + "reflect" "testing" "text/template" @@ -457,3 +458,84 @@ chapter: } } } + +func TestValuesMergeInto(t *testing.T) { + testCases := map[string]struct { + destination string + source string + result string + }{ + "maps are merged": { + ` +resources: + requests: + cpu: 400m + something: else +`, + ` +resources: + requests: + cpu: 500m +`, + ` +resources: + requests: + cpu: 500m + something: else +`, + }, + "values are replaced": { + ` +firstKey: firstValue +secondKey: secondValue +thirdKey: thirdValue +`, + ` +firstKey: newFirstValue +thirdKey: newThirdValue +`, + ` +firstKey: newFirstValue +secondKey: secondValue +thirdKey: newThirdValue +`, + }, + "new values are added": { + ` +existingKey: existingValue +`, + ` +newKey: newValue +anotherNewKey: + nestedNewKey: nestedNewValue +`, + ` +existingKey: existingValue +newKey: newValue +anotherNewKey: + nestedNewKey: nestedNewValue +`, + }, + } + + for name, tc := range testCases { + d, err := ReadValues([]byte(tc.destination)) + if err != nil { + t.Error(err) + } + s, err := ReadValues([]byte(tc.source)) + if err != nil { + t.Error(err) + } + expectedRes, err := ReadValues([]byte(tc.result)) + if err != nil { + t.Error(err) + } + + d.MergeInto(s) + + if !reflect.DeepEqual(expectedRes, d) { + t.Errorf("%s: Expected %v, but got %v", name, expectedRes, d) + } + } +} diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go index f5e96ed00..cf7748f44 100644 --- a/pkg/tiller/release_server.go +++ b/pkg/tiller/release_server.go @@ -25,7 +25,6 @@ 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/client-go/kubernetes" @@ -136,28 +135,28 @@ func (s *ReleaseServer) reuseValues(req *services.UpdateReleaseRequest, current if err != nil { return err } + req.Chart.Values = &chart.Config{Raw: nv} + + reqValues, err := chartutil.ReadValues([]byte(req.Values.Raw)) + if err != nil { + return err + } - // merge new values with current + currentConfig := chartutil.Values{} if current.Config != nil && current.Config.Raw != "" && current.Config.Raw != "{}\n" { - if req.Values.Raw != "{}\n" { - req.Values.Raw = current.Config.Raw + "\n" + req.Values.Raw - } else { - req.Values.Raw = current.Config.Raw + "\n" + currentConfig, err = chartutil.ReadValues([]byte(current.Config.Raw)) + if err != nil { + return err } } - 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) + currentConfig.MergeInto(reqValues) + data, err := currentConfig.YAML() if err != nil { return err } - req.Values.Raw = string(data) + req.Values.Raw = data return nil } diff --git a/pkg/tiller/release_update_test.go b/pkg/tiller/release_update_test.go index 1f189a8b7..fe9ef12eb 100644 --- a/pkg/tiller/release_update_test.go +++ b/pkg/tiller/release_update_test.go @@ -23,10 +23,12 @@ import ( "github.com/golang/protobuf/proto" + "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/helm" "k8s.io/helm/pkg/proto/hapi/chart" "k8s.io/helm/pkg/proto/hapi/release" "k8s.io/helm/pkg/proto/hapi/services" + "reflect" ) func TestUpdateRelease(t *testing.T) { @@ -168,6 +170,71 @@ func TestUpdateRelease_ReuseValuesWithNoValues(t *testing.T) { } } +func TestUpdateRelease_NestedReuseValues(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")}, + }, + Values: &chart.Config{Raw: "defaultFoo: defaultBar"}, + }, + Values: &chart.Config{Raw: ` +foo: bar +root: + nested: nestedValue + anotherNested: anotherNestedValue +`}, + } + + 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")}, + }, + Values: &chart.Config{Raw: "defaultFoo: defaultBar"}, + }, + Values: &chart.Config{Raw: ` +root: + nested: newNestedValue +`}, + ReuseValues: true, + } + + res, err := rs.UpdateRelease(c, req) + if err != nil { + t.Fatalf("Failed updated: %s", err) + } + + expect, _ := chartutil.ReadValues([]byte(` +foo: bar +root: + nested: newNestedValue + anotherNested: anotherNestedValue +`)) + + requestConfig, err := chartutil.ReadValues([]byte(res.Release.Config.Raw)) + if err != nil { + t.Errorf("Request config could not be parsed: %v", err) + } + + if !reflect.DeepEqual(expect, requestConfig) { + t.Errorf("Expected request config to be %v, got %v", expect, requestConfig) + } +} + // This is a regression test for bug found in issue #3655 func TestUpdateRelease_ComplexReuseValues(t *testing.T) { c := helm.NewContext()