gofmt, style, and added comments

pkg/storage/driver/driver.go

@@ -30,34 +30,45 @@ var (
 	ErrDriverAction = errors.New("driver: failed to perform action")
 )
 
-// Creator stores a release.
+// Creator is the interface that wraps the Create method.
+//
+// Create stores the release or returns ErrReleaseExists
+// if an identical release already exists.
 type Creator interface {
 	Create(rls *rspb.Release) error
 }
 
-// Updator updates an existing release or returns
+// Updator is the interface that wraps the Update method.
+//
+// Update updates an existing release or returns
 // ErrReleaseNotFound if the release does not exist.
 type Updator interface {
 	Update(rls *rspb.Release) error
 }
 
-// Deletor deletes the release named by key or returns
+// Deletor is the interface that wraps the Delete method.
+//
+// Delete deletes the release named by key or returns
 // ErrReleaseNotFound if the release does not exist.
 type Deletor interface {
 	Delete(key string) (*rspb.Release, error)
 }
 
-// Queryor defines the behavior on accessing a release from storage.
+// Queryor is the interface that wraps the Get and List methods.
+//
+// Get returns the release named by key or returns ErrReleaseNotFound
+// if the release does not exist.
+//
+// List returns the set of all releases that satisfy the filter predicate.
 type Queryor interface {
-	// Get returns the release named by key or returns ErrReleaseNotFound
-	// if the release does not exist.
 	Get(key string) (*rspb.Release, error)
-	// List returns the set of all releases that satisfy the filter predicate.
 	List(filter func(*rspb.Release) bool) ([]*rspb.Release, error)
 }
 
