Merge pull request #24 from technosophos/feat/storage-crud

fix(storage): Use a CRUD interface

cmd/tiller/environment/environment.go

@@ -58,15 +58,46 @@ type Engine interface {
 //
 // Release storage must be concurrency safe.
 type ReleaseStorage interface {
-	// Get takes a name and returns the accompanying release.
-	Get(key string) (*hapi.Release, error)
-	// Set saves the release with the given name.
-	Set(key string, val *hapi.Release) error
+
+	// Create stores a release in the storage.
+	//
+	// If a release with the same name exists, this returns an error.
+	//
+	// It may return other errors in cases where it cannot write to storage.
+	Create(*hapi.Release) error
+	// Read takes a name and returns a release that has that name.
+	//
+	// It will only return releases that are not deleted and not superseded.
+	//
+	// It will return an error if no relevant release can be found, or if storage
+	// is not properly functioning.
+	Read(name string) (*hapi.Release, error)
+
+	// Update looks for a release with the same name and updates it with the
+	// present release contents.
+	//
+	// For immutable storage backends, this may result in a new release record
+	// being created, and the previous release being marked as superseded.
+	//
+	// It will return an error if a previous release is not found. It may also
+	// return an error if the storage backend encounters an error.
+	Update(*hapi.Release) error
+
+	// Delete marks a Release as deleted.
+	//
+	// It returns the deleted record. If the record is not found or if the
+	// underlying storage encounters an error, this will return an error.
+	Delete(name string) (*hapi.Release, error)
+
 	// List lists all active (non-deleted, non-superseded) releases.
 	//
 	// To get deleted or superseded releases, use Query.
 	List() ([]*hapi.Release, error)
+
 	// Query takes a map of labels and returns any releases that match.
+	//
+	// Query will search all releases, including deleted and superseded ones.
+	// The provided map will be used to filter results.
 	Query(map[string]string) ([]*hapi.Release, error)
 }
 

cmd/tiller/environment/environment_test.go

@@ -18,15 +18,24 @@ type mockReleaseStorage struct {
 	rel *hapi.Release
 }
 
-func (r *mockReleaseStorage) Get(k string) (*hapi.Release, error) {
+func (r *mockReleaseStorage) Create(v *hapi.Release) error {
+	r.rel = v
+	return nil
+}
+
+func (r *mockReleaseStorage) Read(k string) (*hapi.Release, error) {
 	return r.rel, nil
 }
 
-func (r *mockReleaseStorage) Set(k string, v *hapi.Release) error {
+func (r *mockReleaseStorage) Update(v *hapi.Release) error {
 	r.rel = v
 	return nil
 }
 
+func (r *mockReleaseStorage) Delete(k string) (*hapi.Release, error) {
+	return r.rel, nil
+}
+
 func (r *mockReleaseStorage) List() ([]*hapi.Release, error) {
 	return []*hapi.Release{}, nil
 }
@@ -68,15 +77,23 @@ func TestReleaseStorage(t *testing.T) {
 
 	release := &hapi.Release{Name: "mariner"}
 
-	if err := env.Releases.Set("albatross", release); err != nil {
+	if err := env.Releases.Create(release); err != nil {
 		t.Fatalf("failed to store release: %s", err)
 	}
 
-	if v, err := env.Releases.Get("albatross"); err != nil {
+	if err := env.Releases.Update(release); err != nil {
+		t.Fatalf("failed to update release: %s", err)
+	}
+
+	if v, err := env.Releases.Read("albatross"); err != nil {
 		t.Errorf("Error fetching release: %s", err)
 	} else if v.Name != "mariner" {
 		t.Errorf("Expected mariner, got %q", v.Name)
 	}
+
+	if _, err := env.Releases.Delete("albatross"); err != nil {
+		t.Fatalf("failed to delete release: %s", err)
+	}
 }
 
 func TestKubeClient(t *testing.T) {

pkg/storage/memory.go

@@ -19,10 +19,10 @@ func NewMemory() *Memory {
 
 var ErrNotFound = errors.New("release not found")
 
-// Get returns the named Release.
+// Read returns the named Release.
 //
 // If the release is not found, an ErrNotFound error is returned.
-func (m *Memory) Get(k string) (*hapi.Release, error) {
+func (m *Memory) Read(k string) (*hapi.Release, error) {
 	v, ok := m.releases[k]
 	if !ok {
 		return v, ErrNotFound
@@ -30,14 +30,35 @@ func (m *Memory) Get(k string) (*hapi.Release, error) {
 	return v, nil
 }
 
-// Set sets a release.
-//
-// TODO: Is there any reason why Set doesn't just use the release name?
-func (m *Memory) Set(k string, rel *hapi.Release) error {
-	m.releases[k] = rel
+// Create sets a release.
+func (m *Memory) Create(rel *hapi.Release) error {
+	m.releases[rel.Name] = rel
 	return nil
 }
 
+var ErrNoRelease = errors.New("no release found")
+
+// Update sets a release.
+func (m *Memory) Update(rel *hapi.Release) error {
+	if _, ok := m.releases[rel.Name]; !ok {
+		return ErrNoRelease
+	}
+
+	// FIXME: When Release is done, we need to do this right by marking the old
+	// release as superseded, and creating a new release.
+	m.releases[rel.Name] = rel
+	return nil
+}
+
+func (m *Memory) Delete(name string) (*hapi.Release, error) {
+	rel, ok := m.releases[name]
+	if !ok {
+		return nil, ErrNoRelease
+	}
+	delete(m.releases, name)
+	return rel, nil
+}
+
 // List returns all releases
 func (m *Memory) List() ([]*hapi.Release, error) {
 	buf := make([]*hapi.Release, len(m.releases))

pkg/storage/memory_test.go

@@ -6,13 +6,13 @@ import (
 	"github.com/deis/tiller/pkg/hapi"
 )
 
-func TestSet(t *testing.T) {
+func TestCreate(t *testing.T) {
 	k := "test-1"
 	r := &hapi.Release{Name: k}
 
 	ms := NewMemory()
-	if err := ms.Set(k, r); err != nil {
-		t.Fatalf("Failed set: %s", err)
+	if err := ms.Create(r); err != nil {
+		t.Fatalf("Failed create: %s", err)
 	}
 
 	if ms.releases[k].Name != k {
@@ -20,26 +20,43 @@ func TestSet(t *testing.T) {
 	}
 }
 
-func TestGet(t *testing.T) {
+func TestRead(t *testing.T) {
 	k := "test-1"
 	r := &hapi.Release{Name: k}
 
 	ms := NewMemory()
-	ms.Set(k, r)
+	ms.Create(r)
 
-	if out, err := ms.Get(k); err != nil {
+	if out, err := ms.Read(k); err != nil {
 		t.Errorf("Could not get %s: %s", k, err)
 	} else if out.Name != k {
 		t.Errorf("Expected %s, got %s", k, out.Name)
 	}
 }
 
+func TestUpdate(t *testing.T) {
+	k := "test-1"
+	r := &hapi.Release{Name: k}
+
+	ms := NewMemory()
+	if err := ms.Create(r); err != nil {
+		t.Fatalf("Failed create: %s", err)
+	}
+	if err := ms.Update(r); err != nil {
+		t.Fatalf("Failed update: %s", err)
+	}
+
+	if ms.releases[k].Name != k {
+		t.Errorf("Unexpected release name: %s", ms.releases[k].Name)
+	}
+}
+
 func TestList(t *testing.T) {
 	ms := NewMemory()
 	rels := []string{"a", "b", "c"}
 
 	for _, k := range rels {
-		ms.Set(k, &hapi.Release{Name: k})
+		ms.Create(&hapi.Release{Name: k})
 	}
 
 	l, err := ms.List()