Merge pull request #5429 from jszczepkowski/podfactory

Cleanup: removed BoundPodFactory.

pkg/master/master.go

@@ -368,9 +368,7 @@ func logStackOnRecover(panicReason interface{}, httpWriter http.ResponseWriter)
 
 // init initializes master.
 func (m *Master) init(c *Config) {
-
+	podStorage, bindingStorage, podStatusStorage := podetcd.NewREST(c.EtcdHelper)
-	boundPodFactory := &pod.BasicBoundPodFactory{}
-	podStorage, bindingStorage, podStatusStorage := podetcd.NewREST(c.EtcdHelper, boundPodFactory)
 	podRegistry := pod.NewRegistry(podStorage)
 
 	eventRegistry := event.NewEtcdRegistry(c.EtcdHelper, uint64(c.EventTTL.Seconds()))

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
 }

pkg/registry/pod/bound_pod_factory.go

@@ -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
-}

pkg/registry/pod/bound_pod_factory_test.go

@@ -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)
-	}
-}

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
+	store *etcdgeneric.Etcd
-	factory pod.BoundPodFactory
 }
 
 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

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)