From ba9d02c0c7563c87b2a86237a8601b1a4e820e8f Mon Sep 17 00:00:00 2001 From: Jerzy Szczepkowski Date: Fri, 13 Mar 2015 09:58:00 +0100 Subject: [PATCH] Cleanup: removed BoundPodFactory. Removed unused code: BoundPodFactory. Fixes #5384. --- pkg/master/master.go | 4 +- pkg/registry/etcd/etcd_test.go | 2 +- pkg/registry/pod/bound_pod_factory.go | 38 ---------------- pkg/registry/pod/bound_pod_factory_test.go | 53 ---------------------- pkg/registry/pod/etcd/etcd.go | 9 ++-- pkg/registry/pod/etcd/etcd_test.go | 36 +++++++-------- 6 files changed, 24 insertions(+), 118 deletions(-) delete mode 100644 pkg/registry/pod/bound_pod_factory.go delete mode 100644 pkg/registry/pod/bound_pod_factory_test.go diff --git a/pkg/master/master.go b/pkg/master/master.go index 899985d5b0088..b9dda880759a2 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -368,9 +368,7 @@ func logStackOnRecover(panicReason interface{}, httpWriter http.ResponseWriter) // init initializes master. func (m *Master) init(c *Config) { - - boundPodFactory := &pod.BasicBoundPodFactory{} - podStorage, bindingStorage, podStatusStorage := podetcd.NewREST(c.EtcdHelper, boundPodFactory) + podStorage, bindingStorage, podStatusStorage := podetcd.NewREST(c.EtcdHelper) podRegistry := pod.NewRegistry(podStorage) eventRegistry := event.NewEtcdRegistry(c.EtcdHelper, uint64(c.EventTTL.Seconds())) diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index b9a62f1684604..466caa478ad31 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -42,7 +42,7 @@ func NewTestEtcdRegistry(client tools.EtcdClient) *Registry { func NewTestEtcdRegistryWithPods(client tools.EtcdClient) *Registry { helper := tools.EtcdHelper{client, latest.Codec, tools.RuntimeVersionAdapter{latest.ResourceVersioner}} - podStorage, _, _ := podetcd.NewREST(helper, nil) + podStorage, _, _ := podetcd.NewREST(helper) registry := NewRegistry(helper, pod.NewRegistry(podStorage)) return registry } diff --git a/pkg/registry/pod/bound_pod_factory.go b/pkg/registry/pod/bound_pod_factory.go deleted file mode 100644 index d421d1def6e9b..0000000000000 --- a/pkg/registry/pod/bound_pod_factory.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2014 Google Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package pod - -import ( - "github.com/GoogleCloudPlatform/kubernetes/pkg/api" -) - -type BoundPodFactory interface { - // Make a container object for a given pod, given the machine that the pod is running on. - MakeBoundPod(machine string, pod *api.Pod) (*api.BoundPod, error) -} - -type BasicBoundPodFactory struct{} - -func (b *BasicBoundPodFactory) MakeBoundPod(machine string, pod *api.Pod) (*api.BoundPod, error) { - boundPod := &api.BoundPod{} - if err := api.Scheme.Convert(pod, boundPod); err != nil { - return nil, err - } - // Make a dummy self link so that references to this bound pod will work. - boundPod.SelfLink = "/api/v1beta1/boundPods/" + boundPod.Name - return boundPod, nil -} diff --git a/pkg/registry/pod/bound_pod_factory_test.go b/pkg/registry/pod/bound_pod_factory_test.go deleted file mode 100644 index 68336e0bb15c5..0000000000000 --- a/pkg/registry/pod/bound_pod_factory_test.go +++ /dev/null @@ -1,53 +0,0 @@ -/* -Copyright 2014 Google Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package pod - -import ( - "testing" - - "github.com/GoogleCloudPlatform/kubernetes/pkg/api" -) - -func TestMakeBoundPodNoServices(t *testing.T) { - factory := &BasicBoundPodFactory{} - - pod, err := factory.MakeBoundPod("machine", &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foobar"}, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "foo", - }, - }, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - container := pod.Spec.Containers[0] - if len(container.Env) != 0 { - t.Errorf("Expected zero env vars, got: %#v", pod) - } - if pod.Name != "foobar" { - t.Errorf("Failed to assign ID to pod: %#v", pod.Name) - } - - if _, err := api.GetReference(pod); err != nil { - t.Errorf("Unable to get a reference to bound pod: %v", err) - } -} diff --git a/pkg/registry/pod/etcd/etcd.go b/pkg/registry/pod/etcd/etcd.go index 1bfaf50cade25..6878599b347d5 100644 --- a/pkg/registry/pod/etcd/etcd.go +++ b/pkg/registry/pod/etcd/etcd.go @@ -39,7 +39,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against pods. -func NewREST(h tools.EtcdHelper, factory pod.BoundPodFactory) (*REST, *BindingREST, *StatusREST) { +func NewREST(h tools.EtcdHelper) (*REST, *BindingREST, *StatusREST) { prefix := "/registry/pods" store := &etcdgeneric.Etcd{ NewFunc: func() runtime.Object { return &api.Pod{} }, @@ -71,7 +71,7 @@ func NewREST(h tools.EtcdHelper, factory pod.BoundPodFactory) (*REST, *BindingRE statusStore.UpdateStrategy = pod.StatusStrategy - return &REST{store: store}, &BindingREST{store: store, factory: factory}, &StatusREST{store: &statusStore} + return &REST{store: store}, &BindingREST{store: store}, &StatusREST{store: &statusStore} } // WithPodStatus returns a rest object that decorates returned responses with extra @@ -130,8 +130,7 @@ func (r *REST) ResourceLocation(ctx api.Context, name string) (string, error) { // BindingREST implements the REST endpoint for binding pods to nodes when etcd is in use. type BindingREST struct { - store *etcdgeneric.Etcd - factory pod.BoundPodFactory + store *etcdgeneric.Etcd } func (r *BindingREST) New() runtime.Object { @@ -167,7 +166,7 @@ func (r *BindingREST) setPodHostTo(ctx api.Context, podID, oldMachine, machine s return nil, fmt.Errorf("unexpected object: %#v", obj) } if pod.Spec.Host != oldMachine || pod.Status.Host != oldMachine { - return nil, fmt.Errorf("pod %v is already assigned to host %v or %v", pod.Name, pod.Spec.Host, pod.Status.Host) + return nil, fmt.Errorf("pod %v is already assigned to host %q or %q", pod.Name, pod.Spec.Host, pod.Status.Host) } pod.Spec.Host = machine pod.Status.Host = machine diff --git a/pkg/registry/pod/etcd/etcd_test.go b/pkg/registry/pod/etcd/etcd_test.go index 05994b8546a5d..af74afa185ef1 100644 --- a/pkg/registry/pod/etcd/etcd_test.go +++ b/pkg/registry/pod/etcd/etcd_test.go @@ -69,7 +69,7 @@ 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 := NewREST(h, &pod.BasicBoundPodFactory{}) + storage, bindingStorage, statusStorage := NewREST(h) storage = storage.WithPodStatus(&fakeCache{statusToReturn: &api.PodStatus{}}) return storage, bindingStorage, statusStorage, fakeEtcdClient, h } @@ -112,7 +112,7 @@ func TestStorage(t *testing.T) { func TestCreate(t *testing.T) { fakeEtcdClient, helper := newHelper(t) - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{}} storage = storage.WithPodStatus(cache) test := resttest.New(t, storage, fakeEtcdClient.SetError) @@ -142,7 +142,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, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) pod := validNewPod() _, err := storage.Create(api.NewDefaultContext(), pod) @@ -153,7 +153,7 @@ func TestCreateRegistryError(t *testing.T) { func TestCreateSetsFields(t *testing.T) { fakeEtcdClient, helper := newHelper(t) - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{}} storage = storage.WithPodStatus(cache) pod := validNewPod() @@ -177,7 +177,7 @@ func TestCreateSetsFields(t *testing.T) { func TestListError(t *testing.T) { fakeEtcdClient, helper := newHelper(t) fakeEtcdClient.Err = fmt.Errorf("test error") - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{} storage = storage.WithPodStatus(cache) pods, err := storage.List(api.NewDefaultContext(), labels.Everything(), fields.Everything()) @@ -205,7 +205,7 @@ func TestListCacheError(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{errorToReturn: client.ErrPodInfoNotAvailable} storage = storage.WithPodStatus(cache) @@ -230,7 +230,7 @@ func TestListEmptyPodList(t *testing.T) { E: fakeEtcdClient.NewError(tools.EtcdErrorCodeNotFound), } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{} storage = storage.WithPodStatus(cache) pods, err := storage.List(api.NewContext(), labels.Everything(), fields.Everything()) @@ -268,7 +268,7 @@ func TestListPodList(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{Phase: api.PodRunning}} storage = storage.WithPodStatus(cache) @@ -319,7 +319,7 @@ func TestListPodListSelection(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{Phase: api.PodRunning}} storage = storage.WithPodStatus(cache) @@ -386,7 +386,7 @@ func TestListPodListSelection(t *testing.T) { } func TestPodDecode(t *testing.T) { - storage, _, _ := NewREST(tools.EtcdHelper{}, nil) + storage, _, _ := NewREST(tools.EtcdHelper{}) expected := validNewPod() body, err := latest.Codec.Encode(expected) if err != nil { @@ -415,7 +415,7 @@ func TestGet(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{Phase: api.PodRunning}} storage = storage.WithPodStatus(cache) @@ -443,7 +443,7 @@ func TestGetCacheError(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{errorToReturn: client.ErrPodInfoNotAvailable} storage = storage.WithPodStatus(cache) @@ -463,7 +463,7 @@ func TestGetCacheError(t *testing.T) { func TestPodStorageValidatesCreate(t *testing.T) { fakeEtcdClient, helper := newHelper(t) fakeEtcdClient.Err = fmt.Errorf("test error") - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{}} storage = storage.WithPodStatus(cache) @@ -483,7 +483,7 @@ func TestPodStorageValidatesCreate(t *testing.T) { // TODO: remove, this is covered by RESTTest.TestCreate func TestCreatePod(t *testing.T) { _, helper := newHelper(t) - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{}} storage = storage.WithPodStatus(cache) @@ -507,7 +507,7 @@ func TestCreatePod(t *testing.T) { // TODO: remove, this is covered by RESTTest.TestCreate func TestCreateWithConflictingNamespace(t *testing.T) { _, helper := newHelper(t) - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{} storage = storage.WithPodStatus(cache) @@ -538,7 +538,7 @@ func TestUpdateWithConflictingNamespace(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{} storage = storage.WithPodStatus(cache) @@ -650,7 +650,7 @@ func TestResourceLocation(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{PodIP: expectedIP}} storage = storage.WithPodStatus(cache) @@ -684,7 +684,7 @@ func TestDeletePod(t *testing.T) { }, }, } - storage, _, _ := NewREST(helper, nil) + storage, _, _ := NewREST(helper) cache := &fakeCache{statusToReturn: &api.PodStatus{}} storage = storage.WithPodStatus(cache)