-// Driver defines the behavior for storing, updating, deleted, and retrieving
-// tiller releases from some underlying storage mechanism, e.g. memory, configmaps.
+// Driver is the interface composed of Creator, Updator, Deletor, Queryor
+// interfaces. It defines the behavior for storing, updating, deleted,
+// and retrieving tiller releases from some underlying storage mechanism,
+// e.g. memory, configmaps.
 type Driver interface {
 	Creator
 	Updator

pkg/storage/driver/memory.go

@@ -22,28 +22,18 @@ import (
 	rspb "k8s.io/helm/pkg/proto/hapi/release"
 )
 
-// Memory is an in-memory storage driver implementation.
+// Memory is the in-memory storage driver implementation.
 type Memory struct {
 	sync.RWMutex
 	cache map[string]*rspb.Release
 }
 
+// NewMemory initializes a new memory driver.
 func NewMemory() *Memory {
 	return &Memory{cache: map[string]*rspb.Release{}}
 }
 
-// Create creates a new release or error.
-func (mem *Memory) Create(rls *rspb.Release) error {
-	defer unlock(mem.wlock())
-
-	if _, ok := mem.cache[rls.Name]; ok {
-		return ErrReleaseExists
-	}
-	mem.cache[rls.Name] = rls
-	return nil
-}
-
-// Get returns the release named by key.
+// Get returns the release named by key or returns ErrReleaseNotFound.
 func (mem *Memory) Get(key string) (*rspb.Release, error) {
 	defer unlock(mem.rlock())
 
@@ -66,33 +56,40 @@ func (mem *Memory) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error
 	return releases, nil
 }
 
-// Update updates a release or error.
-func (mem *Memory) Update(rls *rspb.Release) error {
+// Create creates a new release or returns ErrReleaseExists.
+func (mem *Memory) Create(rls *rspb.Release) error {
 	defer unlock(mem.wlock())
 
-	if old, ok := mem.cache[rls.Name]; ok {
-		// FIXME: when release update is complete, old release should
-		// be marked as superseded, creating the new release.
-		_ = old
+	if _, ok := mem.cache[rls.Name]; ok {
+		return ErrReleaseExists
+	}
+	mem.cache[rls.Name] = rls
+	return nil
+}
 
+// Update updates a release or returns ErrReleaseNotFound.
+func (mem *Memory) Update(rls *rspb.Release) error {
+	defer unlock(mem.wlock())
+
+	if _, ok := mem.cache[rls.Name]; ok {
 		mem.cache[rls.Name] = rls
 		return nil
 	}
 	return ErrReleaseNotFound
 }
 
-// Delete deletes a release or error.
+// Delete deletes a release or returns ErrReleaseNotFound.
 func (mem *Memory) Delete(key string) (*rspb.Release, error) {
 	defer unlock(mem.wlock())
 
 	if old, ok := mem.cache[key]; ok {
-		old.Info.Status.Code = rspb.Status_DELETED
 		delete(mem.cache, key)
 		return old, nil
 	}
 	return nil, ErrReleaseNotFound
 }
 
+// wlock locks mem for writing
 func (mem *Memory) wlock() func() {
 	mem.Lock()
 	return func() {
@@ -100,6 +97,7 @@ func (mem *Memory) wlock() func() {
 	}
 }
 
+// rlock locks mem for reading
 func (mem *Memory) rlock() func() {
 	mem.RLock()
 	return func() {
@@ -107,4 +105,7 @@ func (mem *Memory) rlock() func() {
 	}
 }
 
+// unlock calls fn which reverses a mem.rlock or mem.wlock. e.g:
+// ```defer unlock(mem.rlock())```, locks mem for reading at the
+// call point of defer and unlocks upon exiting the block.
 func unlock(fn func()) { fn() }

pkg/storage/driver/memory_test.go

@@ -73,7 +73,7 @@ func TestMemoryDelete(t *testing.T) {
 	rls := &rspb.Release{
 		Name: key,
 		Info: &rspb.Info{
-			Status: &rspb.Status{Code: rspb.Status_DEPLOYED},
+			Status: &rspb.Status{Code: rspb.Status_DELETED},
 		},
 	}
 

pkg/storage/storage.go

@@ -22,15 +22,24 @@ import (
 	"log"
 )
 
+// Storage represents a storage engine for a Release.
 type Storage struct {
 	driver.Driver
 }
 
+// Get retrieves the release from storage. An error is returned
+// if the storage driver failed to fetch the release, or the
+// release identified by key does not exist.
+func (s *Storage) Get(key string) (*rspb.Release, error) {
+	log.Printf("Getting release %q from storage\n", key)
+	return s.Driver.Get(key)
+}
+
 // Create creates a new storage entry holding the release. An
 // error is returned if the storage driver failed to store the
 // release, or a release with identical an key already exists.
 func (s *Storage) Create(rls *rspb.Release) error {
-	log.Printf("storage: create: %q\n", rls.Name)
+	log.Printf("Create release %q in storage\n", rls.Name)
 	return s.Driver.Create(rls)
 }
 
@@ -38,17 +47,7 @@ func (s *Storage) Create(rls *rspb.Release) error {
 // storage backend fails to update the release or if the release
 // does not exist.
 func (s *Storage) Update(rls *rspb.Release) error {
-	// TODO:
-	// 		1. Fetch most recent deployed release.
-	//     	2. If it exists:
-	// 			- Then mark as 'SUPERSEDED'
-	//			- Then store both new and old.
-	// 	   	3. Else:
-	//			- Create(rls)
-	log.Printf("storage: update: %q\n", rls.Name)
-
-	// fetch most recent deployed release
-	// old, err := s.Get(rls.Name)
+	log.Printf("Updating %q in storage\n", rls.Name)
 	return s.Driver.Update(rls)
 }
 
@@ -56,22 +55,21 @@ func (s *Storage) Update(rls *rspb.Release) error {
 // the storage backend fails to delete the release or if the release
 // does not exist.
 func (s *Storage) Delete(key string) (*rspb.Release, error) {
-	// TODO: Mark the release as DELETED.
-	log.Printf("storage: delete: %q\n", key)
+	log.Printf("Deleting release %q from storage\n", key)
 	return s.Driver.Delete(key)
 }
 
 // ListReleases returns all releases from storage. An error is returned if the
 // storage backend fails to retrieve the releases.
 func (s *Storage) ListReleases() ([]*rspb.Release, error) {
-	log.Println("storage: list all releases")
+	log.Println("Listing all releases in storage")
 	return s.Driver.List(func(_ *rspb.Release) bool { return true })
 }
 
 // ListDeleted returns all releases with Status == DELETED. An error is returned
 // if the storage backend fails to retrieve the releases.
 func (s *Storage) ListDeleted() ([]*rspb.Release, error) {
-	log.Println("storage: list deleted releases")
+	log.Println("List deleted releases in storage")
 	return s.Driver.List(func(rls *rspb.Release) bool {
 		return StatusFilter(rspb.Status_DELETED).Check(rls)
 	})
@@ -80,7 +78,7 @@ func (s *Storage) ListDeleted() ([]*rspb.Release, error) {
 // ListDeployed returns all releases with Status == DEPLOYED. An error is returned
 // if the storage backend fails to retrieve the releases.
 func (s *Storage) ListDeployed() ([]*rspb.Release, error) {
-	log.Println("storage: list deployed releases")
+	log.Println("Listing all deployed releases in storage")
 	return s.Driver.List(func(rls *rspb.Release) bool {
 		return StatusFilter(rspb.Status_DEPLOYED).Check(rls)
 	})
@@ -90,7 +88,7 @@ func (s *Storage) ListDeployed() ([]*rspb.Release, error) {
 // (filter0 && filter1 && ... && filterN), i.e. a Release is included in the results
 // if and only if all filters return true.
 func (s *Storage) ListFilterAll(filters ...FilterFunc) ([]*rspb.Release, error) {
-	log.Println("storage: list all releases with filter")
+	log.Println("Listing all releases with filter")
 	return s.Driver.List(func(rls *rspb.Release) bool {
 		return All(filters...).Check(rls)
 	})
@@ -100,12 +98,14 @@ func (s *Storage) ListFilterAll(filters ...FilterFunc) ([]*rspb.Release, error)
 // (filter0 || filter1 || ... || filterN), i.e. a Release is included in the results
 // if at least one of the filters returns true.
 func (s *Storage) ListFilterAny(filters ...FilterFunc) ([]*rspb.Release, error) {
-	log.Println("storage: list any releases with filter")
+	log.Println("Listing any releases with filter")
 	return s.Driver.List(func(rls *rspb.Release) bool {
 		return Any(filters...).Check(rls)
 	})
 }
 
+// Init initializes a new storage backend with the driver d.
+// If d is nil, the default in-memory driver is used.
 func Init(d driver.Driver) *Storage {
 	if d == nil {
 		d = driver.NewMemory()