From 58a1b308c1358e655e77f69e0887e34cbbea3030 Mon Sep 17 00:00:00 2001 From: Cesar Wong Date: Mon, 6 Apr 2015 08:23:33 -0400 Subject: [PATCH] Refactor storage return for pod etcd storage 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. --- pkg/master/master.go | 12 +++++----- pkg/registry/etcd/etcd_test.go | 4 ++-- pkg/registry/pod/etcd/etcd.go | 19 +++++++++++++--- pkg/registry/pod/etcd/etcd_test.go | 36 +++++++++++++++--------------- 4 files changed, 42 insertions(+), 29 deletions(-) diff --git a/pkg/master/master.go b/pkg/master/master.go index 9a92bf48d82..0339856229b 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -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) @@ -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), diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index 1e51b3d5f6c..430e8f64897 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -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 } diff --git a/pkg/registry/pod/etcd/etcd.go b/pkg/registry/pod/etcd/etcd.go index f35eaf8128e..8729ea1e557 100644 --- a/pkg/registry/pod/etcd/etcd.go +++ b/pkg/registry/pod/etcd/etcd.go @@ -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{} }, @@ -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. @@ -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{} } @@ -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{} } diff --git a/pkg/registry/pod/etcd/etcd_test.go b/pkg/registry/pod/etcd/etcd_test.go index 2b414572568..9d0f4dad589 100644 --- a/pkg/registry/pod/etcd/etcd_test.go +++ b/pkg/registry/pod/etcd/etcd_test.go @@ -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 { @@ -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{} @@ -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 { @@ -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) @@ -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 { @@ -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) @@ -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) @@ -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) @@ -280,7 +280,7 @@ func TestListPodListSelection(t *testing.T) { }, }, } - storage, _, _ := NewStorage(helper) + storage := NewStorage(helper).Pod ctx := api.NewDefaultContext() @@ -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 { @@ -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) @@ -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{ @@ -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) @@ -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" @@ -461,7 +461,7 @@ func TestUpdateWithConflictingNamespace(t *testing.T) { }, }, } - storage, _, _ := NewStorage(helper) + storage := NewStorage(helper).Pod pod := validChangedPod() pod.Namespace = "not-default" @@ -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) @@ -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 {