Merge pull request #8282 from andrewmelis/list-history-filter

Make helm ls return only current releases if providing state filter

pkg/action/list.go

@@ -152,12 +152,6 @@ func (l *List) Run() ([]*release.Release, error) {
 	}
 
 	results, err := l.cfg.Releases.List(func(rel *release.Release) bool {
-		// Skip anything that the mask doesn't cover
-		currentStatus := l.StateMask.FromName(rel.Info.Status.String())
-		if l.StateMask&currentStatus == 0 {
-			return false
-		}
-
 		// Skip anything that doesn't match the filter.
 		if filter != nil && !filter.MatchString(rel.Name) {
 			return false
@@ -173,7 +167,16 @@ func (l *List) Run() ([]*release.Release, error) {
 		return results, nil
 	}
 
-	results = filterList(results)
+	// by definition, superseded releases are never shown if
+	// only the latest releases are returned. so if requested statemask
+	// is _only_ ListSuperseded, skip the latest release filter
+	if l.StateMask != ListSuperseded {
+		results = filterLatestReleases(results)
+	}
+
+	// State mask application must occur after filtering to
+	// latest releases, otherwise outdated entries can be returned
+	results = l.filterStateMask(results)
 
 	// Unfortunately, we have to sort before truncating, which can incur substantial overhead
 	l.sort(results)
@@ -222,8 +225,8 @@ func (l *List) sort(rels []*release.Release) {
 	}
 }
 
-// filterList returns a list scrubbed of old releases.
+// filterLatestReleases returns a list scrubbed of old releases.
-func filterList(releases []*release.Release) []*release.Release {
+func filterLatestReleases(releases []*release.Release) []*release.Release {
 	latestReleases := make(map[string]*release.Release)
 
 	for _, rls := range releases {
@@ -242,6 +245,21 @@ func filterList(releases []*release.Release) []*release.Release {
 	return list
 }
 
+func (l *List) filterStateMask(releases []*release.Release) []*release.Release {
+	desiredStateReleases := make([]*release.Release, 0)
+
+	for _, rls := range releases {
+		currentStatus := l.StateMask.FromName(rls.Info.Status.String())
+		mask := l.StateMask & currentStatus
+		if mask == 0 {
+			continue
+		}
+		desiredStateReleases = append(desiredStateReleases, rls)
+	}
+
+	return desiredStateReleases
+}
+
 // SetStateMask calculates the state mask based on parameters.
 func (l *List) SetStateMask() {
 	if l.All {

pkg/action/list_test.go

@@ -188,6 +188,56 @@ func TestList_StateMask(t *testing.T) {
 	is.Len(res, 3)
 }
 
+func TestList_StateMaskWithStaleRevisions(t *testing.T) {
+	is := assert.New(t)
+	lister := newListFixture(t)
+	lister.StateMask = ListFailed
+
+	makeMeSomeReleasesWithStaleFailure(lister.cfg.Releases, t)
+
+	res, err := lister.Run()
+
+	is.NoError(err)
+	is.Len(res, 1)
+
+	// "dirty" release should _not_ be present as most recent
+	// release is deployed despite failed release in past
+	is.Equal("failed", res[0].Name)
+}
+
+func makeMeSomeReleasesWithStaleFailure(store *storage.Storage, t *testing.T) {
+	t.Helper()
+	one := namedReleaseStub("clean", release.StatusDeployed)
+	one.Namespace = "default"
+	one.Version = 1
+
+	two := namedReleaseStub("dirty", release.StatusDeployed)
+	two.Namespace = "default"
+	two.Version = 1
+
+	three := namedReleaseStub("dirty", release.StatusFailed)
+	three.Namespace = "default"
+	three.Version = 2
+
+	four := namedReleaseStub("dirty", release.StatusDeployed)
+	four.Namespace = "default"
+	four.Version = 3
+
+	five := namedReleaseStub("failed", release.StatusFailed)
+	five.Namespace = "default"
+	five.Version = 1
+
+	for _, rel := range []*release.Release{one, two, three, four, five} {
+		if err := store.Create(rel); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	all, err := store.ListReleases()
+	assert.NoError(t, err)
+	assert.Len(t, all, 5, "sanity test: five items added")
+}
+
 func TestList_Filter(t *testing.T) {
 	is := assert.New(t)
 	lister := newListFixture(t)
@@ -236,7 +286,7 @@ func makeMeSomeReleases(store *storage.Storage, t *testing.T) {
 	assert.Len(t, all, 3, "sanity test: three items added")
 }
 
-func TestFilterList(t *testing.T) {
+func TestFilterLatestReleases(t *testing.T) {
 	t.Run("should filter old versions of the same release", func(t *testing.T) {
 		r1 := releaseStub()
 		r1.Name = "r"
@@ -248,7 +298,7 @@ func TestFilterList(t *testing.T) {
 		another.Name = "another"
 		another.Version = 1
 
-		filteredList := filterList([]*release.Release{r1, r2, another})
+		filteredList := filterLatestReleases([]*release.Release{r1, r2, another})
 		expectedFilteredList := []*release.Release{r2, another}
 
 		assert.ElementsMatch(t, expectedFilteredList, filteredList)
@@ -264,7 +314,7 @@ func TestFilterList(t *testing.T) {
 		r2.Namespace = "testing"
 		r2.Version = 2
 
-		filteredList := filterList([]*release.Release{r1, r2})
+		filteredList := filterLatestReleases([]*release.Release{r1, r2})
 		expectedFilteredList := []*release.Release{r1, r2}
 
 		assert.ElementsMatch(t, expectedFilteredList, filteredList)