refactor destroy func in unit testing

pkg/registry/experimental/controller/etcd/etcd_test.go

@@ -28,12 +28,17 @@ import (
 	"k8s.io/kubernetes/pkg/storage"
 	"k8s.io/kubernetes/pkg/storage/etcd/etcdtest"
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
+	"k8s.io/kubernetes/pkg/storage/storagebackend/factory"
 )
 
-func newStorage(t *testing.T) (*ScaleREST, *etcdtesting.EtcdTestServer, storage.Interface, func()) {
+func newStorage(t *testing.T) (*ScaleREST, *etcdtesting.EtcdTestServer, storage.Interface, factory.DestroyFunc) {
 	etcdStorage, server := registrytest.NewEtcdStorage(t, "")
 	restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "controllers"}
-	s, destroyFunc := generic.NewRawStorage(etcdStorage)
+	s, d := generic.NewRawStorage(etcdStorage)
+	destroyFunc := func() {
+		d()
+		server.Terminate(t)
+	}
 	return NewStorage(restOptions).Scale, server, s, destroyFunc
 }
 
@@ -83,11 +88,8 @@ var validScale = extensions.Scale{
 }
 
 func TestGet(t *testing.T) {
-	storage, server, si, destroyFunc := newStorage(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	storage, _, si, destroyFunc := newStorage(t)
+	defer destroyFunc()
 
 	ctx := api.WithNamespace(api.NewContext(), "test")
 	key := etcdtest.AddPrefix("/controllers/test/foo")
@@ -105,11 +107,8 @@ func TestGet(t *testing.T) {
 }
 
 func TestUpdate(t *testing.T) {
-	storage, server, si, destroyFunc := newStorage(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	storage, _, si, destroyFunc := newStorage(t)
+	defer destroyFunc()
 
 	ctx := api.WithNamespace(api.NewContext(), "test")
 	key := etcdtest.AddPrefix("/controllers/test/foo")

pkg/registry/generic/registry/storage_factory.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/kubernetes/pkg/storage"
 	etcdstorage "k8s.io/kubernetes/pkg/storage/etcd"
 	"k8s.io/kubernetes/pkg/storage/storagebackend"
+	"k8s.io/kubernetes/pkg/storage/storagebackend/factory"
 )
 
 // Creates a cacher based given storageConfig.
@@ -33,7 +34,7 @@ func StorageWithCacher(
 	resourcePrefix string,
 	scopeStrategy rest.NamespaceScopedStrategy,
 	newListFunc func() runtime.Object,
-	triggerFunc storage.TriggerPublisherFunc) (storage.Interface, func()) {
+	triggerFunc storage.TriggerPublisherFunc) (storage.Interface, factory.DestroyFunc) {
 
 	s, d := generic.NewRawStorage(storageConfig)
 	// TODO: we would change this later to make storage always have cacher and hide low level KV layer inside.

pkg/registry/generic/registry/store_test.go

@@ -40,6 +40,7 @@ import (
 	etcdstorage "k8s.io/kubernetes/pkg/storage/etcd"
 	"k8s.io/kubernetes/pkg/storage/etcd/etcdtest"
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
+	"k8s.io/kubernetes/pkg/storage/storagebackend/factory"
 	storagetesting "k8s.io/kubernetes/pkg/storage/testing"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/util/validation/field"
@@ -88,7 +89,7 @@ func (t *testRESTStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Obje
 }
 func (t *testRESTStrategy) Canonicalize(obj runtime.Object) {}
 
-func NewTestGenericStoreRegistry(t *testing.T) (*etcdtesting.EtcdTestServer, func(), *Store) {
+func NewTestGenericStoreRegistry(t *testing.T) (factory.DestroyFunc, *Store) {
 	return newTestGenericStoreRegistry(t, false)
 }
 
@@ -173,7 +174,7 @@ func TestStoreList(t *testing.T) {
 		if item.context != nil {
 			ctx = item.context
 		}
-		server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
+		destroyFunc, registry := NewTestGenericStoreRegistry(t)
 
 		if item.in != nil {
 			if err := storagetesting.CreateList("/pods", registry.Storage, item.in); err != nil {
@@ -192,7 +193,6 @@ func TestStoreList(t *testing.T) {
 			t.Errorf("%v: Expected %#v, got %#v", name, e, a)
 		}
 		destroyFunc()
-		server.Terminate(t)
 	}
 }
 
@@ -209,11 +209,8 @@ func TestStoreListResourceVersion(t *testing.T) {
 	}
 	ctx := api.WithNamespace(api.NewContext(), "test")
 
-	server, destroyFunc, registry := newTestGenericStoreRegistry(t, true)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := newTestGenericStoreRegistry(t, true)
+	defer destroyFunc()
 
 	obj, err := registry.Create(ctx, fooPod)
 	if err != nil {
@@ -274,11 +271,8 @@ func TestStoreCreate(t *testing.T) {
 	}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	// create the object
 	objA, err := registry.Create(testContext, podA)
@@ -337,11 +331,8 @@ func TestStoreUpdate(t *testing.T) {
 	}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	// Test1 try to update a non-existing node
 	_, _, err := registry.Update(testContext, podA.Name, rest.DefaultUpdatedObjectInfo(podA, api.Scheme))
@@ -378,11 +369,8 @@ func TestStoreUpdate(t *testing.T) {
 }
 
 func TestNoOpUpdates(t *testing.T) {
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	newPod := func() *api.Pod {
 		return &api.Pod{
@@ -463,11 +451,8 @@ func TestStoreCustomExport(t *testing.T) {
 		Spec: api.PodSpec{NodeName: "machine"},
 	}
 
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	registry.ExportStrategy = testPodExport{}
 
@@ -512,11 +497,8 @@ func TestStoreBasicExport(t *testing.T) {
 		Status: api.PodStatus{HostIP: "1.2.3.4"},
 	}
 
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
 	registry.UpdateStrategy.(*testRESTStrategy).allowCreateOnUpdate = true
@@ -547,11 +529,8 @@ func TestStoreGet(t *testing.T) {
 	}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	_, err := registry.Get(testContext, podA.Name)
 	if !errors.IsNotFound(err) {
@@ -571,11 +550,8 @@ func TestStoreDelete(t *testing.T) {
 	}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	// test failure condition
 	_, err := registry.Delete(testContext, podA.Name, nil)
@@ -612,11 +588,8 @@ func TestStoreHandleFinalizers(t *testing.T) {
 	}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 	// create pod
 	_, err := registry.Create(testContext, podWithFinalizer)
 	if err != nil {
@@ -911,11 +884,8 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) {
 	}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	for _, tc := range testcases {
 		registry.DeleteStrategy = tc.strategy
@@ -961,11 +931,8 @@ func TestStoreDeleteCollection(t *testing.T) {
 	podB := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "bar"}}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	if _, err := registry.Create(testContext, podA); err != nil {
 		t.Errorf("Unexpected error: %v", err)
@@ -993,11 +960,8 @@ func TestStoreDeleteCollection(t *testing.T) {
 }
 
 func TestStoreDeleteCollectionNotFound(t *testing.T) {
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
 
@@ -1042,11 +1006,8 @@ func TestStoreDeleteCollectionWithWatch(t *testing.T) {
 	podA := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}
 
 	testContext := api.WithNamespace(api.NewContext(), "test")
-	server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	destroyFunc, registry := NewTestGenericStoreRegistry(t)
+	defer destroyFunc()
 
 	objCreated, err := registry.Create(testContext, podA)
 	if err != nil {
@@ -1112,7 +1073,7 @@ func TestStoreWatch(t *testing.T) {
 			Spec: api.PodSpec{NodeName: "machine"},
 		}
 
-		server, destroyFunc, registry := NewTestGenericStoreRegistry(t)
+		destroyFunc, registry := NewTestGenericStoreRegistry(t)
 		wi, err := registry.WatchPredicate(ctx, m.selectPred, "0")
 		if err != nil {
 			t.Errorf("%v: unexpected error: %v", name, err)
@@ -1131,18 +1092,19 @@ func TestStoreWatch(t *testing.T) {
 			wi.Stop()
 		}
 		destroyFunc()
-		server.Terminate(t)
 	}
 }
 
-func newTestGenericStoreRegistry(t *testing.T, hasCacheEnabled bool) (*etcdtesting.EtcdTestServer, func(), *Store) {
+func newTestGenericStoreRegistry(t *testing.T, hasCacheEnabled bool) (factory.DestroyFunc, *Store) {
 	podPrefix := "/pods"
 	server := etcdtesting.NewEtcdTestClientServer(t)
 	strategy := &testRESTStrategy{api.Scheme, api.SimpleNameGenerator, true, false, true}
 
 	codec := testapi.Default.StorageCodec()
 	s := etcdstorage.NewEtcdStorage(server.Client, codec, etcdtest.PathPrefix(), false, etcdtest.DeserializationCacheSize)
-	destroyFunc := func() {}
+	destroyFunc := func() {
+		server.Terminate(t)
+	}
 	if hasCacheEnabled {
 		config := storage.CacherConfig{
 			CacheCapacity:  10,
@@ -1155,11 +1117,15 @@ func newTestGenericStoreRegistry(t *testing.T, hasCacheEnabled bool) (*etcdtesti
 			Codec:          codec,
 		}
 		cacher := storage.NewCacherFromConfig(config)
+		d := destroyFunc
 		s = cacher
-		destroyFunc = cacher.Stop
+		destroyFunc = func() {
+			cacher.Stop()
+			d()
+		}
 	}
 
-	return server, destroyFunc, &Store{
+	return destroyFunc, &Store{
 		NewFunc:           func() runtime.Object { return &api.Pod{} },
 		NewListFunc:       func() runtime.Object { return &api.PodList{} },
 		QualifiedResource: api.Resource("pods"),

pkg/registry/generic/storage_decorator.go

@@ -34,7 +34,7 @@ type StorageDecorator func(
 	resourcePrefix string,
 	scopeStrategy rest.NamespaceScopedStrategy,
 	newListFunc func() runtime.Object,
-	trigger storage.TriggerPublisherFunc) (storage.Interface, func())
+	trigger storage.TriggerPublisherFunc) (storage.Interface, factory.DestroyFunc)
 
 // Returns given 'storageInterface' without any decoration.
 func UndecoratedStorage(
@@ -44,14 +44,14 @@ func UndecoratedStorage(
 	resourcePrefix string,
 	scopeStrategy rest.NamespaceScopedStrategy,
 	newListFunc func() runtime.Object,
-	trigger storage.TriggerPublisherFunc) (storage.Interface, func()) {
+	trigger storage.TriggerPublisherFunc) (storage.Interface, factory.DestroyFunc) {
 	return NewRawStorage(config)
 }
 
 // NewRawStorage creates the low level kv storage. This is a work-around for current
 // two layer of same storage interface.
 // TODO: Once cacher is enabled on all registries (event registry is special), we will remove this method.
-func NewRawStorage(config *storagebackend.Config) (storage.Interface, func()) {
+func NewRawStorage(config *storagebackend.Config) (storage.Interface, factory.DestroyFunc) {
 	s, d, err := factory.Create(*config)
 	if err != nil {
 		glog.Fatalf("Unable to create storage backend: config (%v), err (%v)", config, err)

pkg/registry/service/ipallocator/etcd/etcd_test.go

@@ -30,11 +30,12 @@ import (
 	"k8s.io/kubernetes/pkg/storage"
 	"k8s.io/kubernetes/pkg/storage/etcd/etcdtest"
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
+	"k8s.io/kubernetes/pkg/storage/storagebackend/factory"
 
 	"golang.org/x/net/context"
 )
 
-func newStorage(t *testing.T) (*etcdtesting.EtcdTestServer, ipallocator.Interface, allocator.Interface, storage.Interface, func()) {
+func newStorage(t *testing.T) (*etcdtesting.EtcdTestServer, ipallocator.Interface, allocator.Interface, storage.Interface, factory.DestroyFunc) {
 	etcdStorage, server := registrytest.NewEtcdStorage(t, "")
 	_, cidr, err := net.ParseCIDR("192.168.1.0/24")
 	if err != nil {
@@ -48,7 +49,11 @@ func newStorage(t *testing.T) (*etcdtesting.EtcdTestServer, ipallocator.Interfac
 		etcd := allocatoretcd.NewEtcd(mem, "/ranges/serviceips", api.Resource("serviceipallocations"), etcdStorage)
 		return etcd
 	})
-	s, destroyFunc := generic.NewRawStorage(etcdStorage)
+	s, d := generic.NewRawStorage(etcdStorage)
+	destroyFunc := func() {
+		d()
+		server.Terminate(t)
+	}
 	return server, storage, backing, s, destroyFunc
 }
 
@@ -65,33 +70,24 @@ func key() string {
 }
 
 func TestEmpty(t *testing.T) {
-	server, storage, _, _, destroyFunc := newStorage(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	_, storage, _, _, destroyFunc := newStorage(t)
+	defer destroyFunc()
 	if err := storage.Allocate(net.ParseIP("192.168.1.2")); !strings.Contains(err.Error(), "cannot allocate resources of type serviceipallocations at this time") {
 		t.Fatal(err)
 	}
 }
 
 func TestErrors(t *testing.T) {
-	server, storage, _, _, destroyFunc := newStorage(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	_, storage, _, _, destroyFunc := newStorage(t)
+	defer destroyFunc()
 	if err := storage.Allocate(net.ParseIP("192.168.0.0")); err != ipallocator.ErrNotInRange {
 		t.Fatal(err)
 	}
 }
 
 func TestStore(t *testing.T) {
-	server, storage, backing, si, destroyFunc := newStorage(t)
-	defer func() {
-		destroyFunc()
-		server.Terminate(t)
-	}()
+	_, storage, backing, si, destroyFunc := newStorage(t)
+	defer destroyFunc()
 	if err := si.Create(context.TODO(), key(), validNewRangeAllocation(), nil, 0); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

pkg/storage/storagebackend/factory/etcd2.go

@@ -30,7 +30,7 @@ import (
 	utilnet "k8s.io/kubernetes/pkg/util/net"
 )
 
-func newETCD2Storage(c storagebackend.Config) (storage.Interface, func(), error) {
+func newETCD2Storage(c storagebackend.Config) (storage.Interface, DestroyFunc, error) {
 	tr, err := newTransportForETCD2(c.CertFile, c.KeyFile, c.CAFile)
 	if err != nil {
 		return nil, nil, err

pkg/storage/storagebackend/factory/etcd3.go

@@ -26,7 +26,7 @@ import (
 	"golang.org/x/net/context"
 )
 
-func newETCD3Storage(c storagebackend.Config) (storage.Interface, func(), error) {
+func newETCD3Storage(c storagebackend.Config) (storage.Interface, DestroyFunc, error) {
 	tlsInfo := transport.TLSInfo{
 		CertFile: c.CertFile,
 		KeyFile:  c.KeyFile,

pkg/storage/storagebackend/factory/factory.go

@@ -23,8 +23,11 @@ import (
 	"k8s.io/kubernetes/pkg/storage/storagebackend"
 )
 
+// DestroyFunc is to destroy any resources used by the storage returned in Create() together.
+type DestroyFunc func()
+
 // Create creates a storage backend based on given config.
-func Create(c storagebackend.Config) (storage.Interface, func(), error) {
+func Create(c storagebackend.Config) (storage.Interface, DestroyFunc, error) {
 	switch c.Type {
 	case storagebackend.StorageTypeUnset, storagebackend.StorageTypeETCD2:
 		return newETCD2Storage(c)