chore(pkg/storage): introduce key name convention for storage objects with updated tests

pkg/storage/driver/cfgmaps.go

@@ -114,7 +114,7 @@ func (cfgmaps *ConfigMaps) List(filter func(*rspb.Release) bool) ([]*rspb.Releas
 
 // Create creates a new ConfigMap holding the release. If the
 // ConfigMap already exists, ErrReleaseExists is returned.
-func (cfgmaps *ConfigMaps) Create(rls *rspb.Release) error {
+func (cfgmaps *ConfigMaps) Create(key string, rls *rspb.Release) error {
 	// set labels for configmaps object meta data
 	var lbs labels
 
@@ -122,9 +122,9 @@ func (cfgmaps *ConfigMaps) Create(rls *rspb.Release) error {
 	lbs.set("CREATED_AT", strconv.Itoa(int(time.Now().Unix())))
 
 	// create a new configmap to hold the release
-	obj, err := newConfigMapsObject(rls, lbs)
+	obj, err := newConfigMapsObject(key, rls, lbs)
 	if err != nil {
-		logerrf(err, "create: failed to encode release %q", rls.Name)
+		logerrf(err, "create: failed to encode release %q", key)
 		return err
 	}
 	// push the configmap object out into the kubiverse
@@ -141,7 +141,7 @@ func (cfgmaps *ConfigMaps) Create(rls *rspb.Release) error {
 
 // Update updates the ConfigMap holding the release. If not found
 // the ConfigMap is created to hold the release.
-func (cfgmaps *ConfigMaps) Update(rls *rspb.Release) error {
+func (cfgmaps *ConfigMaps) Update(key string, rls *rspb.Release) error {
 	// set labels for configmaps object meta data
 	var lbs labels
 
@@ -149,9 +149,9 @@ func (cfgmaps *ConfigMaps) Update(rls *rspb.Release) error {
 	lbs.set("MODIFIED_AT", strconv.Itoa(int(time.Now().Unix())))
 
 	// create a new configmap object to hold the release
-	obj, err := newConfigMapsObject(rls, lbs)
+	obj, err := newConfigMapsObject(key, rls, lbs)
 	if err != nil {
-		logerrf(err, "update: failed to encode release %q", rls.Name)
+		logerrf(err, "update: failed to encode release %q", key)
 		return err
 	}
 	// push the configmap object out into the kubiverse
@@ -194,7 +194,7 @@ func (cfgmaps *ConfigMaps) Delete(key string) (rls *rspb.Release, err error) {
 //    "OWNER"          - owner of the configmap, currently "TILLER".
 //    "NAME"           - name of the release.
 //
-func newConfigMapsObject(rls *rspb.Release, lbs labels) (*api.ConfigMap, error) {
+func newConfigMapsObject(objkey string, rls *rspb.Release, lbs labels) (*api.ConfigMap, error) {
 	const owner = "TILLER"
 
 	// encode the release
@@ -216,7 +216,7 @@ func newConfigMapsObject(rls *rspb.Release, lbs labels) (*api.ConfigMap, error)
 	// create and return configmap object
 	return &api.ConfigMap{
 		ObjectMeta: api.ObjectMeta{
-			Name:   rls.Name,
+			Name:   objkey,
 			Labels: lbs.toMap(),
 		},
 		Data: map[string]string{"release": s},

pkg/storage/driver/cfgmaps_test.go

@@ -37,8 +37,10 @@ func TestConfigMapName(t *testing.T) {
 }
 
 func TestConfigMapGet(t *testing.T) {
-	key := "key-1"
+	vers := int32(1)
-	rel := newTestRelease(key, 1, rspb.Status_DEPLOYED)
+	name := "smug-pigeon"
+	key := testKey(name, vers)
+	rel := newTestRelease(name, vers, rspb.Status_DEPLOYED)
 
 	cfgmaps := newTestFixture(t, []*rspb.Release{rel}...)
 
@@ -103,11 +105,13 @@ func TestConfigMapList(t *testing.T) {
 func TestConfigMapCreate(t *testing.T) {
 	cfgmaps := newTestFixture(t)
 
-	key := "key-1"
+	vers := int32(1)
-	rel := newTestRelease(key, 1, rspb.Status_DEPLOYED)
+	name := "smug-pigeon"
+	key := testKey(name, vers)
+	rel := newTestRelease(name, vers, rspb.Status_DEPLOYED)
 
 	// store the release in a configmap
-	if err := cfgmaps.Create(rel); err != nil {
+	if err := cfgmaps.Create(key, rel); err != nil {
 		t.Fatalf("Failed to create release with key %q: %s", key, err)
 	}
 
@@ -124,16 +128,18 @@ func TestConfigMapCreate(t *testing.T) {
 }
 
 func TestConfigMapUpdate(t *testing.T) {
-	key := "key-1"
+	vers := int32(1)
-	rel := newTestRelease(key, 1, rspb.Status_DEPLOYED)
+	name := "smug-pigeon"
+	key := testKey(name, vers)
+	rel := newTestRelease(name, vers, rspb.Status_DEPLOYED)
 
 	cfgmaps := newTestFixture(t, []*rspb.Release{rel}...)
 
-	// modify release status code & version
+	// modify release status code
-	rel = newTestRelease(key, 2, rspb.Status_SUPERSEDED)
+	rel.Info.Status.Code = rspb.Status_SUPERSEDED
 
 	// perform the update
-	if err := cfgmaps.Update(rel); err != nil {
+	if err := cfgmaps.Update(key, rel); err != nil {
 		t.Fatalf("Failed to update release: %s", err)
 	}
 
@@ -144,11 +150,8 @@ func TestConfigMapUpdate(t *testing.T) {
 	}
 
 	// check release has actually been updated by comparing modified fields
-	switch {
+	if rel.Info.Status.Code != got.Info.Status.Code {
-	case rel.Info.Status.Code != got.Info.Status.Code:
 		t.Errorf("Expected status %s, got status %s", rel.Info.Status.Code, got.Info.Status.Code)
-	case rel.Version != got.Version:
-		t.Errorf("Expected version %d, got version %d", rel.Version, got.Version)
 	}
 }
 
@@ -177,11 +180,13 @@ func (mock *MockConfigMapsInterface) Init(t *testing.T, releases ...*rspb.Releas
 	mock.objects = map[string]*api.ConfigMap{}
 
 	for _, rls := range releases {
-		cfgmap, err := newConfigMapsObject(rls, nil)
+		objkey := testKey(rls.Name, rls.Version)
+
+		cfgmap, err := newConfigMapsObject(objkey, rls, nil)
 		if err != nil {
 			t.Fatalf("Failed to create configmap: %s", err)
 		}
-		mock.objects[rls.Name] = cfgmap
+		mock.objects[objkey] = cfgmap
 	}
 }
 

pkg/storage/driver/driver.go

@@ -34,7 +34,7 @@ var (
 // Create stores the release or returns ErrReleaseExists
 // if an identical release already exists.
 type Creator interface {
-	Create(rls *rspb.Release) error
+	Create(key string, rls *rspb.Release) error
 }
 
 // Updator is the interface that wraps the Update method.
@@ -42,7 +42,7 @@ type Creator interface {
 // Update updates an existing release or returns
 // ErrReleaseNotFound if the release does not exist.
 type Updator interface {
-	Update(rls *rspb.Release) error
+	Update(key string, rls *rspb.Release) error
 }
 
 // Deletor is the interface that wraps the Delete method.

pkg/storage/driver/memory.go

@@ -65,22 +65,22 @@ func (mem *Memory) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error
 }
 
 // Create creates a new release or returns ErrReleaseExists.
-func (mem *Memory) Create(rls *rspb.Release) error {
+func (mem *Memory) Create(key string, rls *rspb.Release) error {
 	defer unlock(mem.wlock())
 
-	if _, ok := mem.cache[rls.Name]; ok {
+	if _, ok := mem.cache[key]; ok {
 		return ErrReleaseExists
 	}
-	mem.cache[rls.Name] = rls
+	mem.cache[key] = rls
 	return nil
 }
 
 // Update updates a release or returns ErrReleaseNotFound.
-func (mem *Memory) Update(rls *rspb.Release) error {
+func (mem *Memory) Update(key string, rls *rspb.Release) error {
 	defer unlock(mem.wlock())
 
-	if _, ok := mem.cache[rls.Name]; ok {
+	if _, ok := mem.cache[key]; ok {
-		mem.cache[rls.Name] = rls
+		mem.cache[key] = rls
 		return nil
 	}
 	return ErrReleaseNotFound

pkg/storage/driver/memory_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package driver // import "k8s.io/helm/pkg/storage/driver"
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -33,11 +34,14 @@ func TestMemoryName(t *testing.T) {
 }
 
 func TestMemoryGet(t *testing.T) {
-	key := "test-1"
+	vers := int32(1)
-	rls := &rspb.Release{Name: key}
+	name := "smug-pigeon"
+	key := testKey(name, vers)
+
+	rls := &rspb.Release{Name: name, Version: vers}
 
 	mem := NewMemory()
-	if err := mem.Create(rls); err != nil {
+	if err := mem.Create(key, rls); err != nil {
 		t.Fatalf("Failed create: %s", err)
 	}
 
@@ -45,46 +49,76 @@ func TestMemoryGet(t *testing.T) {
 	if err != nil {
 		t.Errorf("Could not get %s: %s", key, err)
 	}
-	if res.Name != key {
+	if res.Name != name {
-		t.Errorf("Expected %s, got %s", key, res.Name)
+		t.Errorf("Expected name %s, got name %s", key, res.Name)
+	}
+	if res.Version != vers {
+		t.Errorf("Expected version %d, got version %d", vers, res.Version)
 	}
 }
 
 func TestMemoryCreate(t *testing.T) {
-	key := "test-1"
+	vers := int32(1)
-	rls := &rspb.Release{Name: key}
+	name := "smug-pigeon"
+	key := testKey(name, vers)
+
+	rls := &rspb.Release{Name: name, Version: vers}
 
 	mem := NewMemory()
-	if err := mem.Create(rls); err != nil {
+	if err := mem.Create(key, rls); err != nil {
 		t.Fatalf("Failed created: %s", err)
 	}
-	if mem.cache[key].Name != key {
+	if mem.cache[key].Name != name {
 		t.Errorf("Unexpected release name: %s", mem.cache[key].Name)
 	}
+	if mem.cache[key].Version != vers {
+		t.Errorf("Unexpected release version: %d", mem.cache[key].Version)
+	}
 }
 
 func TestMemoryUpdate(t *testing.T) {
-	key := "test-1"
+	vers := int32(1)
-	rls := &rspb.Release{Name: key}
+	name := "smug-pigeon"
+	key := testKey(name, vers)
+
+	rls := &rspb.Release{
+		Name:    name,
+		Version: vers,
+		Info:    &rspb.Info{Status: &rspb.Status{Code: rspb.Status_DEPLOYED}},
+	}
 
 	mem := NewMemory()
-	if err := mem.Create(rls); err != nil {
+	if err := mem.Create(key, rls); err != nil {
 		t.Fatalf("Failed create: %s", err)
 	}
-	if err := mem.Update(rls); err != nil {
+
+	// make an update to the release
+	rls.Info.Status.Code = rspb.Status_DELETED
+
+	if err := mem.Update(key, rls); err != nil {
 		t.Fatalf("Failed update: %s", err)
 	}
-	if mem.cache[key].Name != key {
+
+	if mem.cache[key].Name != name {
 		t.Errorf("Unexpected release name: %s", mem.cache[key].Name)
 	}
+	if mem.cache[key].Version != vers {
+		t.Errorf("Unexpected release version: %d", mem.cache[key].Version)
+	}
+	if mem.cache[key].Info.Status.Code != rspb.Status_DELETED {
+		t.Errorf("Unexpected status code: %s", mem.cache[key].Info.Status.Code)
+	}
 }
 
 func TestMemoryDelete(t *testing.T) {
-	key := "test-1"
+	vers := int32(1)
-	rls := &rspb.Release{Name: key}
+	name := "smug-pigeon"
+	key := testKey(name, vers)
+
+	rls := &rspb.Release{Name: key, Version: vers}
 
 	mem := NewMemory()
-	if err := mem.Create(rls); err != nil {
+	if err := mem.Create(key, rls); err != nil {
 		t.Fatalf("Failed create: %s", err)
 	}
 
@@ -99,3 +133,7 @@ func TestMemoryDelete(t *testing.T) {
 		t.Errorf("Expected %s, got %s", rls, res)
 	}
 }
+
+func testKey(name string, version int32) string {
+	return fmt.Sprintf("%s#v%d", name, version)
+}

pkg/storage/storage.go

@@ -17,6 +17,7 @@ limitations under the License.
 package storage // import "k8s.io/helm/pkg/storage"
 
 import (
+	"fmt"
 	"log"
 
 	rspb "k8s.io/helm/pkg/proto/hapi/release"
@@ -30,34 +31,34 @@ type Storage struct {
 
 // 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.
+// release identified by the key, version pair does not exist.
-func (s *Storage) Get(key string) (*rspb.Release, error) {
+func (s *Storage) Get(name string, version int32) (*rspb.Release, error) {
-	log.Printf("Getting release %q from storage\n", key)
+	log.Printf("Getting release %q (v%d) from storage\n", name, version)
-	return s.Driver.Get(key)
+	return s.Driver.Get(makeKey(name, version))
 }
 
 // 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("Create release %q in storage\n", rls.Name)
+	log.Printf("Create release %q (v%d) in storage\n", rls.Name, rls.Version)
-	return s.Driver.Create(rls)
+	return s.Driver.Create(makeKey(rls.Name, rls.Version), rls)
 }
 
 // Update update the release in storage. An error is returned if the
 // storage backend fails to update the release or if the release
 // does not exist.
 func (s *Storage) Update(rls *rspb.Release) error {
-	log.Printf("Updating %q in storage\n", rls.Name)
+	log.Printf("Updating %q (v%d) in storage\n", rls.Name, rls.Version)
-	return s.Driver.Update(rls)
+	return s.Driver.Update(makeKey(rls.Name, rls.Version), rls)
 }
 
 // Delete deletes the release from storage. An error is returned if
 // the storage backend fails to delete the release or if the release
 // does not exist.
-func (s *Storage) Delete(key string) (*rspb.Release, error) {
+func (s *Storage) Delete(name string, version int32) (*rspb.Release, error) {
-	log.Printf("Deleting release %q from storage\n", key)
+	log.Printf("Deleting release %q (v%d) from storage\n", name, version)
-	return s.Driver.Delete(key)
+	return s.Driver.Delete(makeKey(name, version))
 }
 
 // ListReleases returns all releases from storage. An error is returned if the
@@ -114,3 +115,10 @@ func Init(d driver.Driver) *Storage {
 	}
 	return &Storage{Driver: d}
 }
+
+// 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.
+func makeKey(rlsname string, version int32) string {
+	return fmt.Sprintf("%s#v%d", rlsname, version)
+}

pkg/storage/storage_test.go

@@ -30,11 +30,12 @@ func TestStorageCreate(t *testing.T) {
 	storage := Init(driver.NewMemory())
 
 	// create fake release
-	rls := ReleaseTestData{Name: "angry-beaver"}.ToRelease()
+	rls := ReleaseTestData{Name: "angry-beaver", Version: 1}.ToRelease()
+
 	assertErrNil(t.Fatal, storage.Create(rls), "StoreRelease")
 
 	// fetch the release
-	res, err := storage.Get(rls.Name)
+	res, err := storage.Get(rls.Name, rls.Version)
 	assertErrNil(t.Fatal, err, "QueryRelease")
 
 	// verify the fetched and created release are the same
@@ -48,16 +49,16 @@ func TestStorageUpdate(t *testing.T) {
 	storage := Init(driver.NewMemory())
 
 	// create fake release
-	rls := ReleaseTestData{Name: "angry-beaver"}.ToRelease()
+	rls := ReleaseTestData{Name: "angry-beaver", Version: 1}.ToRelease()
+
 	assertErrNil(t.Fatal, storage.Create(rls), "StoreRelease")
 
 	// modify the release
-	rls.Version = 2
 	rls.Manifest = "new-manifest"
 	assertErrNil(t.Fatal, storage.Update(rls), "UpdateRelease")
 
 	// retrieve the updated release
-	res, err := storage.Get(rls.Name)
+	res, err := storage.Get(rls.Name, rls.Version)
 	assertErrNil(t.Fatal, err, "QueryRelease")
 
 	// verify updated and fetched releases are the same.
@@ -71,11 +72,11 @@ func TestStorageDelete(t *testing.T) {
 	storage := Init(driver.NewMemory())
 
 	// create fake release
-	rls := ReleaseTestData{Name: "angry-beaver"}.ToRelease()
+	rls := ReleaseTestData{Name: "angry-beaver", Version: 1}.ToRelease()
 	assertErrNil(t.Fatal, storage.Create(rls), "StoreRelease")
 
 	// delete the release
-	res, err := storage.Delete(rls.Name)
+	res, err := storage.Delete(rls.Name, rls.Version)
 	assertErrNil(t.Fatal, err, "DeleteRelease")
 
 	// verify updated and fetched releases are the same.