fix(tiller): remove locking system from storage and rely on backend controls

Tiller currently hangs indefinitely when deadlocks arise from certain
concurrent operations. This commit removes the nested mutex locking
system from pkg/Storage and relies on resource contention controls in k8s.

Closes #2560

Author: jascott1

pkg/storage/driver/cfgmaps.go

@@ -173,7 +173,7 @@ func (cfgmaps *ConfigMaps) Create(key string, rls *rspb.Release) error {
 	// push the configmap object out into the kubiverse
 	if _, err := cfgmaps.impl.Create(obj); err != nil {
 		if apierrors.IsAlreadyExists(err) {
-			return ErrReleaseExists(rls.Name)
+			return ErrReleaseExists(key)
 		}
 
 		cfgmaps.Log("create: failed to create: %s", err)

pkg/storage/storage.go

@@ -18,7 +18,6 @@ package storage // import "k8s.io/helm/pkg/storage"
 
 import (
 	"fmt"
-	"sync"
 
 	rspb "k8s.io/helm/pkg/proto/hapi/release"
 	relutil "k8s.io/helm/pkg/releaseutil"
@@ -28,12 +27,6 @@ import (
 // Storage represents a storage engine for a Release.
 type Storage struct {
 	driver.Driver
-
-	// releaseLocks are for locking releases to make sure that only one operation at a time is executed on each release
-	releaseLocks map[string]*sync.Mutex
-	// releaseLocksLock is a mutex for accessing releaseLocks
-	releaseLocksLock *sync.Mutex
-
 	Log func(string, ...interface{})
 }
 
@@ -157,53 +150,6 @@ func (s *Storage) Last(name string) (*rspb.Release, error) {
 	return h[0], nil
 }
 
-// LockRelease gains a mutually exclusive access to a release via a mutex.
-func (s *Storage) LockRelease(name string) error {
-	s.Log("locking release %s", name)
-	s.releaseLocksLock.Lock()
-	defer s.releaseLocksLock.Unlock()
-
-	var lock *sync.Mutex
-	lock, exists := s.releaseLocks[name]
-
-	if !exists {
-		releases, err := s.ListReleases()
-		if err != nil {
-			return err
-		}
-
-		found := false
-		for _, release := range releases {
-			if release.Name == name {
-				found = true
-			}
-		}
-		if !found {
-			return fmt.Errorf("Unable to lock release %q: release not found", name)
-		}
-
-		lock = &sync.Mutex{}
-		s.releaseLocks[name] = lock
-	}
-	lock.Lock()
-	return nil
-}
-
-// UnlockRelease releases a mutually exclusive access to a release.
-// If release doesn't exist or wasn't previously locked - the unlock will pass
-func (s *Storage) UnlockRelease(name string) {
-	s.Log("unlocking release %s", name)
-	s.releaseLocksLock.Lock()
-	defer s.releaseLocksLock.Unlock()
-
-	var lock *sync.Mutex
-	lock, exists := s.releaseLocks[name]
-	if !exists {
-		return
-	}
-	lock.Unlock()
-}
-
 // 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.
@@ -219,9 +165,7 @@ func Init(d driver.Driver) *Storage {
 		d = driver.NewMemory()
 	}
 	return &Storage{
-		Driver:           d,
-		releaseLocks:     make(map[string]*sync.Mutex),
-		releaseLocksLock: &sync.Mutex{},
-		Log:              func(_ string, _ ...interface{}) {},
+		Driver: d,
+		Log:    func(_ string, _ ...interface{}) {},
 	}
 }

pkg/storage/storage_test.go

@@ -272,31 +272,3 @@ func assertErrNil(eh func(args ...interface{}), err error, message string) {
 		eh(fmt.Sprintf("%s: %q", message, err))
 	}
 }
-
-func TestReleaseLocksNotExist(t *testing.T) {
-	s := Init(driver.NewMemory())
-
-	err := s.LockRelease("no-such-release")
-
-	if err == nil {
-		t.Errorf("Exptected error when trying to lock non-existing release, got nil")
-	}
-}
-
-func TestReleaseLocks(t *testing.T) {
-	s := Init(driver.NewMemory())
-
-	releaseName := "angry-beaver"
-	rls := ReleaseTestData{
-		Name:    releaseName,
-		Version: 1,
-	}.ToRelease()
-
-	s.Create(rls)
-
-	err := s.LockRelease(releaseName)
-	if err != nil {
-		t.Errorf("Exptected nil err when locking existing release")
-	}
-	s.UnlockRelease(releaseName)
-}

pkg/tiller/release_rollback.go

@@ -29,12 +29,6 @@ import (
 
 // RollbackRelease rolls back to a previous version of the given release.
 func (s *ReleaseServer) RollbackRelease(c ctx.Context, req *services.RollbackReleaseRequest) (*services.RollbackReleaseResponse, error) {
-	err := s.env.Releases.LockRelease(req.Name)
-	if err != nil {
-		return nil, err
-	}
-	defer s.env.Releases.UnlockRelease(req.Name)
-
 	s.Log("preparing rollback of %s", req.Name)
 	currentRelease, targetRelease, err := s.prepareRollback(req)
 	if err != nil {

pkg/tiller/release_uninstall.go

@@ -40,12 +40,6 @@ func (s *ReleaseServer) UninstallRelease(c ctx.Context, req *services.UninstallR
 		return nil, fmt.Errorf("release name %q exceeds max length of %d", req.Name, releaseNameMaxLen)
 	}
 
-	err := s.env.Releases.LockRelease(req.Name)
-	if err != nil {
-		return nil, err
-	}
-	defer s.env.Releases.UnlockRelease(req.Name)
-
 	rels, err := s.env.Releases.History(req.Name)
 	if err != nil {
 		s.Log("uninstall: Release not loaded: %s", req.Name)

pkg/tiller/release_update.go

@@ -30,12 +30,6 @@ import (
 
 // UpdateRelease takes an existing release and new information, and upgrades the release.
 func (s *ReleaseServer) UpdateRelease(c ctx.Context, req *services.UpdateReleaseRequest) (*services.UpdateReleaseResponse, error) {
-	err := s.env.Releases.LockRelease(req.Name)
-	if err != nil {
-		return nil, err
-	}
-	defer s.env.Releases.UnlockRelease(req.Name)
-
 	s.Log("preparing update for %s", req.Name)
 	currentRelease, updatedRelease, err := s.prepareUpdate(req)
 	if err != nil {