From bfd825080377db771821ef742ac513dc1dcc7ccb Mon Sep 17 00:00:00 2001 From: Karuppiah Natarajan Date: Thu, 24 Oct 2019 19:04:48 +0530 Subject: [PATCH] fix list not showing multiple releases with same name in different namespaces (#6756) Signed-off-by: Karuppiah Natarajan --- pkg/action/list.go | 31 ++++++++++++-------------- pkg/action/list_test.go | 35 +++++++++++++++++++++++++----- pkg/storage/driver/cfgmaps_test.go | 2 +- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/pkg/action/list.go b/pkg/action/list.go index f72edc83b..4af0ef8e5 100644 --- a/pkg/action/list.go +++ b/pkg/action/list.go @@ -17,6 +17,7 @@ limitations under the License. package action import ( + "path" "regexp" "helm.sh/helm/v3/pkg/release" @@ -218,27 +219,23 @@ func (l *List) sort(rels []*release.Release) { } // filterList returns a list scrubbed of old releases. -func filterList(rels []*release.Release) []*release.Release { - idx := map[string]int{} - - for _, r := range rels { - name, version := r.Name, r.Version - if max, ok := idx[name]; ok { - // check if we have a greater version already - if max > version { - continue - } +func filterList(releases []*release.Release) []*release.Release { + latestReleases := make(map[string]*release.Release) + + for _, rls := range releases { + name, namespace := rls.Name, rls.Namespace + key := path.Join(namespace, name) + if latestRelease, exists := latestReleases[key]; exists && latestRelease.Version > rls.Version { + continue } - idx[name] = version + latestReleases[key] = rls } - uniq := make([]*release.Release, 0, len(idx)) - for _, r := range rels { - if idx[r.Name] == r.Version { - uniq = append(uniq, r) - } + var list = make([]*release.Release, 0, len(latestReleases)) + for _, rls := range latestReleases { + list = append(list, rls) } - return uniq + return list } // setStateMask calculates the state mask based on parameters. diff --git a/pkg/action/list_test.go b/pkg/action/list_test.go index cab0111fd..cd86aee68 100644 --- a/pkg/action/list_test.go +++ b/pkg/action/list_test.go @@ -159,6 +159,7 @@ func TestList_LimitOffsetOutOfBounds(t *testing.T) { is.NoError(err) is.Len(list, 2) } + func TestList_StateMask(t *testing.T) { is := assert.New(t) lister := newListFixture(t) @@ -166,7 +167,8 @@ func TestList_StateMask(t *testing.T) { one, err := lister.cfg.Releases.Get("one", 1) is.NoError(err) one.SetStatus(release.StatusUninstalled, "uninstalled") - lister.cfg.Releases.Update(one) + err = lister.cfg.Releases.Update(one) + is.NoError(err) res, err := lister.Run() is.NoError(err) @@ -222,11 +224,6 @@ func makeMeSomeReleases(store *storage.Storage, t *testing.T) { three.Name = "three" three.Namespace = "default" three.Version = 3 - four := releaseStub() - four.Name = "four" - four.Namespace = "default" - four.Version = 4 - four.Info.Status = release.StatusSuperseded for _, rel := range []*release.Release{one, two, three} { if err := store.Create(rel); err != nil { @@ -238,3 +235,29 @@ func makeMeSomeReleases(store *storage.Storage, t *testing.T) { assert.NoError(t, err) assert.Len(t, all, 3, "sanity test: three items added") } + +func TestFilterList(t *testing.T) { + one := releaseStub() + one.Name = "one" + one.Namespace = "default" + one.Version = 1 + two := releaseStub() + two.Name = "two" + two.Namespace = "default" + two.Version = 1 + anotherOldOne := releaseStub() + anotherOldOne.Name = "one" + anotherOldOne.Namespace = "testing" + anotherOldOne.Version = 1 + anotherOne := releaseStub() + anotherOne.Name = "one" + anotherOne.Namespace = "testing" + anotherOne.Version = 2 + + list := []*release.Release{one, two, anotherOne} + expectedFilteredList := []*release.Release{one, two, anotherOne} + + filteredList := filterList(list) + + assert.ElementsMatch(t, expectedFilteredList, filteredList) +} diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go index 6efd790ce..2aa38f284 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -51,7 +51,7 @@ func TestConfigMapGet(t *testing.T) { } } -func TestUNcompressedConfigMapGet(t *testing.T) { +func TestUncompressedConfigMapGet(t *testing.T) { vers := 1 name := "smug-pigeon" namespace := "default"