You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
helm/pkg/action/list_test.go

369 lines
9.1 KiB

/*
Copyright The Helm Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package action
import (
"testing"
"github.com/stretchr/testify/assert"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/storage"
)
func TestListStates(t *testing.T) {
for input, expect := range map[string]ListStates{
"deployed": ListDeployed,
"uninstalled": ListUninstalled,
"uninstalling": ListUninstalling,
"superseded": ListSuperseded,
"failed": ListFailed,
"pending-install": ListPendingInstall,
"pending-rollback": ListPendingRollback,
"pending-upgrade": ListPendingUpgrade,
"unknown": ListUnknown,
"totally made up key": ListUnknown,
} {
if expect != expect.FromName(input) {
t.Errorf("Expected %d for %s", expect, input)
}
// This is a cheap way to verify that ListAll actually allows everything but Unknown
if got := expect.FromName(input); got != ListUnknown && got&ListAll == 0 {
t.Errorf("Expected %s to match the ListAll filter", input)
}
}
filter := ListDeployed | ListPendingRollback
if status := filter.FromName("deployed"); filter&status == 0 {
t.Errorf("Expected %d to match mask %d", status, filter)
}
if status := filter.FromName("failed"); filter&status != 0 {
t.Errorf("Expected %d to fail to match mask %d", status, filter)
}
}
func TestList_Empty(t *testing.T) {
lister := NewList(actionConfigFixture(t))
list, err := lister.Run()
assert.NoError(t, err)
assert.Len(t, list, 0)
}
func newListFixture(t *testing.T) *List {
return NewList(actionConfigFixture(t))
}
func TestList_OneNamespace(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
makeMeSomeReleases(lister.cfg.Releases, t)
list, err := lister.Run()
is.NoError(err)
is.Len(list, 3)
}
func TestList_AllNamespaces(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
makeMeSomeReleases(lister.cfg.Releases, t)
lister.AllNamespaces = true
lister.SetStateMask()
list, err := lister.Run()
is.NoError(err)
is.Len(list, 3)
}
func TestList_Sort(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
lister.Sort = ByNameDesc // Other sorts are tested elsewhere
makeMeSomeReleases(lister.cfg.Releases, t)
list, err := lister.Run()
is.NoError(err)
is.Len(list, 3)
is.Equal("two", list[0].Name)
is.Equal("three", list[1].Name)
is.Equal("one", list[2].Name)
}
func TestList_Limit(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
lister.Limit = 2
makeMeSomeReleases(lister.cfg.Releases, t)
list, err := lister.Run()
is.NoError(err)
is.Len(list, 2)
// Lex order means one, three, two
is.Equal("one", list[0].Name)
is.Equal("three", list[1].Name)
}
func TestList_BigLimit(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
lister.Limit = 20
makeMeSomeReleases(lister.cfg.Releases, t)
list, err := lister.Run()
is.NoError(err)
is.Len(list, 3)
// Lex order means one, three, two
is.Equal("one", list[0].Name)
is.Equal("three", list[1].Name)
is.Equal("two", list[2].Name)
}
func TestList_LimitOffset(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
lister.Limit = 2
lister.Offset = 1
makeMeSomeReleases(lister.cfg.Releases, t)
list, err := lister.Run()
is.NoError(err)
is.Len(list, 2)
// Lex order means one, three, two
is.Equal("three", list[0].Name)
is.Equal("two", list[1].Name)
}
func TestList_LimitOffsetOutOfBounds(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
lister.Limit = 2
lister.Offset = 3 // Last item is index 2
makeMeSomeReleases(lister.cfg.Releases, t)
list, err := lister.Run()
is.NoError(err)
is.Len(list, 0)
lister.Limit = 10
lister.Offset = 1
list, err = lister.Run()
is.NoError(err)
is.Len(list, 2)
}
func TestList_StateMask(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
makeMeSomeReleases(lister.cfg.Releases, t)
one, err := lister.cfg.Releases.Get("one", 1)
is.NoError(err)
one.SetStatus(release.StatusUninstalled, "uninstalled")
err = lister.cfg.Releases.Update(one)
is.NoError(err)
res, err := lister.Run()
is.NoError(err)
is.Len(res, 2)
is.Equal("three", res[0].Name)
is.Equal("two", res[1].Name)
lister.StateMask = ListUninstalled
res, err = lister.Run()
is.NoError(err)
is.Len(res, 1)
is.Equal("one", res[0].Name)
lister.StateMask |= ListDeployed
res, err = lister.Run()
is.NoError(err)
is.Len(res, 3)
}
Make helm ls return only current releases if providing state filter Previously, the `helm ls --$state` operation would display outdated releases under certain conditions. Given the following set of releases: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE bar 1 Wed Apr 8 16:54:39 2020 DEPLOYED bar-4.0.0 1.0 default foo 1 Fri Feb 7 06:16:56 2020 DEPLOYED foo-0.1.0 1.0 default foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default foo 4 Tue May 5 08:16:56 2020 DEPLOYED foo-0.2.0 1.0 default qux 1 Tue Jun 9 10:32:00 2020 DEPLOYED qux-4.0.3 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.3 1.0 default ``` `helm ls --failed` produced the following output: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Including the `qux` release in that `helm ls --failed` output is not controversial; the most recent revision of `qux` was not successful and an operator should investigate. Including the `foo` release in the output, however, is questionable. Revision 3 of `foo` is _not_ the most recent release of `foo`, and that FAILED release was fixed in a susubsequent upgrade. A user may see that FAILED deploy and start taking inappropriate action. Further, that issue was fixed months ago in this example -- troubleshooting an old deploy may not be safe if significant changes have occurred. Concern over this behavior was raised in https://github.com/helm/helm/issues/7495. This behavior applied to all the state filter flags (--deployed, --failed, --pending, etc.), and a user could pass multiple state filter flags to a single command. The previous behavior can be summarized as follows: For each release name, all release revisions having any of the supplied state flags were retrieved, and the most recent revision among these was returned (regardless of whether a newer revision of an unspecified state exists). This change request alters the helm list action to match user expectations such that only "current" releases are shown when filtering on release state. After this change, the following output would be produced by `helm ls --failed`: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` The command now returns only `qux` because it is the only "current" FAILED release. This behavior change applies to all the state filters _except_ `superseded`, which now becomes a special case. By definition, at least one newer release exists ahead of each superseded release. A conditional is included in this change request to maintain the preexisting behavior (return "most recent" superseded revison for each release name) if the superseded state filter is requested. --- Note that there is an alternate perspective that a state filter flag should return all releases of a given state rather than only the "current" releases. In the above example, `helm ls --failed` with this approach would return the following: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Multiple FAILED `foo` revisions are included in the output, unlike the current behavior. This approach is logical and achievable. It allows a user to find exactly what is requested: all historical releases of a given state. In order to achieve continuity with helm behavior, however, a new filter (something like "current") would probably need to be implemented and become the new default. Given current helm behavior as well as the comments in the #7495, I did not pursue this approach. --- Technical details: - Moved list action state mask filter after latest release filter Previously, the list operation in helm/pkg/action/list.go skipped releases that were not covered by the state mask on _retrieval_ from the Releases store: ``` 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 } ... ``` https://github.com/helm/helm/blob/8ea6b970ecd02365a230420692350057d48278e5/pkg/action/list.go#L154-L159 While filtering on retrieval in this manner avoided an extra iteration through the entire list to check on the supplied condition later, it introduced the possibility of returning an outdated release to the user because newer releases (that would have otherwise squashed outdated releases in the `filterList` function) are simply not included in the set of working records. This change moves the state mask filtering process to _after_ the set of current releases is built. Outdated, potentially misleading releases are scrubbed out prior to the application of the state mask filter. As written, this state mask filtration (in the new `filterStateMask` method on `*List`) incurs an additional, potentially expensive iteration over the set of releases to return to the user. An alternative approach could avoid that extra iteration and fit this logic into the existing `filterList` function at the cost of making `filterList` function a little harder to understand. - Rename filterList to filterLatestReleases for clarity Another function that filters the list is added, so update to the more descriptive name here. - List superseded releases without filtering for latest This change makes superseded releases a special case, as they would _never_ be displayed otherwise (by definition, as superseded releases have been replaced by a newer release), so a conditional maintains current behavior ("return newest superseded revision for each release name") Fixes #7495. Signed-off-by: Andrew Melis <andrewmelis@gmail.com>
4 years ago
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)
lister.Filter = "th."
makeMeSomeReleases(lister.cfg.Releases, t)
res, err := lister.Run()
is.NoError(err)
is.Len(res, 1)
is.Equal("three", res[0].Name)
}
func TestList_FilterFailsCompile(t *testing.T) {
is := assert.New(t)
lister := newListFixture(t)
lister.Filter = "t[h.{{{"
makeMeSomeReleases(lister.cfg.Releases, t)
_, err := lister.Run()
is.Error(err)
}
func makeMeSomeReleases(store *storage.Storage, t *testing.T) {
t.Helper()
one := releaseStub()
one.Name = "one"
one.Namespace = "default"
one.Version = 1
two := releaseStub()
two.Name = "two"
two.Namespace = "default"
two.Version = 2
three := releaseStub()
three.Name = "three"
three.Namespace = "default"
three.Version = 3
for _, rel := range []*release.Release{one, two, three} {
if err := store.Create(rel); err != nil {
t.Fatal(err)
}
}
all, err := store.ListReleases()
assert.NoError(t, err)
assert.Len(t, all, 3, "sanity test: three items added")
}
Make helm ls return only current releases if providing state filter Previously, the `helm ls --$state` operation would display outdated releases under certain conditions. Given the following set of releases: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE bar 1 Wed Apr 8 16:54:39 2020 DEPLOYED bar-4.0.0 1.0 default foo 1 Fri Feb 7 06:16:56 2020 DEPLOYED foo-0.1.0 1.0 default foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default foo 4 Tue May 5 08:16:56 2020 DEPLOYED foo-0.2.0 1.0 default qux 1 Tue Jun 9 10:32:00 2020 DEPLOYED qux-4.0.3 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.3 1.0 default ``` `helm ls --failed` produced the following output: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Including the `qux` release in that `helm ls --failed` output is not controversial; the most recent revision of `qux` was not successful and an operator should investigate. Including the `foo` release in the output, however, is questionable. Revision 3 of `foo` is _not_ the most recent release of `foo`, and that FAILED release was fixed in a susubsequent upgrade. A user may see that FAILED deploy and start taking inappropriate action. Further, that issue was fixed months ago in this example -- troubleshooting an old deploy may not be safe if significant changes have occurred. Concern over this behavior was raised in https://github.com/helm/helm/issues/7495. This behavior applied to all the state filter flags (--deployed, --failed, --pending, etc.), and a user could pass multiple state filter flags to a single command. The previous behavior can be summarized as follows: For each release name, all release revisions having any of the supplied state flags were retrieved, and the most recent revision among these was returned (regardless of whether a newer revision of an unspecified state exists). This change request alters the helm list action to match user expectations such that only "current" releases are shown when filtering on release state. After this change, the following output would be produced by `helm ls --failed`: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` The command now returns only `qux` because it is the only "current" FAILED release. This behavior change applies to all the state filters _except_ `superseded`, which now becomes a special case. By definition, at least one newer release exists ahead of each superseded release. A conditional is included in this change request to maintain the preexisting behavior (return "most recent" superseded revison for each release name) if the superseded state filter is requested. --- Note that there is an alternate perspective that a state filter flag should return all releases of a given state rather than only the "current" releases. In the above example, `helm ls --failed` with this approach would return the following: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Multiple FAILED `foo` revisions are included in the output, unlike the current behavior. This approach is logical and achievable. It allows a user to find exactly what is requested: all historical releases of a given state. In order to achieve continuity with helm behavior, however, a new filter (something like "current") would probably need to be implemented and become the new default. Given current helm behavior as well as the comments in the #7495, I did not pursue this approach. --- Technical details: - Moved list action state mask filter after latest release filter Previously, the list operation in helm/pkg/action/list.go skipped releases that were not covered by the state mask on _retrieval_ from the Releases store: ``` 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 } ... ``` https://github.com/helm/helm/blob/8ea6b970ecd02365a230420692350057d48278e5/pkg/action/list.go#L154-L159 While filtering on retrieval in this manner avoided an extra iteration through the entire list to check on the supplied condition later, it introduced the possibility of returning an outdated release to the user because newer releases (that would have otherwise squashed outdated releases in the `filterList` function) are simply not included in the set of working records. This change moves the state mask filtering process to _after_ the set of current releases is built. Outdated, potentially misleading releases are scrubbed out prior to the application of the state mask filter. As written, this state mask filtration (in the new `filterStateMask` method on `*List`) incurs an additional, potentially expensive iteration over the set of releases to return to the user. An alternative approach could avoid that extra iteration and fit this logic into the existing `filterList` function at the cost of making `filterList` function a little harder to understand. - Rename filterList to filterLatestReleases for clarity Another function that filters the list is added, so update to the more descriptive name here. - List superseded releases without filtering for latest This change makes superseded releases a special case, as they would _never_ be displayed otherwise (by definition, as superseded releases have been replaced by a newer release), so a conditional maintains current behavior ("return newest superseded revision for each release name") Fixes #7495. Signed-off-by: Andrew Melis <andrewmelis@gmail.com>
4 years ago
func TestFilterLatestReleases(t *testing.T) {
t.Run("should filter old versions of the same release", func(t *testing.T) {
r1 := releaseStub()
r1.Name = "r"
r1.Version = 1
r2 := releaseStub()
r2.Name = "r"
r2.Version = 2
another := releaseStub()
another.Name = "another"
another.Version = 1
Make helm ls return only current releases if providing state filter Previously, the `helm ls --$state` operation would display outdated releases under certain conditions. Given the following set of releases: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE bar 1 Wed Apr 8 16:54:39 2020 DEPLOYED bar-4.0.0 1.0 default foo 1 Fri Feb 7 06:16:56 2020 DEPLOYED foo-0.1.0 1.0 default foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default foo 4 Tue May 5 08:16:56 2020 DEPLOYED foo-0.2.0 1.0 default qux 1 Tue Jun 9 10:32:00 2020 DEPLOYED qux-4.0.3 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.3 1.0 default ``` `helm ls --failed` produced the following output: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Including the `qux` release in that `helm ls --failed` output is not controversial; the most recent revision of `qux` was not successful and an operator should investigate. Including the `foo` release in the output, however, is questionable. Revision 3 of `foo` is _not_ the most recent release of `foo`, and that FAILED release was fixed in a susubsequent upgrade. A user may see that FAILED deploy and start taking inappropriate action. Further, that issue was fixed months ago in this example -- troubleshooting an old deploy may not be safe if significant changes have occurred. Concern over this behavior was raised in https://github.com/helm/helm/issues/7495. This behavior applied to all the state filter flags (--deployed, --failed, --pending, etc.), and a user could pass multiple state filter flags to a single command. The previous behavior can be summarized as follows: For each release name, all release revisions having any of the supplied state flags were retrieved, and the most recent revision among these was returned (regardless of whether a newer revision of an unspecified state exists). This change request alters the helm list action to match user expectations such that only "current" releases are shown when filtering on release state. After this change, the following output would be produced by `helm ls --failed`: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` The command now returns only `qux` because it is the only "current" FAILED release. This behavior change applies to all the state filters _except_ `superseded`, which now becomes a special case. By definition, at least one newer release exists ahead of each superseded release. A conditional is included in this change request to maintain the preexisting behavior (return "most recent" superseded revison for each release name) if the superseded state filter is requested. --- Note that there is an alternate perspective that a state filter flag should return all releases of a given state rather than only the "current" releases. In the above example, `helm ls --failed` with this approach would return the following: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Multiple FAILED `foo` revisions are included in the output, unlike the current behavior. This approach is logical and achievable. It allows a user to find exactly what is requested: all historical releases of a given state. In order to achieve continuity with helm behavior, however, a new filter (something like "current") would probably need to be implemented and become the new default. Given current helm behavior as well as the comments in the #7495, I did not pursue this approach. --- Technical details: - Moved list action state mask filter after latest release filter Previously, the list operation in helm/pkg/action/list.go skipped releases that were not covered by the state mask on _retrieval_ from the Releases store: ``` 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 } ... ``` https://github.com/helm/helm/blob/8ea6b970ecd02365a230420692350057d48278e5/pkg/action/list.go#L154-L159 While filtering on retrieval in this manner avoided an extra iteration through the entire list to check on the supplied condition later, it introduced the possibility of returning an outdated release to the user because newer releases (that would have otherwise squashed outdated releases in the `filterList` function) are simply not included in the set of working records. This change moves the state mask filtering process to _after_ the set of current releases is built. Outdated, potentially misleading releases are scrubbed out prior to the application of the state mask filter. As written, this state mask filtration (in the new `filterStateMask` method on `*List`) incurs an additional, potentially expensive iteration over the set of releases to return to the user. An alternative approach could avoid that extra iteration and fit this logic into the existing `filterList` function at the cost of making `filterList` function a little harder to understand. - Rename filterList to filterLatestReleases for clarity Another function that filters the list is added, so update to the more descriptive name here. - List superseded releases without filtering for latest This change makes superseded releases a special case, as they would _never_ be displayed otherwise (by definition, as superseded releases have been replaced by a newer release), so a conditional maintains current behavior ("return newest superseded revision for each release name") Fixes #7495. Signed-off-by: Andrew Melis <andrewmelis@gmail.com>
4 years ago
filteredList := filterLatestReleases([]*release.Release{r1, r2, another})
expectedFilteredList := []*release.Release{r2, another}
assert.ElementsMatch(t, expectedFilteredList, filteredList)
})
t.Run("should not filter out any version across namespaces", func(t *testing.T) {
r1 := releaseStub()
r1.Name = "r"
r1.Namespace = "default"
r1.Version = 1
r2 := releaseStub()
r2.Name = "r"
r2.Namespace = "testing"
r2.Version = 2
Make helm ls return only current releases if providing state filter Previously, the `helm ls --$state` operation would display outdated releases under certain conditions. Given the following set of releases: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE bar 1 Wed Apr 8 16:54:39 2020 DEPLOYED bar-4.0.0 1.0 default foo 1 Fri Feb 7 06:16:56 2020 DEPLOYED foo-0.1.0 1.0 default foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default foo 4 Tue May 5 08:16:56 2020 DEPLOYED foo-0.2.0 1.0 default qux 1 Tue Jun 9 10:32:00 2020 DEPLOYED qux-4.0.3 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.3 1.0 default ``` `helm ls --failed` produced the following output: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Including the `qux` release in that `helm ls --failed` output is not controversial; the most recent revision of `qux` was not successful and an operator should investigate. Including the `foo` release in the output, however, is questionable. Revision 3 of `foo` is _not_ the most recent release of `foo`, and that FAILED release was fixed in a susubsequent upgrade. A user may see that FAILED deploy and start taking inappropriate action. Further, that issue was fixed months ago in this example -- troubleshooting an old deploy may not be safe if significant changes have occurred. Concern over this behavior was raised in https://github.com/helm/helm/issues/7495. This behavior applied to all the state filter flags (--deployed, --failed, --pending, etc.), and a user could pass multiple state filter flags to a single command. The previous behavior can be summarized as follows: For each release name, all release revisions having any of the supplied state flags were retrieved, and the most recent revision among these was returned (regardless of whether a newer revision of an unspecified state exists). This change request alters the helm list action to match user expectations such that only "current" releases are shown when filtering on release state. After this change, the following output would be produced by `helm ls --failed`: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` The command now returns only `qux` because it is the only "current" FAILED release. This behavior change applies to all the state filters _except_ `superseded`, which now becomes a special case. By definition, at least one newer release exists ahead of each superseded release. A conditional is included in this change request to maintain the preexisting behavior (return "most recent" superseded revison for each release name) if the superseded state filter is requested. --- Note that there is an alternate perspective that a state filter flag should return all releases of a given state rather than only the "current" releases. In the above example, `helm ls --failed` with this approach would return the following: ``` NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default ``` Multiple FAILED `foo` revisions are included in the output, unlike the current behavior. This approach is logical and achievable. It allows a user to find exactly what is requested: all historical releases of a given state. In order to achieve continuity with helm behavior, however, a new filter (something like "current") would probably need to be implemented and become the new default. Given current helm behavior as well as the comments in the #7495, I did not pursue this approach. --- Technical details: - Moved list action state mask filter after latest release filter Previously, the list operation in helm/pkg/action/list.go skipped releases that were not covered by the state mask on _retrieval_ from the Releases store: ``` 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 } ... ``` https://github.com/helm/helm/blob/8ea6b970ecd02365a230420692350057d48278e5/pkg/action/list.go#L154-L159 While filtering on retrieval in this manner avoided an extra iteration through the entire list to check on the supplied condition later, it introduced the possibility of returning an outdated release to the user because newer releases (that would have otherwise squashed outdated releases in the `filterList` function) are simply not included in the set of working records. This change moves the state mask filtering process to _after_ the set of current releases is built. Outdated, potentially misleading releases are scrubbed out prior to the application of the state mask filter. As written, this state mask filtration (in the new `filterStateMask` method on `*List`) incurs an additional, potentially expensive iteration over the set of releases to return to the user. An alternative approach could avoid that extra iteration and fit this logic into the existing `filterList` function at the cost of making `filterList` function a little harder to understand. - Rename filterList to filterLatestReleases for clarity Another function that filters the list is added, so update to the more descriptive name here. - List superseded releases without filtering for latest This change makes superseded releases a special case, as they would _never_ be displayed otherwise (by definition, as superseded releases have been replaced by a newer release), so a conditional maintains current behavior ("return newest superseded revision for each release name") Fixes #7495. Signed-off-by: Andrew Melis <andrewmelis@gmail.com>
4 years ago
filteredList := filterLatestReleases([]*release.Release{r1, r2})
expectedFilteredList := []*release.Release{r1, r2}
assert.ElementsMatch(t, expectedFilteredList, filteredList)
})
}
func TestSelectorList(t *testing.T) {
r1 := releaseStub()
r1.Name = "r1"
r1.Version = 1
r1.Labels = map[string]string{"key": "value1"}
r2 := releaseStub()
r2.Name = "r2"
r2.Version = 1
r2.Labels = map[string]string{"key": "value2"}
r3 := releaseStub()
r3.Name = "r3"
r3.Version = 1
r3.Labels = map[string]string{}
lister := newListFixture(t)
for _, rel := range []*release.Release{r1, r2, r3} {
if err := lister.cfg.Releases.Create(rel); err != nil {
t.Fatal(err)
}
}
t.Run("should fail selector parsing", func(t *testing.T) {
is := assert.New(t)
lister.Selector = "a?=b"
_, err := lister.Run()
is.Error(err)
})
t.Run("should select one release with matching label", func(t *testing.T) {
lister.Selector = "key==value1"
res, _ := lister.Run()
expectedFilteredList := []*release.Release{r1}
assert.ElementsMatch(t, expectedFilteredList, res)
})
t.Run("should select two releases with non matching label", func(t *testing.T) {
lister.Selector = "key!=value1"
res, _ := lister.Run()
expectedFilteredList := []*release.Release{r2, r3}
assert.ElementsMatch(t, expectedFilteredList, res)
})
}