From 8cc300665e2288b5b9bd8bf94006622a833fd1d8 Mon Sep 17 00:00:00 2001
From: Igor Bazhitov <ibazhitov@virtuozzo.com>
Date: Thu, 13 Jul 2017 16:36:56 +0300
Subject: [PATCH] Do not initialize custom REST handler during ApiVersion
 initialization

ApiVersion variable from the versioned package is initialized both
in the apiserver and in the controller-manager because they both
import the versioned package.
When a custom REST handler is defined, its NewFooREST() constructor is
being called directly during ApiVersion initialization. This constructor
may contain some other logic besides simply initializing the custom REST
registry type. For example it may establish a connection to some service.
And we'll end up with an extra unused connection to the external
service, established from the controller manager.

Passing NewFooREST as a function value to builders.NewApiResourceWithStorage()
and then calling it from versionedResourceBuilder.registerEndpoints()
solves this problem.

Fixes #92.
---
 .../generators/versioned_generator.go         | 20 ++++++++++++++++---
 .../api_versioned_resource_builder.go         | 18 ++++++++---------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/cmd/apiregister-gen/generators/versioned_generator.go b/cmd/apiregister-gen/generators/versioned_generator.go
index 842d6c3fd0..c24b81c5d5 100644
--- a/cmd/apiregister-gen/generators/versioned_generator.go
+++ b/cmd/apiregister-gen/generators/versioned_generator.go
@@ -39,14 +39,28 @@ func CreateVersionedGenerator(apiversion *APIVersion, apigroup *APIGroup, filena
 	}
 }
 
+func hasSubresources(version *APIVersion) bool {
+	for _, v := range version.Resources {
+		if len(v.Subresources) != 0 {
+			return true
+		}
+	}
+	return false
+}
+
 func (d *versionedGenerator) Imports(c *generator.Context) []string {
-	return []string{
+	imports := []string{
 		"metav1 \"k8s.io/apimachinery/pkg/apis/meta/v1\"",
 		"k8s.io/apimachinery/pkg/runtime",
 		"github.com/kubernetes-incubator/apiserver-builder/pkg/builders",
 		"k8s.io/apimachinery/pkg/runtime/schema",
 		d.apigroup.Pkg.Path,
 	}
+	if hasSubresources(d.apiversion) {
+		imports = append(imports, "k8s.io/apiserver/pkg/registry/rest")
+	}
+
+	return imports
 }
 
 func (d *versionedGenerator) Finalize(context *generator.Context, w io.Writer) error {
@@ -64,7 +78,7 @@ var (
 			{{.Kind}}SchemeFns{},
 			func() runtime.Object { return &{{ $api.Kind }}{} },     // Register versioned resource
 			func() runtime.Object { return &{{ $api.Kind }}List{} }, // Register versioned resource list
-			New{{ $api.REST }}(),
+			New{{ $api.REST }},
 		)
 	{{ else -}}
 		{{$api.Group}}{{$api.Kind}}Storage = builders.NewApiResource( // Resource status endpoint
@@ -95,7 +109,7 @@ var (
 			builders.SchemeFnsSingleton,
 			func() runtime.Object { return &{{ $subresource.Request }}{} }, // Register versioned resource
 			nil,
-			&{{ $subresource.REST }}{ {{$api.Group}}.New{{$api.Kind}}Registry({{$api.Group}}{{$api.Kind}}Storage) },
+			func() rest.Storage { return &{{ $subresource.REST }}{ {{$api.Group}}.New{{$api.Kind}}Registry({{$api.Group}}{{$api.Kind}}Storage) } },
 		),
 		{{ end -}}
 		{{ end -}}
diff --git a/pkg/builders/api_versioned_resource_builder.go b/pkg/builders/api_versioned_resource_builder.go
index b557c3f9ae..c662df371e 100644
--- a/pkg/builders/api_versioned_resource_builder.go
+++ b/pkg/builders/api_versioned_resource_builder.go
@@ -61,15 +61,15 @@ func NewApiResourceWithStorage(
 	unversionedBuilder UnversionedResourceBuilder,
 	schemeFns SchemeFns,
 	new, newList func() runtime.Object,
-	storage rest.Storage) *versionedResourceBuilder {
+	RESTFunc func() rest.Storage) *versionedResourceBuilder {
 	v := &versionedResourceBuilder{
-		unversionedBuilder, schemeFns, new, newList, nil, storage, nil,
+		unversionedBuilder, schemeFns, new, newList, nil, RESTFunc, nil,
 	}
 	if new == nil {
 		panic(fmt.Errorf("Cannot call NewApiResourceWithStorage with nil new function."))
 	}
-	if storage == nil {
-		panic(fmt.Errorf("Cannot call NewApiResourceWithStorage with nil new storage."))
+	if RESTFunc == nil {
+		panic(fmt.Errorf("Cannot call NewApiResourceWithStorage with nil RESTFunc function."))
 	}
 	return v
 }
@@ -84,11 +84,11 @@ type versionedResourceBuilder struct {
 	// NewListFunc returns and empty unversioned instance of a resource List
 	NewListFunc func() runtime.Object
 
-	// Store is used to modify the default storage, mutually exclusive with RESTFunc
+	// StorageBuilder is used to modify the default storage, mutually exclusive with RESTFunc
 	StorageBuilder StorageBuilder
 
-	// REST a rest.Store implementation, mutually exclusive with StoreFunc
-	REST rest.Storage
+	// RESTFunc returns a rest.Storage implementation, mutually exclusive with StorageBuilder
+	RESTFunc func() rest.Storage
 
 	Storage rest.StandardStorage
 }
@@ -173,9 +173,9 @@ func (b *versionedResourceBuilder) registerEndpoints(
 		path = b.Unversioned.GetName()
 	}
 
-	if b.REST != nil {
+	if b.RESTFunc != nil {
 		// Use the REST implementation directly.
-		registry[path] = b.REST
+		registry[path] = b.RESTFunc()
 	} else {
 		// Create a new REST implementation wired to storage.
 		registry[path] = b.