From c32c9a510bce24278ff5c17cc8401e0ff5c32042 Mon Sep 17 00:00:00 2001
From: Cristian Klein <cristian.klein@elastisys.com>
Date: Thu, 9 Jan 2020 00:41:39 +0100
Subject: [PATCH] fix(tiller): Avoid corrupting storage via a lock

If two `helm upgrade`s are executed at the exact same time, then one of
the invocations will fail with "already exists".

If one `helm upgrade` is executed and a second one is started while the
first is in `pending-upgrade`, then the second invocation will create a
new release. Effectively, two helm invocations will simultaneously
change the state of Kubernetes resources -- which is scary -- then two
releases will be in `deployed` state -- which can cause other issues.

This commit fixes the corrupted storage problem, by introducting a poor
person's lock. If the last release is in a pending state, then helm will
abort. If the last release is in a pending state, due to a previously
killed helm, then the user is expected to do `helm rollback`.

This is a port to Helm v2 of #7322.

Signed-off-by: Cristian Klein <cristian.klein@elastisys.com>
---
 pkg/tiller/release_server.go      |  2 ++
 pkg/tiller/release_update.go      |  8 ++++++++
 pkg/tiller/release_update_test.go | 25 +++++++++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go
index 68169fe89..900a52365 100644
--- a/pkg/tiller/release_server.go
+++ b/pkg/tiller/release_server.go
@@ -65,6 +65,8 @@ var (
 	errInvalidRevision = errors.New("invalid release revision")
 	//errInvalidName indicates that an invalid release name was provided
 	errInvalidName = errors.New("invalid release name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not be longer than 53")
+	// errPending indicates that Tiller is already applying an operation on a release
+	errPending = errors.New("another operation (install/upgrade/rollback) is in progress")
 )
 
 // ListDefaultLimit is the default limit for number of items returned in a list.
diff --git a/pkg/tiller/release_update.go b/pkg/tiller/release_update.go
index 5fb1552bf..c9ac87334 100644
--- a/pkg/tiller/release_update.go
+++ b/pkg/tiller/release_update.go
@@ -93,6 +93,14 @@ func (s *ReleaseServer) prepareUpdate(req *services.UpdateReleaseRequest) (*rele
 		return nil, nil, err
 	}
 
+	// Concurrent `helm upgrade`s will either fail here with `errPending` or
+	// when creating the release with "already exists". This should act as a
+	// pessimistic lock.
+	sc := lastRelease.Info.Status.Code
+	if sc == release.Status_PENDING_INSTALL || sc == release.Status_PENDING_UPGRADE || sc == release.Status_PENDING_ROLLBACK {
+		return nil, nil, errPending
+	}
+
 	// Increment revision count. This is passed to templates, and also stored on
 	// the release object.
 	revision := lastRelease.Version + 1
diff --git a/pkg/tiller/release_update_test.go b/pkg/tiller/release_update_test.go
index a626295c2..0346ba180 100644
--- a/pkg/tiller/release_update_test.go
+++ b/pkg/tiller/release_update_test.go
@@ -104,6 +104,31 @@ func TestUpdateRelease(t *testing.T) {
 		t.Errorf("Expected description %q, got %q", edesc, got)
 	}
 }
+func TestUpdateReleasePendingError(t *testing.T) {
+	c := helm.NewContext()
+	rs := rsFixture()
+	rel := releaseStub()
+	rs.env.Releases.Create(rel)
+	rel2 := releaseStub()
+	rel2.Info.Status.Code = release.Status_PENDING_UPGRADE
+	rel2.Version = 2
+	rs.env.Releases.Create(rel2)
+
+	req := &services.UpdateReleaseRequest{
+		Name: rel.Name,
+		Chart: &chart.Chart{
+			Metadata: &chart.Metadata{Name: "hello"},
+			Templates: []*chart.Template{
+				{Name: "templates/hello", Data: []byte("hello: world")},
+				{Name: "templates/hooks", Data: []byte(manifestWithUpgradeHooks)},
+			},
+		},
+	}
+	_, err := rs.UpdateRelease(c, req)
+	if err == nil {
+		t.Fatalf("Expected failure to update")
+	}
+}
 func TestUpdateRelease_ResetValues(t *testing.T) {
 	c := helm.NewContext()
 	rs := rsFixture()