From 57b2e6429e9888f84977e0fa4aebc307f57cbd4a Mon Sep 17 00:00:00 2001
From: Michelle Noorali <michelle@deis.com>
Date: Tue, 11 Oct 2016 12:30:54 -0400
Subject: [PATCH] bug(tiller): set status correctly in performUpdate

Handling release status updates on errors better
resolves #1137
---
 cmd/tiller/release_server.go      | 15 +++++----
 cmd/tiller/release_server_test.go | 51 +++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/cmd/tiller/release_server.go b/cmd/tiller/release_server.go
index 850fedd72..5f05a5783 100644
--- a/cmd/tiller/release_server.go
+++ b/cmd/tiller/release_server.go
@@ -278,12 +278,12 @@ func (s *releaseServer) UpdateRelease(c ctx.Context, req *services.UpdateRelease
 
 	res, err := s.performUpdate(currentRelease, updatedRelease, req)
 	if err != nil {
-		return nil, err
+		return res, err
 	}
 
 	if !req.DryRun {
 		if err := s.env.Releases.Create(updatedRelease); err != nil {
-			return nil, err
+			return res, err
 		}
 	}
 
@@ -306,7 +306,12 @@ func (s *releaseServer) performUpdate(originalRelease, updatedRelease *release.R
 	}
 
 	if err := s.performKubeUpdate(originalRelease, updatedRelease); err != nil {
-		return nil, err
+		log.Printf("warning: Release Upgrade %q failed: %s", updatedRelease.Name, err)
+		originalRelease.Info.Status.Code = release.Status_SUPERSEDED
+		updatedRelease.Info.Status.Code = release.Status_FAILED
+		s.recordRelease(originalRelease, true)
+		s.recordRelease(updatedRelease, false)
+		return res, err
 	}
 
 	// post-upgrade hooks
@@ -317,9 +322,7 @@ func (s *releaseServer) performUpdate(originalRelease, updatedRelease *release.R
 	}
 
 	originalRelease.Info.Status.Code = release.Status_SUPERSEDED
-	if err := s.env.Releases.Update(originalRelease); err != nil {
-		return nil, fmt.Errorf("Update of %s failed: %s", originalRelease.Name, err)
-	}
+	s.recordRelease(originalRelease, true)
 
 	updatedRelease.Info.Status.Code = release.Status_DEPLOYED
 
diff --git a/cmd/tiller/release_server_test.go b/cmd/tiller/release_server_test.go
index d63a52e42..72b054c20 100644
--- a/cmd/tiller/release_server_test.go
+++ b/cmd/tiller/release_server_test.go
@@ -587,6 +587,42 @@ func TestUpdateRelease(t *testing.T) {
 	}
 }
 
+func TestUpdateReleaseFailure(t *testing.T) {
+	c := helm.NewContext()
+	rs := rsFixture()
+	rel := releaseStub()
+	rs.env.Releases.Create(rel)
+	rs.env.KubeClient = newUpdateFailingKubeClient()
+
+	req := &services.UpdateReleaseRequest{
+		Name:         rel.Name,
+		DisableHooks: true,
+		Chart: &chart.Chart{
+			Metadata: &chart.Metadata{Name: "hello"},
+			Templates: []*chart.Template{
+				{Name: "something", Data: []byte("hello: world")},
+			},
+		},
+	}
+
+	res, err := rs.UpdateRelease(c, req)
+	if err == nil {
+		t.Error("Expected failed update")
+	}
+
+	if updatedStatus := res.Release.Info.Status.Code; updatedStatus != release.Status_FAILED {
+		t.Errorf("Expected FAILED release. Got %d", updatedStatus)
+	}
+
+	oldRelease, err := rs.env.Releases.Get(rel.Name, rel.Version)
+	if err != nil {
+		t.Errorf("Expected to be able to get previous release")
+	}
+	if oldStatus := oldRelease.Info.Status.Code; oldStatus != release.Status_SUPERSEDED {
+		t.Errorf("Expected SUPERSEDED status on previous Release version. Got %v", oldStatus)
+	}
+}
+
 func TestUpdateReleaseNoHooks(t *testing.T) {
 	c := helm.NewContext()
 	rs := rsFixture()
@@ -1136,6 +1172,21 @@ func mockEnvironment() *environment.Environment {
 	return e
 }
 
+func newUpdateFailingKubeClient() *updateFailingKubeClient {
+	return &updateFailingKubeClient{
+		PrintingKubeClient: environment.PrintingKubeClient{Out: os.Stdout},
+	}
+
+}
+
+type updateFailingKubeClient struct {
+	environment.PrintingKubeClient
+}
+
+func (u *updateFailingKubeClient) Update(namespace string, originalReader, modifiedReader io.Reader) error {
+	return errors.New("Failed update in kube client")
+}
+
 func newHookFailingKubeClient() *hookFailingKubeClient {
 	return &hookFailingKubeClient{
 		PrintingKubeClient: environment.PrintingKubeClient{Out: os.Stdout},