Skip to content

Commit

Permalink
fix(tiller): Avoid corrupting storage via a lock
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
cristiklein committed Jan 8, 2020
1 parent e910a1c commit c32c9a5
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 0 deletions.
2 changes: 2 additions & 0 deletions pkg/tiller/release_server.go
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions pkg/tiller/release_update.go
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions pkg/tiller/release_update_test.go
Expand Up @@ -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()
Expand Down

0 comments on commit c32c9a5

Please sign in to comment.