From 1bcea54104cb7f53e58924dd5413cf4ba7ceb587 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 6 Sep 2017 18:06:18 +0200 Subject: [PATCH] apiserver: make config completion structural recursion --- pkg/master/controller.go | 2 +- pkg/master/master.go | 34 +++++++++------- .../pkg/apiserver/apiserver.go | 31 +++++++------- .../src/k8s.io/apiserver/pkg/server/config.go | 15 +++---- .../apiserver/pkg/server/config_test.go | 2 +- .../pkg/server/genericapiserver_test.go | 6 +-- .../pkg/apiserver/apiserver.go | 40 ++++++++++--------- .../pkg/apiserver/apiserver.go | 26 ++++++------ .../sample-apiserver/pkg/cmd/server/start.go | 5 +-- 9 files changed, 87 insertions(+), 74 deletions(-) diff --git a/pkg/master/controller.go b/pkg/master/controller.go index 085687b13ca..feb347237a9 100644 --- a/pkg/master/controller.go +++ b/pkg/master/controller.go @@ -77,7 +77,7 @@ type Controller struct { } // NewBootstrapController returns a controller for watching the core capabilities of the master -func (c *Config) NewBootstrapController(legacyRESTStorage corerest.LegacyRESTStorage, serviceClient coreclient.ServicesGetter, nsClient coreclient.NamespacesGetter) *Controller { +func (c *completedConfig) NewBootstrapController(legacyRESTStorage corerest.LegacyRESTStorage, serviceClient coreclient.ServicesGetter, nsClient coreclient.NamespacesGetter) *Controller { return &Controller{ ServiceClient: serviceClient, NamespaceClient: nsClient, diff --git a/pkg/master/master.go b/pkg/master/master.go index ba3365de675..184d10a282b 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -138,6 +138,16 @@ type Config struct { ExtraConfig ExtraConfig } +type completedConfig struct { + GenericConfig genericapiserver.CompletedConfig + ExtraConfig *ExtraConfig +} + +type CompletedConfig struct { + // Embed a private pointer that cannot be instantiated outside of this package. + *completedConfig +} + // EndpointReconcilerConfig holds the endpoint reconciler and endpoint reconciliation interval to be // used by the master. type EndpointReconcilerConfig struct { @@ -152,13 +162,12 @@ type Master struct { ClientCARegistrationHook ClientCARegistrationHook } -type completedConfig struct { - *Config -} - // Complete fills in any fields not set that are required to have valid data. It's mutating the receiver. -func (c *Config) Complete() completedConfig { - c.GenericConfig.Complete() +func (cfg *Config) Complete() CompletedConfig { + c := completedConfig{ + cfg.GenericConfig.Complete(), + &cfg.ExtraConfig, + } serviceIPRange, apiServerServiceIP, err := DefaultServiceIPRange(c.ExtraConfig.ServiceIPRange) if err != nil { @@ -201,12 +210,7 @@ func (c *Config) Complete() completedConfig { // this has always been hardcoded true in the past c.GenericConfig.EnableMetrics = true - return completedConfig{c} -} - -// SkipComplete provides a way to construct a server instance without config completion. -func (c *Config) SkipComplete() completedConfig { - return completedConfig{c} + return CompletedConfig{&c} } // New returns a new instance of Master from the given config. @@ -218,7 +222,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) return nil, fmt.Errorf("Master.New() called with empty config.KubeletClientConfig") } - s, err := c.Config.GenericConfig.SkipComplete().New("kube-apiserver", delegationTarget) // completion is done in Complete, no need for a second time + s, err := c.GenericConfig.New("kube-apiserver", delegationTarget) if err != nil { return nil, err } @@ -245,7 +249,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) ServiceNodePortRange: c.ExtraConfig.ServiceNodePortRange, LoopbackClientConfig: c.GenericConfig.LoopbackClientConfig, } - m.InstallLegacyAPI(c.Config, c.Config.GenericConfig.RESTOptionsGetter, legacyRESTStorageProvider) + m.InstallLegacyAPI(&c, c.GenericConfig.RESTOptionsGetter, legacyRESTStorageProvider) } // The order here is preserved in discovery. @@ -284,7 +288,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) return m, nil } -func (m *Master) InstallLegacyAPI(c *Config, restOptionsGetter generic.RESTOptionsGetter, legacyRESTStorageProvider corerest.LegacyRESTStorageProvider) { +func (m *Master) InstallLegacyAPI(c *completedConfig, restOptionsGetter generic.RESTOptionsGetter, legacyRESTStorageProvider corerest.LegacyRESTStorageProvider) { legacyRESTStorage, apiGroupInfo, err := legacyRESTStorageProvider.NewLegacyRESTStorage(restOptionsGetter) if err != nil { glog.Fatalf("Error building core storage: %v", err) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go index b5b659a643b..e98f20d1c97 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go @@ -81,11 +81,20 @@ func init() { type ExtraConfig struct { CRDRESTOptionsGetter genericregistry.RESTOptionsGetter } + type Config struct { GenericConfig *genericapiserver.Config ExtraConfig ExtraConfig } +type completedConfig struct { + GenericConfig genericapiserver.CompletedConfig + ExtraConfig *ExtraConfig +} + +type CompletedConfig struct { + // Embed a private pointer that cannot be instantiated outside of this package. + *completedConfig } type CustomResourceDefinitions struct { @@ -95,31 +104,25 @@ type CustomResourceDefinitions struct { Informers internalinformers.SharedInformerFactory } -type completedConfig struct { - *Config -} - // Complete fills in any fields not set that are required to have valid data. It's mutating the receiver. -func (c *Config) Complete() completedConfig { - c.GenericConfig.EnableDiscovery = false - c.GenericConfig.Complete() +func (cfg *Config) Complete() CompletedConfig { + c := completedConfig{ + cfg.GenericConfig.Complete(), + &cfg.ExtraConfig, + } + c.GenericConfig.EnableDiscovery = false c.GenericConfig.Version = &version.Info{ Major: "0", Minor: "1", } - return completedConfig{c} -} - -// SkipComplete provides a way to construct a server instance without config completion. -func (c *Config) SkipComplete() completedConfig { - return completedConfig{c} + return CompletedConfig{&c} } // New returns a new instance of CustomResourceDefinitions from the given config. func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) (*CustomResourceDefinitions, error) { - genericServer, err := c.Config.GenericConfig.SkipComplete().New("apiextensions-apiserver", delegationTarget) // completion is done in Complete, no need for a second time + genericServer, err := c.GenericConfig.New("apiextensions-apiserver", delegationTarget) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index f3b37518238..062c5b3359d 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -62,6 +62,7 @@ import ( certutil "k8s.io/client-go/util/cert" openapicommon "k8s.io/kube-openapi/pkg/common" + // install apis _ "k8s.io/apiserver/pkg/apis/apiserver/install" ) @@ -302,9 +303,14 @@ type completedConfig struct { *Config } +type CompletedConfig struct { + // Embed a private pointer that cannot be instantiated outside of this package. + *completedConfig +} + // Complete fills in any fields not set that are required to have valid data and can be derived // from other fields. If you're going to `ApplyOptions`, do that first. It's mutating the receiver. -func (c *Config) Complete() completedConfig { +func (c *Config) Complete() CompletedConfig { if len(c.ExternalAddress) == 0 && c.PublicAddress != nil { hostAndPort := c.PublicAddress.String() if c.ReadWritePort != 0 { @@ -379,12 +385,7 @@ func (c *Config) Complete() completedConfig { c.RequestInfoResolver = NewRequestInfoResolver(c) } - return completedConfig{c} -} - -// SkipComplete provides a way to construct a server instance without config completion. -func (c *Config) SkipComplete() completedConfig { - return completedConfig{c} + return CompletedConfig{&completedConfig{c}} } // New creates a new server which logically combines the handling chain with the passed server. diff --git a/staging/src/k8s.io/apiserver/pkg/server/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/config_test.go index c94046d0b8e..8fd48f1fc68 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config_test.go @@ -52,7 +52,7 @@ func TestNewWithDelegate(t *testing.T) { return fmt.Errorf("delegate failed healthcheck") })) - delegateServer, err := delegateConfig.SkipComplete().New("test", EmptyDelegate) + delegateServer, err := delegateConfig.Complete().New("test", EmptyDelegate) if err != nil { t.Fatal(err) } diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go index 215960a678e..e74fbf10fb8 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go @@ -147,7 +147,7 @@ func TestInstallAPIGroups(t *testing.T) { config.LegacyAPIGroupPrefixes = sets.NewString("/apiPrefix") config.DiscoveryAddresses = discovery.DefaultAddresses{DefaultAddress: "ExternalAddress"} - s, err := config.SkipComplete().New("test", EmptyDelegate) + s, err := config.Complete().New("test", EmptyDelegate) if err != nil { t.Fatalf("Error in bringing up the server: %v", err) } @@ -351,7 +351,7 @@ func TestCustomHandlerChain(t *testing.T) { called = true }) - s, err := config.SkipComplete().New("test", EmptyDelegate) + s, err := config.Complete().New("test", EmptyDelegate) if err != nil { t.Fatalf("Error in bringing up the server: %v", err) } @@ -406,7 +406,7 @@ func TestNotRestRoutesHaveAuth(t *testing.T) { kubeVersion := fakeVersion() config.Version = &kubeVersion - s, err := config.SkipComplete().New("test", EmptyDelegate) + s, err := config.Complete().New("test", EmptyDelegate) if err != nil { t.Fatalf("Error in bringing up the server: %v", err) } diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go index bdf55a9688f..1fff050c193 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go @@ -88,10 +88,20 @@ type ExtraConfig struct { } type Config struct { - GenericConfig *genericapiserver.RecommendedConfig + GenericConfig *genericapiserver.Config ExtraConfig ExtraConfig } +type completedConfig struct { + GenericConfig genericapiserver.CompletedConfig + ExtraConfig *ExtraConfig +} + +type CompletedConfig struct { + // Embed a private pointer that cannot be instantiated outside of this package. + *completedConfig +} + // APIAggregator contains state for a Kubernetes cluster master/api server. type APIAggregator struct { GenericAPIServer *genericapiserver.GenericAPIServer @@ -124,41 +134,35 @@ type APIAggregator struct { openAPIAggregationController *openapicontroller.AggregationController } -type completedConfig struct { - *Config -} - // Complete fills in any fields not set that are required to have valid data. It's mutating the receiver. -func (c *Config) Complete() completedConfig { +func (cfg *Config) Complete() CompletedConfig { + c := completedConfig{ + cfg.GenericConfig.Complete(), + &cfg.ExtraConfig, + } + // the kube aggregator wires its own discovery mechanism // TODO eventually collapse this by extracting all of the discovery out c.GenericConfig.EnableDiscovery = false - c.GenericConfig.Complete() - version := version.Get() c.GenericConfig.Version = &version - return completedConfig{c} -} - -// SkipComplete provides a way to construct a server instance without config completion. -func (c *Config) SkipComplete() completedConfig { - return completedConfig{c} + return CompletedConfig{&c} } // New returns a new instance of APIAggregator from the given config. func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.DelegationTarget) (*APIAggregator, error) { // Prevent generic API server to install OpenAPI handler. Aggregator server // has its own customized OpenAPI handler. - openApiConfig := c.Config.GenericConfig.OpenAPIConfig - c.Config.GenericConfig.OpenAPIConfig = nil + openApiConfig := c.GenericConfig.OpenAPIConfig + c.GenericConfig.OpenAPIConfig = nil - genericServer, err := c.Config.GenericConfig.SkipComplete().New("kube-aggregator", delegationTarget) // completion is done in Complete, no need for a second time + genericServer, err := c.GenericConfig.New("kube-aggregator", delegationTarget) if err != nil { return nil, err } - apiregistrationClient, err := internalclientset.NewForConfig(c.Config.GenericConfig.LoopbackClientConfig) + apiregistrationClient, err := internalclientset.NewForConfig(c.GenericConfig.LoopbackClientConfig) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go b/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go index 32b3acfa2c1..c6c0f36ccf4 100644 --- a/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go @@ -67,8 +67,6 @@ type ExtraConfig struct { type Config struct { GenericConfig *genericapiserver.Config - // SharedInformerFactory provides shared informers for resources - SharedInformerFactory informers.SharedInformerFactory ExtraConfig ExtraConfig } @@ -78,29 +76,33 @@ type WardleServer struct { } type completedConfig struct { - *Config + GenericConfig genericapiserver.CompletedConfig + ExtraConfig *ExtraConfig +} + +type CompletedConfig struct { + // Embed a private pointer that cannot be instantiated outside of this package. + *completedConfig } // Complete fills in any fields not set that are required to have valid data. It's mutating the receiver. -func (c *Config) Complete() completedConfig { - c.GenericConfig.Complete() +func (cfg *Config) Complete() CompletedConfig { + c := completedConfig{ + cfg.GenericConfig.Complete(), + &cfg.ExtraConfig, + } c.GenericConfig.Version = &version.Info{ Major: "1", Minor: "0", } - return completedConfig{c} -} - -// SkipComplete provides a way to construct a server instance without config completion. -func (c *Config) SkipComplete() completedConfig { - return completedConfig{c} + return CompletedConfig{&c} } // New returns a new instance of WardleServer from the given config. func (c completedConfig) New() (*WardleServer, error) { - genericServer, err := c.Config.GenericConfig.SkipComplete().New("sample-apiserver", genericapiserver.EmptyDelegate) // completion is done in Complete, no need for a second time + genericServer, err := c.GenericConfig.New("sample-apiserver", genericapiserver.EmptyDelegate) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go index 69b2aad9871..7ebc41e187e 100644 --- a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go +++ b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go @@ -124,8 +124,7 @@ func (o WardleServerOptions) Config() (*apiserver.Config, error) { } config := &apiserver.Config{ - GenericConfig: serverConfig, - SharedInformerFactory: informerFactory, + GenericConfig: serverConfig, ExtraConfig: apiserver.ExtraConfig{}, } return config, nil @@ -143,7 +142,7 @@ func (o WardleServerOptions) RunWardleServer(stopCh <-chan struct{}) error { } server.GenericAPIServer.AddPostStartHook("start-sample-server-informers", func(context genericapiserver.PostStartHookContext) error { - config.SharedInformerFactory.Start(context.StopCh) + config.GenericConfig.SharedInformerFactory.Start(context.StopCh) return nil })