From 871b092f32ea5920dbec881f75e3f9caa3db2748 Mon Sep 17 00:00:00 2001 From: rokii Date: Sat, 1 Jun 2019 00:14:53 +0800 Subject: [PATCH 1/2] fix issue 5792 Signed-off-by: rokii --- .../upgrade-with-missing-dependencies.txt | 4 +- .../upgradetest/templates/configmap.yaml | 7 +++ cmd/helm/upgrade_test.go | 62 ++++++++++++++++++- pkg/action/upgrade.go | 12 ++-- 4 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/upgradetest/templates/configmap.yaml diff --git a/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt b/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt index e2186cd90..de62e1d2a 100644 --- a/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt +++ b/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt @@ -1,3 +1 @@ -Error: "helm upgrade" requires 2 arguments - -Usage: helm upgrade [RELEASE] [CHART] [flags] +Error: found in Chart.yaml, but missing in charts/ directory: reqsubchart2 diff --git a/cmd/helm/testdata/testcharts/upgradetest/templates/configmap.yaml b/cmd/helm/testdata/testcharts/upgradetest/templates/configmap.yaml new file mode 100644 index 000000000..b6b90efba --- /dev/null +++ b/cmd/helm/testdata/testcharts/upgradetest/templates/configmap.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: "{{ .Release.Name }}-configmap" +data: + myvalue: "Hello World" + drink: {{ .Values.favoriteDrink }} \ No newline at end of file diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index 1d8dc0317..cf3e7b899 100644 --- a/cmd/helm/upgrade_test.go +++ b/cmd/helm/upgrade_test.go @@ -18,6 +18,8 @@ package main import ( "fmt" + "strings" + "io/ioutil" "path/filepath" "testing" @@ -125,7 +127,7 @@ func TestUpgradeCmd(t *testing.T) { }, { name: "upgrade a release with missing dependencies", - cmd: fmt.Sprintf("upgrade bonkers-bunny%s", missingDepsPath), + cmd: fmt.Sprintf("upgrade bonkers-bunny %s", missingDepsPath), golden: "output/upgrade-with-missing-dependencies.txt", wantError: true, }, @@ -138,3 +140,61 @@ func TestUpgradeCmd(t *testing.T) { } runTestCmd(t, tests) } + +func TestUpgradeWithValue(t *testing.T) { + tmpChart := testTempDir(t) + configmapData, err := ioutil.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml") + if err != nil { + t.Fatalf("Error loading template yaml %v", err) + } + cfile := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV1, + Name: "testUpgradeChart", + Description: "A Helm chart for Kubernetes", + Version: "0.1.0", + }, + Templates: []*chart.File{&chart.File{Name: "templates/configmap.yaml", Data: configmapData}}, + } + chartPath := filepath.Join(tmpChart, cfile.Metadata.Name) + if err := chartutil.SaveDir(cfile, tmpChart); err != nil { + t.Fatalf("Error creating chart for upgrade: %v", err) + } + ch, err := loader.Load(chartPath) + if err != nil { + t.Fatalf("Error loading chart: %v", err) + } + _ = release.Mock(&release.MockReleaseOptions{ + Name: "funny-bunny-v2", + Chart: ch, + }) + + + relMock := func(n string, v int, ch *chart.Chart) *release.Release { + return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch}) + } + + defer resetEnv()() + + store := storageFixture() + + store.Create(relMock("funny-bunny-v2", 3, ch)) + + cmd := fmt.Sprintf("upgrade funny-bunny-v2 --set favoriteDrink=tea '%s'", chartPath) + _, _, err = executeActionCommandC(store, cmd) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + updatedRel, err := store.Get("funny-bunny-v2", 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + if !strings.Contains(updatedRel.Manifest, "drink: tea") { + t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest) + } + + + +} \ No newline at end of file diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index eddacc6ea..2e06596e8 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -45,8 +45,6 @@ type Upgrade struct { Namespace string Timeout time.Duration Wait bool - // Values is a string containing (unparsed) YAML values. - Values map[string]interface{} DisableHooks bool DryRun bool Force bool @@ -67,7 +65,7 @@ func NewUpgrade(cfg *Configuration) *Upgrade { // Run executes the upgrade on the given release. func (u *Upgrade) Run(name string, chart *chart.Chart) (*release.Release, error) { - if err := chartutil.ProcessDependencies(chart, u.Values); err != nil { + if err := chartutil.ProcessDependencies(chart, u.rawValues); err != nil { return nil, err } @@ -154,7 +152,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele if err != nil { return nil, nil, err } - valuesToRender, err := chartutil.ToRenderValues(chart, u.Values, options, caps) + valuesToRender, err := chartutil.ToRenderValues(chart, u.rawValues, options, caps) if err != nil { return nil, nil, err } @@ -169,7 +167,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele Name: name, Namespace: currentRelease.Namespace, Chart: chart, - Config: u.Values, + Config: u.rawValues, Info: &release.Info{ FirstDeployed: currentRelease.Info.FirstDeployed, LastDeployed: Timestamper(), @@ -262,7 +260,7 @@ func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release) erro return errors.Wrap(err, "failed to rebuild old values") } - u.Values = chartutil.CoalesceTables(current.Config, u.Values) + u.rawValues = chartutil.CoalesceTables(current.Config, u.rawValues) chart.Values = oldVals @@ -271,7 +269,7 @@ func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release) erro if len(u.Values) == 0 && len(current.Config) > 0 { u.cfg.Log("copying values from %s (v%d) to new release.", current.Name, current.Version) - u.Values = current.Config + u.rawValues = current.Config } return nil } From 897a79a57f728e619b7db5bfd0583a1e484e3ff6 Mon Sep 17 00:00:00 2001 From: rokii Date: Tue, 4 Jun 2019 16:27:41 +0800 Subject: [PATCH 2/2] fix and add test cases Signed-off-by: rokii --- .../testcharts/upgradetest/values.yaml | 1 + cmd/helm/upgrade_test.go | 113 +++++++++++++----- pkg/action/upgrade.go | 2 +- 3 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/upgradetest/values.yaml diff --git a/cmd/helm/testdata/testcharts/upgradetest/values.yaml b/cmd/helm/testdata/testcharts/upgradetest/values.yaml new file mode 100644 index 000000000..c429f41f4 --- /dev/null +++ b/cmd/helm/testdata/testcharts/upgradetest/values.yaml @@ -0,0 +1 @@ +favoriteDrink: beer \ No newline at end of file diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index cf3e7b899..6c6165c3f 100644 --- a/cmd/helm/upgrade_test.go +++ b/cmd/helm/upgrade_test.go @@ -18,9 +18,9 @@ package main import ( "fmt" - "strings" "io/ioutil" "path/filepath" + "strings" "testing" "helm.sh/helm/pkg/chart" @@ -142,6 +142,88 @@ func TestUpgradeCmd(t *testing.T) { } func TestUpgradeWithValue(t *testing.T) { + releaseName := "funny-bunny-v2" + relMock, ch, chartPath := prepareMockRelease(releaseName, t) + + defer resetEnv()() + + store := storageFixture() + + store.Create(relMock(releaseName, 3, ch)) + + cmd := fmt.Sprintf("upgrade %s --set favoriteDrink=tea '%s'", releaseName, chartPath) + _, _, err := executeActionCommandC(store, cmd) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + updatedRel, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + if !strings.Contains(updatedRel.Manifest, "drink: tea") { + t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest) + } + +} + +func TestUpgradeWithStringValue(t *testing.T) { + releaseName := "funny-bunny-v3" + relMock, ch, chartPath := prepareMockRelease(releaseName, t) + + defer resetEnv()() + + store := storageFixture() + + store.Create(relMock(releaseName, 3, ch)) + + cmd := fmt.Sprintf("upgrade %s --set-string favoriteDrink=coffee '%s'", releaseName, chartPath) + _, _, err := executeActionCommandC(store, cmd) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + updatedRel, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + if !strings.Contains(updatedRel.Manifest, "drink: coffee") { + t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest) + } + +} + +func TestUpgradeWithValuesFile(t *testing.T) { + + releaseName := "funny-bunny-v4" + relMock, ch, chartPath := prepareMockRelease(releaseName, t) + + defer resetEnv()() + + store := storageFixture() + + store.Create(relMock(releaseName, 3, ch)) + + cmd := fmt.Sprintf("upgrade %s --values testdata/testcharts/upgradetest/values.yaml '%s'", releaseName, chartPath) + _, _, err := executeActionCommandC(store, cmd) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + updatedRel, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + if !strings.Contains(updatedRel.Manifest, "drink: beer") { + t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest) + } + +} + +func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) { tmpChart := testTempDir(t) configmapData, err := ioutil.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml") if err != nil { @@ -165,36 +247,13 @@ func TestUpgradeWithValue(t *testing.T) { t.Fatalf("Error loading chart: %v", err) } _ = release.Mock(&release.MockReleaseOptions{ - Name: "funny-bunny-v2", + Name: releaseName, Chart: ch, }) - relMock := func(n string, v int, ch *chart.Chart) *release.Release { return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch}) } - defer resetEnv()() - - store := storageFixture() - - store.Create(relMock("funny-bunny-v2", 3, ch)) - - cmd := fmt.Sprintf("upgrade funny-bunny-v2 --set favoriteDrink=tea '%s'", chartPath) - _, _, err = executeActionCommandC(store, cmd) - if err != nil { - t.Errorf("unexpected error, got '%v'", err) - } - - updatedRel, err := store.Get("funny-bunny-v2", 4) - if err != nil { - t.Errorf("unexpected error, got '%v'", err) - } - - if !strings.Contains(updatedRel.Manifest, "drink: tea") { - t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest) - } - - - -} \ No newline at end of file + return relMock, ch, chartPath +} diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 2e06596e8..1e2c805db 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -267,7 +267,7 @@ func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release) erro return nil } - if len(u.Values) == 0 && len(current.Config) > 0 { + if len(u.rawValues) == 0 && len(current.Config) > 0 { u.cfg.Log("copying values from %s (v%d) to new release.", current.Name, current.Version) u.rawValues = current.Config }