From 0e26f20268a2723fa7f82b44405b6a902335b59d Mon Sep 17 00:00:00 2001 From: fibonacci1729 Date: Mon, 8 Aug 2016 09:33:35 -0600 Subject: [PATCH] gofmt, style, and added comments --- pkg/storage/driver/driver.go | 29 ++++++++++++++------- pkg/storage/driver/memory.go | 43 ++++++++++++++++--------------- pkg/storage/driver/memory_test.go | 2 +- pkg/storage/storage.go | 38 +++++++++++++-------------- 4 files changed, 62 insertions(+), 50 deletions(-) diff --git a/pkg/storage/driver/driver.go b/pkg/storage/driver/driver.go index 0788fca60..fc833aaf6 100644 --- a/pkg/storage/driver/driver.go +++ b/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 diff --git a/pkg/storage/driver/memory.go b/pkg/storage/driver/memory.go index 88635c7cf..85918a68c 100644 --- a/pkg/storage/driver/memory.go +++ b/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() } diff --git a/pkg/storage/driver/memory_test.go b/pkg/storage/driver/memory_test.go index fe766634d..91b69a274 100644 --- a/pkg/storage/driver/memory_test.go +++ b/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}, }, } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 8996cdf6e..46f534ed0 100644 --- a/pkg/storage/storage.go +++ b/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()