diff --git a/cmd/helm/install.go b/cmd/helm/install.go index bdfd66d50..8b0cbea70 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -155,7 +155,7 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal f.BoolVar(&client.Atomic, "atomic", false, "if set, the installation process deletes the installation on failure. The --wait flag will be set automatically if --atomic is used") f.BoolVar(&client.SkipCRDs, "skip-crds", false, "if set, no CRDs will be installed. By default, CRDs are installed if not already present") f.BoolVar(&client.SubNotes, "render-subchart-notes", false, "if set, render subchart notes along with the parent") - f.StringToStringVarP(&client.Labels, "labels", "l", nil, "Labels that would be added to relese metadata. Should be divided by comma. (Currently works only with configmap and secret storage drivers). Original release labels wouldn't persisted in upgraded release.") + f.StringToStringVarP(&client.Labels, "labels", "l", nil, "Labels that would be added to relese metadata. Should be divided by comma. (Currently works only with configmap and secret storage drivers).") addValueOptionsFlags(f, valueOpts) addChartPathOptionsFlags(f, &client.ChartPathOptions) diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 584816ae9..b08c50e13 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -229,7 +229,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.IntVar(&client.MaxHistory, "history-max", settings.MaxHistory, "limit the maximum number of revisions saved per release. Use 0 for no limit") f.BoolVar(&client.CleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this upgrade when upgrade fails") f.BoolVar(&client.SubNotes, "render-subchart-notes", false, "if set, render subchart notes along with the parent") - f.StringToStringVarP(&client.Labels, "labels", "l", nil, "Labels that would be added to relese metadata. Should be divided by comma. (Currently works only with configmap and secret storage drivers). Original release labels wouldn't persisted in upgraded release.") + f.StringToStringVarP(&client.Labels, "labels", "l", nil, "Labels that would be added to relese metadata. Should be divided by comma. (Currently works only with configmap and secret storage drivers). Original release labels would be merged with upgrade labels. You can unset label using null.") f.StringVar(&client.Description, "description", "", "add a custom description") f.BoolVar(&client.DependencyUpdate, "dependency-update", false, "update dependencies if they are missing before installing the chart") addChartPathOptionsFlags(f, &client.ChartPathOptions) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 5c8899b9c..44036842a 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -252,7 +252,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin Version: revision, Manifest: manifestDoc.String(), Hooks: hooks, - Labels: u.Labels, + Labels: mergeCustomLabels(lastRelease.Labels, u.Labels), } if len(notesTxt) > 0 { @@ -571,3 +571,13 @@ func objectKey(r *resource.Info) string { gvk := r.Object.GetObjectKind().GroupVersionKind() return fmt.Sprintf("%s/%s/%s/%s", gvk.GroupVersion().String(), gvk.Kind, r.Namespace, r.Name) } + +func mergeCustomLabels(current, desired map[string]string) map[string]string { + labels := mergeStrStrMaps(current, desired) + for k, v := range labels { + if v == "null" { + delete(labels, k) + } + } + return labels +} diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 35e1bcac5..44113c865 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -19,6 +19,7 @@ package action import ( "context" "fmt" + "reflect" "testing" "time" @@ -388,6 +389,21 @@ func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { is.Equal(updatedRes.Info.Status, release.StatusDeployed) } +func TestMergeCustomLabels(t *testing.T) { + var tests = [][3]map[string]string{ + {nil, nil, map[string]string{}}, + {map[string]string{}, map[string]string{}, map[string]string{}}, + {map[string]string{"k1": "v1", "k2": "v2"}, nil, map[string]string{"k1": "v1", "k2": "v2"}}, + {nil, map[string]string{"k1": "v1", "k2": "v2"}, map[string]string{"k1": "v1", "k2": "v2"}}, + {map[string]string{"k1": "v1", "k2": "v2"}, map[string]string{"k1": "null", "k2": "v3"}, map[string]string{"k2": "v3"}}, + } + for _, test := range tests { + if output := mergeCustomLabels(test[0], test[1]); !reflect.DeepEqual(test[2], output) { + t.Errorf("Expected {%v}, got {%v}", test[2], output) + } + } +} + func TestUpgradeRelease_Labels(t *testing.T) { is := assert.New(t) upAction := upgradeAction(t) @@ -395,10 +411,9 @@ func TestUpgradeRelease_Labels(t *testing.T) { rel := releaseStub() rel.Name = "labels" // It's needed to check that suppressed release would keep original labels - // Also it's needed for check that original release labels not passed to upgraded release, cause right now this functionality is not implemented and would cause problems with deletion of labels (meant nullifing existing labels) rel.Labels = map[string]string{ "key1": "val1", - "key2": "val2", + "key2": "val2.1", } rel.Info.Status = release.StatusDeployed @@ -406,14 +421,15 @@ func TestUpgradeRelease_Labels(t *testing.T) { is.NoError(err) upAction.Labels = map[string]string{ + "key1": "null", + "key2": "val2.2", "key3": "val3", - "key4": "val4", } // setting newValues and upgrading res, err := upAction.Run(rel.Name, buildChart(), nil) is.NoError(err) - // Now make sure it is actually upgraded + // Now make sure it is actually upgraded and labels were merged updatedRes, err := upAction.cfg.Releases.Get(res.Name, 2) is.NoError(err) @@ -422,7 +438,7 @@ func TestUpgradeRelease_Labels(t *testing.T) { return } is.Equal(release.StatusDeployed, updatedRes.Info.Status) - is.Equal(upAction.Labels, updatedRes.Labels) + is.Equal(mergeCustomLabels(rel.Labels, upAction.Labels), updatedRes.Labels) // Now make sure it is suppressed release still contains original labels initialRes, err := upAction.cfg.Releases.Get(res.Name, 1)