From 96ab5216e798daff8fe1c86c4c255fae49bc781f Mon Sep 17 00:00:00 2001 From: John Montroy Date: Fri, 3 Apr 2020 12:55:35 -0400 Subject: [PATCH] fix(storage): Deployed, DeployedAll, and History methods should return ErrReleaseNotFound if there are no releases, instead of custom error. Signed-off-by: John Montroy --- cmd/helm/rollback_test.go | 15 +++++++++------ pkg/storage/storage.go | 30 ++++++++++++++++-------------- pkg/storage/storage_test.go | 13 ++++++++++++- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/cmd/helm/rollback_test.go b/cmd/helm/rollback_test.go index fdc627b5f..263e5d575 100644 --- a/cmd/helm/rollback_test.go +++ b/cmd/helm/rollback_test.go @@ -23,7 +23,7 @@ import ( "helm.sh/helm/v3/pkg/release" ) -func TestRollbackCmd(t *testing.T) { +func makeTestReleases() []*release.Release { rels := []*release.Release{ { Name: "funny-honey", @@ -38,32 +38,35 @@ func TestRollbackCmd(t *testing.T) { Version: 2, }, } + return rels +} +func TestRollbackCmd(t *testing.T) { tests := []cmdTestCase{{ name: "rollback a release", cmd: "rollback funny-honey 1", golden: "output/rollback.txt", - rels: rels, + rels: makeTestReleases(), }, { name: "rollback a release with timeout", cmd: "rollback funny-honey 1 --timeout 120s", golden: "output/rollback-timeout.txt", - rels: rels, + rels: makeTestReleases(), }, { name: "rollback a release with wait", cmd: "rollback funny-honey 1 --wait", golden: "output/rollback-wait.txt", - rels: rels, + rels: makeTestReleases(), }, { name: "rollback a release without revision", cmd: "rollback funny-honey", golden: "output/rollback-no-revision.txt", - rels: rels, + rels: makeTestReleases(), }, { name: "rollback a release without release name", cmd: "rollback", golden: "output/rollback-no-args.txt", - rels: rels, + rels: makeTestReleases(), wantError: true, }} runTestCmd(t, tests) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 89183355f..7f922240c 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -18,7 +18,6 @@ package storage // import "helm.sh/helm/v3/pkg/storage" import ( "fmt" - "strings" "github.com/pkg/errors" @@ -115,10 +114,6 @@ func (s *Storage) Deployed(name string) (*rspb.Release, error) { return nil, err } - if len(ls) == 0 { - return nil, errors.Errorf("%q has no deployed releases", name) - } - // If executed concurrently, Helm's database gets corrupted // and multiple releases are DEPLOYED. Take the latest. relutil.Reverse(ls, relutil.SortByRevision) @@ -136,13 +131,13 @@ func (s *Storage) DeployedAll(name string) ([]*rspb.Release, error) { "owner": "helm", "status": "deployed", }) - if err == nil { - return ls, nil + if err != nil { + return nil, err } - if strings.Contains(err.Error(), "not found") { - return nil, errors.Errorf("%q has no deployed releases", name) + if len(ls) == 0 { + return nil, driver.ErrReleaseNotFound } - return nil, err + return ls, nil } // History returns the revision history for the release with the provided name, or @@ -150,7 +145,17 @@ func (s *Storage) DeployedAll(name string) ([]*rspb.Release, error) { func (s *Storage) History(name string) ([]*rspb.Release, error) { s.Log("getting release history for %q", name) - return s.Driver.Query(map[string]string{"name": name, "owner": "helm"}) + ls, err := s.Driver.Query(map[string]string{ + "name": name, + "owner": "helm", + }) + if err != nil { + return nil, err + } + if len(ls) == 0 { + return nil, driver.ErrReleaseNotFound + } + return ls, nil } // removeLeastRecent removes items from history until the lengh number of releases @@ -205,9 +210,6 @@ func (s *Storage) Last(name string) (*rspb.Release, error) { if err != nil { return nil, err } - if len(h) == 0 { - return nil, errors.Errorf("no revision for release %q", name) - } relutil.Reverse(h, relutil.SortByRevision) return h[0], nil diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index ee9c68b80..292133ea7 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -188,7 +188,7 @@ func TestStorageDeployed(t *testing.T) { setup() - rls, err := storage.Last(name) + rls, err := storage.Deployed(name) if err != nil { t.Fatalf("Failed to query for deployed release: %s\n", err) } @@ -205,6 +205,17 @@ func TestStorageDeployed(t *testing.T) { } } +func TestStorageDeployedError(t *testing.T) { + storage := Init(driver.NewMemory()) + + const name = "angry-bird" + + _, err := storage.Deployed(name) + if err != driver.ErrReleaseNotFound { + t.Fatalf("Expected ErrReleaseNotFound, got: %v", err) + } +} + func TestStorageDeployedWithCorruption(t *testing.T) { storage := Init(driver.NewMemory())