Skip to content

Commit

Permalink
Refactor storage return for pod etcd storage
Browse files Browse the repository at this point in the history 
Convert the return value of pods rest.NewStorage to a struct.
This will allow returning more storage objects for a pod (sub resources)
without awkwardly adding more return values.
  • Loading branch information
@csrwng
csrwng committed Apr 7, 2015
1 parent d685172 commit 58a1b30
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 29 deletions.
12 changes: 6 additions & 6 deletions pkg/master/master.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,8 @@ func logStackOnRecover(panicReason interface{}, httpWriter http.ResponseWriter)

// init initializes master.
func (m *Master) init(c *Config) {
podStorage, bindingStorage, podStatusStorage := podetcd.NewStorage(c.EtcdHelper)
podRegistry := pod.NewRegistry(podStorage)
podStorage := podetcd.NewStorage(c.EtcdHelper)
podRegistry := pod.NewRegistry(podStorage.Pod)

eventRegistry := event.NewEtcdRegistry(c.EtcdHelper, uint64(c.EventTTL.Seconds()))
limitRangeRegistry := limitrange.NewEtcdRegistry(c.EtcdHelper)
Expand All @@ -385,10 +385,10 @@ func (m *Master) init(c *Config) {

// TODO: Factor out the core API registration
m.storage = map[string]rest.Storage{
"pods": podStorage,
"pods/status": podStatusStorage,
"pods/binding": bindingStorage,
"bindings": bindingStorage,
"pods": podStorage.Pod,
"pods/status": podStorage.Status,
"pods/binding": podStorage.Binding,
"bindings": podStorage.Binding,

"replicationControllers": controllerStorage,
"services": service.NewStorage(m.serviceRegistry, c.Cloud, m.nodeRegistry, m.endpointRegistry, m.portalNet, c.ClusterName),
Expand Down
4 changes: 2 additions & 2 deletions pkg/registry/etcd/etcd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ func NewTestEtcdRegistry(client tools.EtcdClient) *Registry {

func NewTestEtcdRegistryWithPods(client tools.EtcdClient) *Registry {
helper := tools.NewEtcdHelper(client, latest.Codec)
podStorage, _, _ := podetcd.NewStorage(helper)
podStorage := podetcd.NewStorage(helper)
endpointStorage := endpointetcd.NewStorage(helper)
registry := NewRegistry(helper, pod.NewRegistry(podStorage), endpoint.NewRegistry(endpointStorage))
registry := NewRegistry(helper, pod.NewRegistry(podStorage.Pod), endpoint.NewRegistry(endpointStorage))
return registry
}

Expand Down
19 changes: 16 additions & 3 deletions pkg/registry/pod/etcd/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,20 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/util/fielderrors"
)

// rest implements a RESTStorage for pods against etcd
// PodStorage includes storage for pods and all sub resources
type PodStorage struct {
Pod *REST
Binding *BindingREST
Status *StatusREST
}

// REST implements a RESTStorage for pods against etcd
type REST struct {
etcdgeneric.Etcd
}

// NewStorage returns a RESTStorage object that will work against pods.
func NewStorage(h tools.EtcdHelper) (*REST, *BindingREST, *StatusREST) {
func NewStorage(h tools.EtcdHelper) PodStorage {
prefix := "/registry/pods"
store := &etcdgeneric.Etcd{
NewFunc: func() runtime.Object { return &api.Pod{} },
Expand Down Expand Up @@ -74,7 +81,11 @@ func NewStorage(h tools.EtcdHelper) (*REST, *BindingREST, *StatusREST) {

statusStore.UpdateStrategy = pod.StatusStrategy

return &REST{*store}, &BindingREST{store: store}, &StatusREST{store: &statusStore}
return PodStorage{
Pod: &REST{*store},
Binding: &BindingREST{store: store},
Status: &StatusREST{store: &statusStore},
}
}

// Implement Redirector.
Expand All @@ -90,6 +101,7 @@ type BindingREST struct {
store *etcdgeneric.Etcd
}

// New creates a new binding resource
func (r *BindingREST) New() runtime.Object {
return &api.Binding{}
}
Expand Down Expand Up @@ -165,6 +177,7 @@ type StatusREST struct {
store *etcdgeneric.Etcd
}

// New creates a new pod resource
func (r *StatusREST) New() runtime.Object {
return &api.Pod{}
}
Expand Down
36 changes: 18 additions & 18 deletions pkg/registry/pod/etcd/etcd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ func newHelper(t *testing.T) (*tools.FakeEtcdClient, tools.EtcdHelper) {

func newStorage(t *testing.T) (*REST, *BindingREST, *StatusREST, *tools.FakeEtcdClient, tools.EtcdHelper) {
fakeEtcdClient, h := newHelper(t)
storage, bindingStorage, statusStorage := NewStorage(h)
return storage, bindingStorage, statusStorage, fakeEtcdClient, h
storage := NewStorage(h)
return storage.Pod, storage.Binding, storage.Status, fakeEtcdClient, h
}

func validNewPod() *api.Pod {
Expand Down Expand Up @@ -89,7 +89,7 @@ func TestStorage(t *testing.T) {

func TestCreate(t *testing.T) {
fakeEtcdClient, helper := newHelper(t)
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod
test := resttest.New(t, storage, fakeEtcdClient.SetError)
pod := validNewPod()
pod.ObjectMeta = api.ObjectMeta{}
Expand All @@ -107,7 +107,7 @@ func TestCreate(t *testing.T) {

func TestDelete(t *testing.T) {
fakeEtcdClient, helper := newHelper(t)
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod
test := resttest.New(t, storage, fakeEtcdClient.SetError)

createFn := func() runtime.Object {
Expand Down Expand Up @@ -143,7 +143,7 @@ func expectPod(t *testing.T, out runtime.Object) (*api.Pod, bool) {
func TestCreateRegistryError(t *testing.T) {
fakeEtcdClient, helper := newHelper(t)
fakeEtcdClient.Err = fmt.Errorf("test error")
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

pod := validNewPod()
_, err := storage.Create(api.NewDefaultContext(), pod)
Expand All @@ -154,7 +154,7 @@ func TestCreateRegistryError(t *testing.T) {

func TestCreateSetsFields(t *testing.T) {
fakeEtcdClient, helper := newHelper(t)
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod
pod := validNewPod()
_, err := storage.Create(api.NewDefaultContext(), pod)
if err != fakeEtcdClient.Err {
Expand All @@ -176,7 +176,7 @@ func TestCreateSetsFields(t *testing.T) {
func TestListError(t *testing.T) {
fakeEtcdClient, helper := newHelper(t)
fakeEtcdClient.Err = fmt.Errorf("test error")
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod
pods, err := storage.List(api.NewDefaultContext(), labels.Everything(), fields.Everything())
if err != fakeEtcdClient.Err {
t.Fatalf("Expected %#v, Got %#v", fakeEtcdClient.Err, err)
Expand All @@ -194,7 +194,7 @@ func TestListEmptyPodList(t *testing.T) {
E: fakeEtcdClient.NewError(tools.EtcdErrorCodeNotFound),
}

storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod
pods, err := storage.List(api.NewContext(), labels.Everything(), fields.Everything())
if err != nil {
t.Fatalf("unexpected error: %v", err)
Expand Down Expand Up @@ -231,7 +231,7 @@ func TestListPodList(t *testing.T) {
},
},
}
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

podsObj, err := storage.List(api.NewDefaultContext(), labels.Everything(), fields.Everything())
pods := podsObj.(*api.PodList)
Expand Down Expand Up @@ -280,7 +280,7 @@ func TestListPodListSelection(t *testing.T) {
},
},
}
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

ctx := api.NewDefaultContext()

Expand Down Expand Up @@ -345,7 +345,7 @@ func TestListPodListSelection(t *testing.T) {
}

func TestPodDecode(t *testing.T) {
storage, _, _ := NewStorage(tools.EtcdHelper{})
storage := NewStorage(tools.EtcdHelper{}).Pod
expected := validNewPod()
body, err := latest.Codec.Encode(expected)
if err != nil {
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestGet(t *testing.T) {
},
},
}
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

obj, err := storage.Get(api.WithNamespace(api.NewContext(), "test"), "foo")
pod := obj.(*api.Pod)
Expand All @@ -392,7 +392,7 @@ func TestGet(t *testing.T) {
func TestPodStorageValidatesCreate(t *testing.T) {
fakeEtcdClient, helper := newHelper(t)
fakeEtcdClient.Err = fmt.Errorf("test error")
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

pod := validNewPod()
pod.Labels = map[string]string{
Expand All @@ -410,7 +410,7 @@ func TestPodStorageValidatesCreate(t *testing.T) {
// TODO: remove, this is covered by RESTTest.TestCreate
func TestCreatePod(t *testing.T) {
_, helper := newHelper(t)
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

pod := validNewPod()
obj, err := storage.Create(api.NewDefaultContext(), pod)
Expand All @@ -432,7 +432,7 @@ func TestCreatePod(t *testing.T) {
// TODO: remove, this is covered by RESTTest.TestCreate
func TestCreateWithConflictingNamespace(t *testing.T) {
_, helper := newHelper(t)
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

pod := validNewPod()
pod.Namespace = "not-default"
Expand Down Expand Up @@ -461,7 +461,7 @@ func TestUpdateWithConflictingNamespace(t *testing.T) {
},
},
}
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

pod := validChangedPod()
pod.Namespace = "not-default"
Expand Down Expand Up @@ -578,7 +578,7 @@ func TestResourceLocation(t *testing.T) {
},
},
}
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

redirector := rest.Redirector(storage)
location, _, err := redirector.ResourceLocation(api.NewDefaultContext(), tc.query)
Expand Down Expand Up @@ -616,7 +616,7 @@ func TestDeletePod(t *testing.T) {
},
},
}
storage, _, _ := NewStorage(helper)
storage := NewStorage(helper).Pod

_, err := storage.Delete(api.NewDefaultContext(), "foo", nil)
if err != nil {
Expand Down

0 comments on commit 58a1b30

Please sign in to comment.