From 5f66676300a134f467d7dc8cf714947eba04de12 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>
(cherry picked from commit c32c9a510bce24278ff5c17cc8401e0ff5c32042)
---
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()
--
GitLab