Merge pull request #33038 from deads2k/api-11-push-deps-down

Automatic merge from submit-queue

remove storage related fields from genericapiserver

Removes `StorageFactory` and `StorageDecorator` from from `genericapiserver` since both constructs are related to building a `RESTStorage`, which should be provided fully formed (or via factory func) to a truly generic API server.

I found this while trying to move the creation API routes earlier.

cmd/kube-apiserver/app/server.go

@@ -295,7 +295,6 @@ func Run(s *options.APIServer) error {
 
 	genericConfig := genericapiserver.NewConfig(s.ServerRunOptions)
 	// TODO: Move the following to generic api server as well.
-	genericConfig.StorageFactory = storageFactory
 	genericConfig.Authenticator = apiAuthenticator
 	genericConfig.SupportsBasicAuth = len(s.BasicAuthFile) > 0
 	genericConfig.Authorizer = apiAuthorizer
@@ -311,7 +310,10 @@ func Run(s *options.APIServer) error {
 	genericConfig.EnableOpenAPISupport = true
 
 	config := &master.Config{
-		Config:                  genericConfig,
+		Config: genericConfig,
+
+		StorageFactory:          storageFactory,
+		EnableWatchCache:        s.EnableWatchCache,
 		EnableCoreControllers:   true,
 		DeleteCollectionWorkers: s.DeleteCollectionWorkers,
 		EventTTL:                s.EventTTL,

examples/apiserver/apiserver.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/kubernetes/pkg/genericapiserver/authorizer"
 	genericoptions "k8s.io/kubernetes/pkg/genericapiserver/options"
 	genericvalidation "k8s.io/kubernetes/pkg/genericapiserver/validation"
+	"k8s.io/kubernetes/pkg/registry/generic"
 	"k8s.io/kubernetes/pkg/storage/storagebackend"
 
 	// Install the testgroup API
@@ -87,7 +88,7 @@ func Run(serverOptions *genericoptions.ServerRunOptions) error {
 	}
 
 	restStorageMap := map[string]rest.Storage{
-		"testtypes": testgroupetcd.NewREST(storageConfig, s.StorageDecorator()),
+		"testtypes": testgroupetcd.NewREST(storageConfig, generic.UndecoratedStorage),
 	}
 	apiGroupInfo := genericapiserver.APIGroupInfo{
 		GroupMeta: *groupMeta,

federation/cmd/federation-apiserver/app/core.go

@@ -40,11 +40,11 @@ import (
 	serviceetcd "k8s.io/kubernetes/pkg/registry/core/service/etcd"
 )
 
-func installCoreAPIs(s *options.ServerRunOptions, g *genericapiserver.GenericAPIServer, f genericapiserver.StorageFactory) {
+func installCoreAPIs(s *options.ServerRunOptions, g *genericapiserver.GenericAPIServer, restOptionsFactory restOptionsFactory) {
-	serviceStore, serviceStatusStore := serviceetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("service")))
+	serviceStore, serviceStatusStore := serviceetcd.NewREST(restOptionsFactory.NewFor(api.Resource("service")))
-	namespaceStore, namespaceStatusStore, namespaceFinalizeStore := namespaceetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("namespaces")))
+	namespaceStore, namespaceStatusStore, namespaceFinalizeStore := namespaceetcd.NewREST(restOptionsFactory.NewFor(api.Resource("namespaces")))
-	secretStore := secretetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("secrets")))
+	secretStore := secretetcd.NewREST(restOptionsFactory.NewFor(api.Resource("secrets")))
-	eventStore := eventetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("events")), uint64(s.EventTTL.Seconds()))
+	eventStore := eventetcd.NewREST(restOptionsFactory.NewFor(api.Resource("events")), uint64(s.EventTTL.Seconds()))
 	coreResources := map[string]rest.Storage{
 		"secrets":             secretStore,
 		"services":            serviceStore,

federation/cmd/federation-apiserver/app/extensions.go

@@ -18,7 +18,6 @@ package app
 
 import (
 	"github.com/golang/glog"
-	"k8s.io/kubernetes/federation/cmd/federation-apiserver/app/options"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/rest"
 	"k8s.io/kubernetes/pkg/apimachinery/registered"
@@ -29,9 +28,9 @@ import (
 	replicasetetcd "k8s.io/kubernetes/pkg/registry/extensions/replicaset/etcd"
 )
 
-func installExtensionsAPIs(s *options.ServerRunOptions, g *genericapiserver.GenericAPIServer, f genericapiserver.StorageFactory) {
+func installExtensionsAPIs(g *genericapiserver.GenericAPIServer, restOptionsFactory restOptionsFactory) {
-	replicaSetStorage := replicasetetcd.NewStorage(createRESTOptionsOrDie(s, g, f, extensions.Resource("replicasets")))
+	replicaSetStorage := replicasetetcd.NewStorage(restOptionsFactory.NewFor(extensions.Resource("replicasets")))
-	ingressStorage, ingressStatusStorage := ingressetcd.NewREST(createRESTOptionsOrDie(s, g, f, extensions.Resource("ingresses")))
+	ingressStorage, ingressStatusStorage := ingressetcd.NewREST(restOptionsFactory.NewFor(extensions.Resource("ingresses")))
 	extensionsResources := map[string]rest.Storage{
 		"replicasets":        replicaSetStorage.ReplicaSet,
 		"replicasets/status": replicaSetStorage.Status,

federation/cmd/federation-apiserver/app/federation.go

@@ -20,7 +20,6 @@ import (
 	"github.com/golang/glog"
 
 	"k8s.io/kubernetes/federation/apis/federation"
-	"k8s.io/kubernetes/federation/cmd/federation-apiserver/app/options"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/rest"
 	"k8s.io/kubernetes/pkg/apimachinery/registered"
@@ -30,8 +29,8 @@ import (
 	clusteretcd "k8s.io/kubernetes/federation/registry/cluster/etcd"
 )
 
-func installFederationAPIs(s *options.ServerRunOptions, g *genericapiserver.GenericAPIServer, f genericapiserver.StorageFactory) {
+func installFederationAPIs(g *genericapiserver.GenericAPIServer, restOptionsFactory restOptionsFactory) {
-	clusterStorage, clusterStatusStorage := clusteretcd.NewREST(createRESTOptionsOrDie(s, g, f, federation.Resource("clusters")))
+	clusterStorage, clusterStatusStorage := clusteretcd.NewREST(restOptionsFactory.NewFor(federation.Resource("clusters")))
 	federationResources := map[string]rest.Storage{
 		"clusters":        clusterStorage,
 		"clusters/status": clusterStatusStorage,

federation/cmd/federation-apiserver/app/server.go

@@ -44,6 +44,7 @@ import (
 	genericvalidation "k8s.io/kubernetes/pkg/genericapiserver/validation"
 	"k8s.io/kubernetes/pkg/registry/cachesize"
 	"k8s.io/kubernetes/pkg/registry/generic"
+	"k8s.io/kubernetes/pkg/registry/generic/registry"
 	"k8s.io/kubernetes/pkg/registry/rbac/clusterrole"
 	clusterroleetcd "k8s.io/kubernetes/pkg/registry/rbac/clusterrole/etcd"
 	"k8s.io/kubernetes/pkg/registry/rbac/clusterrolebinding"
@@ -201,7 +202,6 @@ func Run(s *options.ServerRunOptions) error {
 	}
 	genericConfig := genericapiserver.NewConfig(s.ServerRunOptions)
 	// TODO: Move the following to generic api server as well.
-	genericConfig.StorageFactory = storageFactory
 	genericConfig.Authenticator = apiAuthenticator
 	genericConfig.SupportsBasicAuth = len(s.BasicAuthFile) > 0
 	genericConfig.Authorizer = apiAuthorizer
@@ -227,24 +227,40 @@ func Run(s *options.ServerRunOptions) error {
 	routes.UIRedirect{}.Install(m.Mux, m.HandlerContainer)
 	routes.Logs{}.Install(m.Mux, m.HandlerContainer)
 
-	installFederationAPIs(s, m, storageFactory)
+	restOptionsFactory := restOptionsFactory{
-	installCoreAPIs(s, m, storageFactory)
+		storageFactory:          storageFactory,
-	installExtensionsAPIs(s, m, storageFactory)
+		deleteCollectionWorkers: s.DeleteCollectionWorkers,
+	}
+	if s.EnableWatchCache {
+		restOptionsFactory.storageDecorator = registry.StorageWithCacher
+	} else {
+		restOptionsFactory.storageDecorator = generic.UndecoratedStorage
+	}
+
+	installFederationAPIs(m, restOptionsFactory)
+	installCoreAPIs(s, m, restOptionsFactory)
+	installExtensionsAPIs(m, restOptionsFactory)
 
 	sharedInformers.Start(wait.NeverStop)
 	m.Run(s.ServerRunOptions)
 	return nil
 }
 
-func createRESTOptionsOrDie(s *options.ServerRunOptions, g *genericapiserver.GenericAPIServer, f genericapiserver.StorageFactory, resource unversioned.GroupResource) generic.RESTOptions {
+type restOptionsFactory struct {
-	config, err := f.NewConfig(resource)
+	storageFactory          genericapiserver.StorageFactory
+	storageDecorator        generic.StorageDecorator
+	deleteCollectionWorkers int
+}
+
+func (f restOptionsFactory) NewFor(resource unversioned.GroupResource) generic.RESTOptions {
+	config, err := f.storageFactory.NewConfig(resource)
 	if err != nil {
 		glog.Fatalf("Unable to find storage config for %v, due to %v", resource, err.Error())
 	}
 	return generic.RESTOptions{
 		StorageConfig:           config,
-		Decorator:               g.StorageDecorator(),
+		Decorator:               f.storageDecorator,
-		DeleteCollectionWorkers: s.DeleteCollectionWorkers,
+		DeleteCollectionWorkers: f.deleteCollectionWorkers,
-		ResourcePrefix:          f.ResourcePrefix(resource),
+		ResourcePrefix:          f.storageFactory.ResourcePrefix(resource),
 	}
 }

pkg/genericapiserver/config.go

@@ -46,8 +46,6 @@ import (
 	"k8s.io/kubernetes/pkg/genericapiserver/routes"
 	genericvalidation "k8s.io/kubernetes/pkg/genericapiserver/validation"
 	ipallocator "k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
-	"k8s.io/kubernetes/pkg/registry/generic"
-	"k8s.io/kubernetes/pkg/registry/generic/registry"
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util"
 	utilnet "k8s.io/kubernetes/pkg/util/net"
@@ -55,8 +53,6 @@ import (
 
 // Config is a structure used to configure a GenericAPIServer.
 type Config struct {
-	// The storage factory for other objects
-	StorageFactory     StorageFactory
 	AuditLogPath       string
 	AuditLogMaxAge     int
 	AuditLogMaxBackups int
@@ -73,7 +69,6 @@ type Config struct {
 	EnableIndex             bool
 	EnableProfiling         bool
 	EnableVersion           bool
-	EnableWatchCache        bool
 	EnableGarbageCollection bool
 	APIPrefix               string
 	APIGroupPrefix          string
@@ -179,7 +174,6 @@ func NewConfig(options *options.ServerRunOptions) *Config {
 		EnableSwaggerSupport:      true,
 		EnableSwaggerUI:           options.EnableSwaggerUI,
 		EnableVersion:             true,
-		EnableWatchCache:          options.EnableWatchCache,
 		ExternalHost:              options.ExternalHost,
 		KubernetesServiceNodePort: options.KubernetesServiceNodePort,
 		MasterCount:               options.MasterCount,
@@ -315,12 +309,6 @@ func (c Config) New() (*GenericAPIServer, error) {
 		openAPIDefinitions:     c.OpenAPIDefinitions,
 	}
 
-	if c.EnableWatchCache {
-		s.storageDecorator = registry.StorageWithCacher
-	} else {
-		s.storageDecorator = generic.UndecoratedStorage
-	}
-
 	if c.RestfulContainer != nil {
 		s.HandlerContainer = c.RestfulContainer
 	} else {

pkg/genericapiserver/genericapiserver.go

@@ -45,7 +45,6 @@ import (
 	"k8s.io/kubernetes/pkg/genericapiserver/openapi"
 	"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
 	"k8s.io/kubernetes/pkg/genericapiserver/options"
-	"k8s.io/kubernetes/pkg/registry/generic"
 	"k8s.io/kubernetes/pkg/runtime"
 	certutil "k8s.io/kubernetes/pkg/util/cert"
 	utilnet "k8s.io/kubernetes/pkg/util/net"
@@ -120,11 +119,6 @@ type GenericAPIServer struct {
 	// requestContextMapper provides a way to get the context for a request.  It may be nil.
 	requestContextMapper api.RequestContextMapper
 
-	// storageDecorator provides a decoration function for storage.  It will never be nil.
-	// TODO: this may be an abstraction at the wrong layer.  It doesn't seem like a genericAPIServer
-	// should be determining the backing storage for the RESTStorage interfaces
-	storageDecorator generic.StorageDecorator
-
 	Mux              *apiserver.PathRecorderMux
 	HandlerContainer *restful.Container
 	MasterCount      int
@@ -175,10 +169,6 @@ type GenericAPIServer struct {
 	openAPIDefinitions   *common.OpenAPIDefinitions
 }
 
-func (s *GenericAPIServer) StorageDecorator() generic.StorageDecorator {
-	return s.storageDecorator
-}
-
 // RequestContextMapper is exposed so that third party resource storage can be build in a different location.
 // TODO refactor third party resource storage
 func (s *GenericAPIServer) RequestContextMapper() api.RequestContextMapper {
@@ -551,7 +541,6 @@ func (s *GenericAPIServer) getSwaggerConfig() *swagger.Config {
 // register their own web services into the Kubernetes mux prior to initialization
 // of swagger, so that other resource types show up in the documentation.
 func (s *GenericAPIServer) InstallSwaggerAPI() {
-
 	// Enable swagger UI and discovery API
 	swagger.RegisterSwaggerService(*s.getSwaggerConfig(), s.HandlerContainer)
 }

pkg/master/master.go

@@ -83,6 +83,7 @@ import (
 	"k8s.io/kubernetes/pkg/registry/extensions/thirdpartyresourcedata"
 	thirdpartyresourcedataetcd "k8s.io/kubernetes/pkg/registry/extensions/thirdpartyresourcedata/etcd"
 	"k8s.io/kubernetes/pkg/registry/generic"
+	"k8s.io/kubernetes/pkg/registry/generic/registry"
 	rbacstorage "k8s.io/kubernetes/pkg/registry/rbac/storage"
 	"k8s.io/kubernetes/pkg/routes"
 	"k8s.io/kubernetes/pkg/runtime"
@@ -103,6 +104,8 @@ const (
 type Config struct {
 	*genericapiserver.Config
 
+	StorageFactory           genericapiserver.StorageFactory
+	EnableWatchCache         bool
 	EnableCoreControllers    bool
 	EndpointReconcilerConfig EndpointReconcilerConfig
 	DeleteCollectionWorkers  int
@@ -155,6 +158,8 @@ type Master struct {
 
 	// Used to start and monitor tunneling
 	tunneler genericapiserver.Tunneler
+
+	restOptionsFactory restOptionsFactory
 }
 
 // thirdPartyEntry combines objects storage and API group into one struct
@@ -201,6 +206,18 @@ func New(c *Config) (*Master, error) {
 		tunneler:                c.Tunneler,
 
 		disableThirdPartyControllerForTesting: c.disableThirdPartyControllerForTesting,
+
+		restOptionsFactory: restOptionsFactory{
+			deleteCollectionWorkers: c.DeleteCollectionWorkers,
+			enableGarbageCollection: c.EnableGarbageCollection,
+			storageFactory:          c.StorageFactory,
+		},
+	}
+
+	if c.EnableWatchCache {
+		m.restOptionsFactory.storageDecorator = registry.StorageWithCacher
+	} else {
+		m.restOptionsFactory.storageDecorator = generic.UndecoratedStorage
 	}
 
 	// Add some hardcoded storage for now.  Append to the map.
@@ -287,7 +304,7 @@ func (m *Master) InstallAPIs(c *Config) {
 	}
 
 	restOptionsGetter := func(resource unversioned.GroupResource) generic.RESTOptions {
-		return m.GetRESTOptionsOrDie(c, resource)
+		return m.restOptionsFactory.NewFor(resource)
 	}
 
 	// stabilize order.
@@ -338,7 +355,7 @@ func (m *Master) InstallAPIs(c *Config) {
 
 func (m *Master) initV1ResourcesStorage(c *Config) {
 	restOptions := func(resource string) generic.RESTOptions {
-		return m.GetRESTOptionsOrDie(c, api.Resource(resource))
+		return m.restOptionsFactory.NewFor(api.Resource(resource))
 	}
 
 	podTemplateStorage := podtemplateetcd.NewREST(restOptions("podTemplates"))
@@ -778,18 +795,25 @@ func (m *Master) thirdpartyapi(group, kind, version, pluralResource string) *api
 	}
 }
 
-func (m *Master) GetRESTOptionsOrDie(c *Config, resource unversioned.GroupResource) generic.RESTOptions {
+type restOptionsFactory struct {
-	storageConfig, err := c.StorageFactory.NewConfig(resource)
+	deleteCollectionWorkers int
+	enableGarbageCollection bool
+	storageFactory          genericapiserver.StorageFactory
+	storageDecorator        generic.StorageDecorator
+}
+
+func (f restOptionsFactory) NewFor(resource unversioned.GroupResource) generic.RESTOptions {
+	storageConfig, err := f.storageFactory.NewConfig(resource)
 	if err != nil {
 		glog.Fatalf("Unable to find storage destination for %v, due to %v", resource, err.Error())
 	}
 
 	return generic.RESTOptions{
 		StorageConfig:           storageConfig,
-		Decorator:               m.StorageDecorator(),
+		Decorator:               f.storageDecorator,
-		DeleteCollectionWorkers: m.deleteCollectionWorkers,
+		DeleteCollectionWorkers: f.deleteCollectionWorkers,
-		EnableGarbageCollection: c.Config.EnableGarbageCollection,
+		EnableGarbageCollection: f.enableGarbageCollection,
-		ResourcePrefix:          c.StorageFactory.ResourcePrefix(resource),
+		ResourcePrefix:          f.storageFactory.ResourcePrefix(resource),
 	}
 }
 

test/integration/framework/master_utils.go

@@ -224,14 +224,12 @@ func NewMasterConfig() *master.Config {
 
 	return &master.Config{
 		Config: &genericapiserver.Config{
-			StorageFactory:          storageFactory,
 			APIResourceConfigSource: master.DefaultAPIResourceConfigSource(),
 			APIPrefix:               "/api",
 			APIGroupPrefix:          "/apis",
 			Authorizer:              authorizer.NewAlwaysAllowAuthorizer(),
 			AdmissionControl:        admit.NewAlwaysAdmit(),
 			Serializer:              api.Codecs,
-			EnableWatchCache:        true,
 			// Set those values to avoid annoying warnings in logs.
 			ServiceClusterIPRange: parseCIDROrDie("10.0.0.0/24"),
 			ServiceNodePortRange:  utilnet.PortRange{Base: 30000, Size: 2768},
@@ -239,7 +237,9 @@ func NewMasterConfig() *master.Config {
 			OpenAPIDefinitions:    openapi.OpenAPIDefinitions,
 			EnableOpenAPISupport:  true,
 		},
-		KubeletClient: kubeletclient.FakeKubeletClient{},
+		StorageFactory:   storageFactory,
+		EnableWatchCache: true,
+		KubeletClient:    kubeletclient.FakeKubeletClient{},
 	}
 }