Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

apiserver: further cleanup of apiserver storage plumbing #41313

Merged
merged 4 commits into from
Feb 15, 2017
Merged

apiserver: further cleanup of apiserver storage plumbing #41313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd41389
k8s.io/apiserver: move server storage code into subpackage
sttts Feb 11, 2017
27e01b5
k8s.io/apiserver: fixup imports and renamed packages
sttts Feb 11, 2017
5e77d01
k8s.io/apiserver: straighten EtcdOptions, backend Config and kube RES…
sttts Feb 11, 2017
0e9c1f7
Update bazel
sttts Feb 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/kube-apiserver/app/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ go_library(
"//vendor:k8s.io/apiserver/pkg/admission",
"//vendor:k8s.io/apiserver/pkg/server",
"//vendor:k8s.io/apiserver/pkg/server/filters",
"//vendor:k8s.io/apiserver/pkg/server/storage",
],
)

Expand Down
1 change: 1 addition & 0 deletions cmd/kube-apiserver/app/options/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ go_library(
"//vendor:github.com/spf13/pflag",
"//vendor:k8s.io/apimachinery/pkg/util/net",
"//vendor:k8s.io/apiserver/pkg/server/options",
"//vendor:k8s.io/apiserver/pkg/storage/storagebackend",
],
)

Expand Down
3 changes: 2 additions & 1 deletion cmd/kube-apiserver/app/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

