From a5d96c27040218d09155d7c713df3e4586cae804 Mon Sep 17 00:00:00 2001
From: Matt Butcher <technosophos@gmail.com>
Date: Fri, 2 Dec 2016 17:14:07 -0700
Subject: [PATCH] fix(tiller): fix spurious "no release found" errors.

There are some places where releases are only located if they are in the
state DEPLOYED. That particular logic was incorrectly used for upgrades.
That caused #1566. While fixing that issue, I found that this was also
the root cause of #1587 (though because it was off by one). I added a
generic method to get the last release, regardless of its status.

This allows some behaviors that previously failed:

- 'helm upgrade' can now be performed on a DELETED release
- 'helm rollback' can now be performed on a DELETED release even if
  there is only one revision of that release history.

Closes #1566
Closes #1587
---
 pkg/storage/storage.go       | 13 +++++++++++++
 pkg/storage/storage_test.go  | 32 ++++++++++++++++++++++++++++++++
 pkg/tiller/release_server.go | 23 +++++------------------
 3 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 25ab6dae4..88fcd255f 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -139,6 +139,19 @@ func (s *Storage) History(name string) ([]*rspb.Release, error) {
 	return l, nil
 }
 
+func (s *Storage) Last(name string) (*rspb.Release, error) {
+	h, err := s.History(name)
+	if err != nil {
+		return nil, err
+	}
+	if len(h) == 0 {
+		return nil, fmt.Errorf("no revision for release %q", name)
+	}
+
+	relutil.Reverse(h, relutil.SortByRevision)
+	return h[0], nil
+}
+
 // makeKey concatenates a release name and version into
 // a string with format ```<release_name>#v<version>```.
 // This key is used to uniquely identify storage objects.
diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go
index 5935b4e88..141a019fa 100644
--- a/pkg/storage/storage_test.go
+++ b/pkg/storage/storage_test.go
@@ -217,6 +217,38 @@ func TestStorageHistory(t *testing.T) {
 	}
 }
 
+func TestStorageLast(t *testing.T) {
+	storage := Init(driver.NewMemory())
+
+	const name = "angry-bird"
+
+	// setup storage with test releases
+	setup := func() {
+		// release records
+		rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.Status_SUPERSEDED}.ToRelease()
+		rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.Status_SUPERSEDED}.ToRelease()
+		rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.Status_SUPERSEDED}.ToRelease()
+		rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.Status_FAILED}.ToRelease()
+
+		// create the release records in the storage
+		assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)")
+		assertErrNil(t.Fatal, storage.Create(rls1), "Storing release 'angry-bird' (v2)")
+		assertErrNil(t.Fatal, storage.Create(rls2), "Storing release 'angry-bird' (v3)")
+		assertErrNil(t.Fatal, storage.Create(rls3), "Storing release 'angry-bird' (v4)")
+	}
+
+	setup()
+
+	h, err := storage.Last(name)
+	if err != nil {
+		t.Fatalf("Failed to query for release history (%q): %s\n", name, err)
+	}
+
+	if h.Version != 4 {
+		t.Errorf("Expected revision 4, got %d", h.Version)
+	}
+}
+
 type ReleaseTestData struct {
 	Name      string
 	Version   int32
diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go
index abf8a30ff..64fc9da6f 100644
--- a/pkg/tiller/release_server.go
+++ b/pkg/tiller/release_server.go
@@ -229,17 +229,11 @@ func (s *ReleaseServer) GetReleaseStatus(c ctx.Context, req *services.GetRelease
 	var rel *release.Release
 
 	if req.Version <= 0 {
-		h, err := s.env.Releases.History(req.Name)
+		var err error
+		rel, err = s.env.Releases.Last(req.Name)
 		if err != nil {
-			return nil, fmt.Errorf("getting deployed release '%s': %s", req.Name, err)
-		}
-		if len(h) < 1 {
-			return nil, errMissingRelease
+			return nil, fmt.Errorf("getting deployed release %q: %s", req.Name, err)
 		}
-
-		relutil.Reverse(h, relutil.SortByRevision)
-		rel = h[0]
-
 	} else {
 		var err error
 		if rel, err = s.env.Releases.Get(req.Name, req.Version); err != nil {
@@ -376,7 +370,7 @@ func (s *ReleaseServer) prepareUpdate(req *services.UpdateReleaseRequest) (*rele
 	}
 
 	// finds the non-deleted release with the given name
-	currentRelease, err := s.env.Releases.Deployed(req.Name)
+	currentRelease, err := s.env.Releases.Last(req.Name)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -504,17 +498,10 @@ func (s *ReleaseServer) prepareRollback(req *services.RollbackReleaseRequest) (*
 		return nil, nil, errInvalidRevision
 	}
 
-	// finds the non-deleted release with the given name
-	h, err := s.env.Releases.History(req.Name)
+	crls, err := s.env.Releases.Last(req.Name)
 	if err != nil {
 		return nil, nil, err
 	}
-	if len(h) <= 1 {
-		return nil, nil, errors.New("no revision to rollback")
-	}
-
-	relutil.SortByRevision(h)
-	crls := h[len(h)-1]
 
 	rbv := req.Version
 	if req.Version == 0 {