utilnet "k8s.io/apimachinery/pkg/util/net"
genericoptions "k8s.io/apiserver/pkg/server/options"
"k8s.io/apiserver/pkg/storage/storagebackend"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/validation"
kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options"
Expand Down Expand Up @@ -68,7 +69,7 @@ type ServerRunOptions struct {
func NewServerRunOptions() *ServerRunOptions {
s := ServerRunOptions{
GenericServerRunOptions: genericoptions.NewServerRunOptions(),
Etcd: genericoptions.NewEtcdOptions(kubeoptions.DefaultEtcdPathPrefix, api.Scheme, nil),
Etcd: genericoptions.NewEtcdOptions(storagebackend.NewDefaultConfig(kubeoptions.DefaultEtcdPathPrefix, api.Scheme, nil)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really like keep the arguments to NewRecommendedOptions to be: "Where do you want it and what's your scheme". Even if we construct this under the covers. I haven't gotten to the point to see if you plumbed it through that way or not, I just thought of it here.

Copy link
Contributor Author

@sttts sttts Feb 14, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that. We construct the storagebackend config behind the scenes and use it from EtcdOptions to create RESTOptionsFactories. How do you want to customize the backend? Patching the config after creating the EtcdOptions is dirty, especially if you pass prefix, scheme and codec before. Using NewDefaultConfig is using clean composition. What's the issue with that?

SecureServing: genericoptions.NewSecureServingOptions(),
InsecureServing: genericoptions.NewInsecureServingOptions(),
Audit: genericoptions.NewAuditLogOptions(),
Expand Down
13 changes: 6 additions & 7 deletions cmd/kube-apiserver/app/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"k8s.io/apiserver/pkg/admission"
genericapiserver "k8s.io/apiserver/pkg/server"
"k8s.io/apiserver/pkg/server/filters"
serverstorage "k8s.io/apiserver/pkg/server/storage"
"k8s.io/kubernetes/cmd/kube-apiserver/app/options"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/apis/batch"
Expand Down Expand Up @@ -203,9 +204,9 @@ func Run(s *options.ServerRunOptions) error {
if err != nil {
return fmt.Errorf("error generating storage version map: %s", err)
}
storageFactory, err := kubeapiserver.BuildDefaultStorageFactory(
storageFactory, err := kubeapiserver.NewStorageFactory(
s.Etcd.StorageConfig, s.Etcd.DefaultStorageMediaType, api.Codecs,
genericapiserver.NewDefaultResourceEncodingConfig(api.Registry), storageGroupsToEncodingVersion,
serverstorage.NewDefaultResourceEncodingConfig(api.Registry), storageGroupsToEncodingVersion,
// FIXME: this GroupVersionResource override should be configurable
[]schema.GroupVersionResource{batch.Resource("cronjobs").WithVersion("v2alpha1")},
master.DefaultAPIResourceConfigSource(), s.APIEnablement.RuntimeConfig)
Expand Down Expand Up @@ -308,11 +309,9 @@ func Run(s *options.ServerRunOptions) error {
sets.NewString("watch", "proxy"),
sets.NewString("attach", "exec", "proxy", "log", "portforward"),
)
genericConfig.RESTOptionsGetter = &kubeapiserver.RESTOptionsFactory{
StorageFactory: storageFactory,
EnableWatchCache: s.Etcd.EnableWatchCache,
EnableGarbageCollection: s.Etcd.EnableGarbageCollection,
DeleteCollectionWorkers: s.Etcd.DeleteCollectionWorkers,

if err := s.Etcd.ApplyWithStorageFactoryTo(storageFactory, genericConfig); err != nil {
return err
}

config := &master.Config{
Expand Down
1 change: 1 addition & 0 deletions examples/apiserver/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ go_library(
"//vendor:k8s.io/apiserver/pkg/registry/rest",
"//vendor:k8s.io/apiserver/pkg/server",
"//vendor:k8s.io/apiserver/pkg/server/options",
"//vendor:k8s.io/apiserver/pkg/server/storage",
"//vendor:k8s.io/apiserver/pkg/storage/storagebackend",
],
)
Expand Down
34 changes: 14 additions & 20 deletions examples/apiserver/apiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ package apiserver
import (
"fmt"

"github.com/golang/glog"

"k8s.io/apimachinery/pkg/runtime/schema"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apiserver/pkg/authorization/authorizerfactory"
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
genericoptions "k8s.io/apiserver/pkg/server/options"
serverstorage "k8s.io/apiserver/pkg/server/storage"
"k8s.io/apiserver/pkg/storage/storagebackend"
"k8s.io/kubernetes/cmd/libs/go2idl/client-gen/test_apis/testgroup/v1"
testgroupetcd "k8s.io/kubernetes/examples/apiserver/rest"
Expand All @@ -34,8 +37,6 @@ import (

// Install the testgroup API
_ "k8s.io/kubernetes/cmd/libs/go2idl/client-gen/test_apis/testgroup/install"

"github.com/golang/glog"
)

const (
Expand All @@ -45,17 +46,6 @@ const (
SecurePort = 6444
)

func newStorageFactory() genericapiserver.StorageFactory {
config := storagebackend.Config{
Prefix: kubeoptions.DefaultEtcdPathPrefix,
ServerList: []string{"http://127.0.0.1:2379"},
Copier: api.Scheme,
}
storageFactory := genericapiserver.NewDefaultStorageFactory(config, "application/json", api.Codecs, genericapiserver.NewDefaultResourceEncodingConfig(api.Registry), genericapiserver.NewResourceConfig())

return storageFactory
}

type ServerRunOptions struct {
GenericServerRunOptions *genericoptions.ServerRunOptions
Etcd *genericoptions.EtcdOptions
Expand All @@ -68,14 +58,15 @@ type ServerRunOptions struct {
func NewServerRunOptions() *ServerRunOptions {
s := ServerRunOptions{
GenericServerRunOptions: genericoptions.NewServerRunOptions(),
Etcd: genericoptions.NewEtcdOptions(kubeoptions.DefaultEtcdPathPrefix, api.Scheme, nil),
Etcd: genericoptions.NewEtcdOptions(storagebackend.NewDefaultConfig(kubeoptions.DefaultEtcdPathPrefix, api.Scheme, nil)),
SecureServing: genericoptions.NewSecureServingOptions(),
InsecureServing: genericoptions.NewInsecureServingOptions(),
Authentication: kubeoptions.NewBuiltInAuthenticationOptions().WithAll(),
CloudProvider: kubeoptions.NewCloudProviderOptions(),
}
s.InsecureServing.BindPort = InsecurePort
s.SecureServing.ServingOptions.BindPort = SecurePort
s.Etcd.StorageConfig.ServerList = []string{"http://127.0.0.1:2379"}

return &s
}
Expand Down Expand Up @@ -122,22 +113,25 @@ func (serverOptions *ServerRunOptions) Run(stopCh <-chan struct{}) error {
config.Authorizer = authorizerfactory.NewAlwaysAllowAuthorizer()
config.SwaggerConfig = genericapiserver.DefaultSwaggerConfig()

s, err := config.Complete().New()
if err != nil {
return fmt.Errorf("Error in bringing up the server: %v", err)
}

groupVersion := v1.SchemeGroupVersion
groupName := groupVersion.Group
groupMeta, err := api.Registry.Group(groupName)
if err != nil {
return fmt.Errorf("%v", err)
}
storageFactory := newStorageFactory()
storageFactory := serverstorage.NewDefaultStorageFactory(serverOptions.Etcd.StorageConfig, "application/json", api.Codecs, serverstorage.NewDefaultResourceEncodingConfig(api.Registry), serverstorage.NewResourceConfig())
storageConfig, err := storageFactory.NewConfig(schema.GroupResource{Group: groupName, Resource: "testtype"})
if err != nil {
return fmt.Errorf("Unable to get storage config: %v", err)
}
if err := serverOptions.Etcd.ApplyWithStorageFactoryTo(storageFactory, config); err != nil {
return fmt.Errorf("failed to configure authentication: %s", err)
}

s, err := config.Complete().New()
if err != nil {
return fmt.Errorf("Error in bringing up the server: %v", err)
}

testTypeOpts := generic.RESTOptions{
StorageConfig: storageConfig,
Expand Down
1 change: 1 addition & 0 deletions federation/cmd/federation-apiserver/app/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ go_library(
"//vendor:k8s.io/apiserver/pkg/registry/rest",
"//vendor:k8s.io/apiserver/pkg/server",
"//vendor:k8s.io/apiserver/pkg/server/filters",
"//vendor:k8s.io/apiserver/pkg/server/storage",
],
)

Expand Down
1 change: 1 addition & 0 deletions federation/cmd/federation-apiserver/app/options/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ go_library(
"//pkg/kubeapiserver/options:go_default_library",
"//vendor:github.com/spf13/pflag",
"//vendor:k8s.io/apiserver/pkg/server/options",
"//vendor:k8s.io/apiserver/pkg/storage/storagebackend",
],
)

Expand Down
3 changes: 2 additions & 1 deletion federation/cmd/federation-apiserver/app/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

genericoptions "k8s.io/apiserver/pkg/server/options"
"k8s.io/apiserver/pkg/storage/storagebackend"
"k8s.io/kubernetes/pkg/api"
kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options"

Expand Down Expand Up @@ -51,7 +52,7 @@ type ServerRunOptions struct {
func NewServerRunOptions() *ServerRunOptions {
s := ServerRunOptions{
GenericServerRunOptions: genericoptions.NewServerRunOptions(),
Etcd: genericoptions.NewEtcdOptions(kubeoptions.DefaultEtcdPathPrefix, api.Scheme, nil),
Etcd: genericoptions.NewEtcdOptions(storagebackend.NewDefaultConfig(kubeoptions.DefaultEtcdPathPrefix, api.Scheme, nil)),
SecureServing: genericoptions.NewSecureServingOptions(),
InsecureServing: genericoptions.NewInsecureServingOptions(),
Audit: genericoptions.NewAuditLogOptions(),
Expand Down
16 changes: 7 additions & 9 deletions federation/cmd/federation-apiserver/app/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"k8s.io/apiserver/pkg/admission"
genericapiserver "k8s.io/apiserver/pkg/server"
"k8s.io/apiserver/pkg/server/filters"
serverstorage "k8s.io/apiserver/pkg/server/storage"
"k8s.io/kubernetes/federation/cmd/federation-apiserver/app/options"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
Expand Down Expand Up @@ -108,7 +109,7 @@ func Run(s *options.ServerRunOptions) error {
}

// TODO: register cluster federation resources here.
resourceConfig := genericapiserver.NewResourceConfig()
resourceConfig := serverstorage.NewResourceConfig()

if s.Etcd.StorageConfig.DeserializationCacheSize == 0 {
// When size of cache is not explicitly set, set it to 50000
Expand All @@ -118,9 +119,9 @@ func Run(s *options.ServerRunOptions) error {
if err != nil {
return fmt.Errorf("error generating storage version map: %s", err)
}
storageFactory, err := kubeapiserver.BuildDefaultStorageFactory(
storageFactory, err := kubeapiserver.NewStorageFactory(
s.Etcd.StorageConfig, s.Etcd.DefaultStorageMediaType, api.Codecs,
genericapiserver.NewDefaultResourceEncodingConfig(api.Registry), storageGroupsToEncodingVersion,
serverstorage.NewDefaultResourceEncodingConfig(api.Registry), storageGroupsToEncodingVersion,
[]schema.GroupVersionResource{}, resourceConfig, s.APIEnablement.RuntimeConfig)
if err != nil {
return fmt.Errorf("error in initializing storage factory: %s", err)
Expand All @@ -145,6 +146,9 @@ func Run(s *options.ServerRunOptions) error {
servers := strings.Split(tokens[1], ";")
storageFactory.SetEtcdLocation(groupResource, servers)
}
if err := s.Etcd.ApplyWithStorageFactoryTo(storageFactory, genericConfig); err != nil {
return err
}

apiAuthenticator, securityDefinitions, err := s.Authentication.ToAuthenticationConfig().New()
if err != nil {
Expand Down Expand Up @@ -187,12 +191,6 @@ func Run(s *options.ServerRunOptions) error {
sets.NewString("watch", "proxy"),
sets.NewString("attach", "exec", "proxy", "log", "portforward"),
)
genericConfig.RESTOptionsGetter = &kubeapiserver.RESTOptionsFactory{
StorageFactory: storageFactory,
EnableWatchCache: s.Etcd.EnableWatchCache,
EnableGarbageCollection: s.Etcd.EnableGarbageCollection,
DeleteCollectionWorkers: s.Etcd.DeleteCollectionWorkers,
}

// TODO: Move this to generic api server (Need to move the command line flag).
if s.Etcd.EnableWatchCache {
Expand Down
7 changes: 2 additions & 5 deletions pkg/kubeapiserver/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,13 @@ go_library(
srcs = [
"default_storage_factory_builder.go",
"doc.go",
"rest.go",
],
tags = ["automanaged"],
deps = [
"//pkg/api:go_default_library",
"//vendor:k8s.io/apimachinery/pkg/runtime",
"//vendor:k8s.io/apimachinery/pkg/runtime/schema",
"//vendor:k8s.io/apiserver/pkg/registry/generic",
"//vendor:k8s.io/apiserver/pkg/registry/generic/registry",
"//vendor:k8s.io/apiserver/pkg/server",
"//vendor:k8s.io/apiserver/pkg/server/storage",
"//vendor:k8s.io/apiserver/pkg/storage/storagebackend",
"//vendor:k8s.io/apiserver/pkg/util/flag",
],
Expand Down Expand Up @@ -59,6 +56,6 @@ go_test(
"//pkg/apis/extensions/install:go_default_library",
"//pkg/apis/extensions/v1beta1:go_default_library",
"//vendor:k8s.io/apimachinery/pkg/runtime/schema",
"//vendor:k8s.io/apiserver/pkg/server",
"//vendor:k8s.io/apiserver/pkg/server/storage",
],
)
18 changes: 9 additions & 9 deletions pkg/kubeapiserver/default_storage_factory_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,30 +23,30 @@ import (

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
genericapiserver "k8s.io/apiserver/pkg/server"
serverstorage "k8s.io/apiserver/pkg/server/storage"
"k8s.io/apiserver/pkg/storage/storagebackend"
utilflag "k8s.io/apiserver/pkg/util/flag"
"k8s.io/kubernetes/pkg/api"
)

// Builds the DefaultStorageFactory.
// NewStorageFactory builds the DefaultStorageFactory.
// Merges defaultResourceConfig with the user specified overrides and merges
// defaultAPIResourceConfig with the corresponding user specified overrides as well.
func BuildDefaultStorageFactory(storageConfig storagebackend.Config, defaultMediaType string, serializer runtime.StorageSerializer,
defaultResourceEncoding *genericapiserver.DefaultResourceEncodingConfig, storageEncodingOverrides map[string]schema.GroupVersion, resourceEncodingOverrides []schema.GroupVersionResource,
defaultAPIResourceConfig *genericapiserver.ResourceConfig, resourceConfigOverrides utilflag.ConfigurationMap) (*genericapiserver.DefaultStorageFactory, error) {
func NewStorageFactory(storageConfig storagebackend.Config, defaultMediaType string, serializer runtime.StorageSerializer,
defaultResourceEncoding *serverstorage.DefaultResourceEncodingConfig, storageEncodingOverrides map[string]schema.GroupVersion, resourceEncodingOverrides []schema.GroupVersionResource,
defaultAPIResourceConfig *serverstorage.ResourceConfig, resourceConfigOverrides utilflag.ConfigurationMap) (*serverstorage.DefaultStorageFactory, error) {

resourceEncodingConfig := mergeGroupEncodingConfigs(defaultResourceEncoding, storageEncodingOverrides)
resourceEncodingConfig = mergeResourceEncodingConfigs(resourceEncodingConfig, resourceEncodingOverrides)
apiResourceConfig, err := mergeAPIResourceConfigs(defaultAPIResourceConfig, resourceConfigOverrides)
if err != nil {
return nil, err
}
return genericapiserver.NewDefaultStorageFactory(storageConfig, defaultMediaType, serializer, resourceEncodingConfig, apiResourceConfig), nil
return serverstorage.NewDefaultStorageFactory(storageConfig, defaultMediaType, serializer, resourceEncodingConfig, apiResourceConfig), nil
}

// Merges the given defaultResourceConfig with specifc GroupvVersionResource overrides.
func mergeResourceEncodingConfigs(defaultResourceEncoding *genericapiserver.DefaultResourceEncodingConfig, resourceEncodingOverrides []schema.GroupVersionResource) *genericapiserver.DefaultResourceEncodingConfig {
func mergeResourceEncodingConfigs(defaultResourceEncoding *serverstorage.DefaultResourceEncodingConfig, resourceEncodingOverrides []schema.GroupVersionResource) *serverstorage.DefaultResourceEncodingConfig {
resourceEncodingConfig := defaultResourceEncoding
for _, gvr := range resourceEncodingOverrides {
resourceEncodingConfig.SetResourceEncoding(gvr.GroupResource(), gvr.GroupVersion(),
Expand All @@ -56,7 +56,7 @@ func mergeResourceEncodingConfigs(defaultResourceEncoding *genericapiserver.Defa
}

// Merges the given defaultResourceConfig with specifc GroupVersion overrides.
func mergeGroupEncodingConfigs(defaultResourceEncoding *genericapiserver.DefaultResourceEncodingConfig, storageEncodingOverrides map[string]schema.GroupVersion) *genericapiserver.DefaultResourceEncodingConfig {
func mergeGroupEncodingConfigs(defaultResourceEncoding *serverstorage.DefaultResourceEncodingConfig, storageEncodingOverrides map[string]schema.GroupVersion) *serverstorage.DefaultResourceEncodingConfig {
resourceEncodingConfig := defaultResourceEncoding
for group, storageEncodingVersion := range storageEncodingOverrides {
resourceEncodingConfig.SetVersionEncoding(group, storageEncodingVersion, schema.GroupVersion{Group: group, Version: runtime.APIVersionInternal})
Expand All @@ -65,7 +65,7 @@ func mergeGroupEncodingConfigs(defaultResourceEncoding *genericapiserver.Default
}

// Merges the given defaultAPIResourceConfig with the given resourceConfigOverrides.
func mergeAPIResourceConfigs(defaultAPIResourceConfig *genericapiserver.ResourceConfig, resourceConfigOverrides utilflag.ConfigurationMap) (*genericapiserver.ResourceConfig, error) {
func mergeAPIResourceConfigs(defaultAPIResourceConfig *serverstorage.ResourceConfig, resourceConfigOverrides utilflag.ConfigurationMap) (*serverstorage.ResourceConfig, error) {
resourceConfig := defaultAPIResourceConfig
overrides := resourceConfigOverrides

Expand Down