Wire etcd prefix to storage and call complete with options (#1394)

Author: pmorie

cmd/apiserver/app/server/etcd_options.go

@@ -18,7 +18,6 @@ package server
 
 import (
 	"github.com/kubernetes-incubator/service-catalog/pkg/api"
-	"github.com/pborman/uuid"
 	"github.com/spf13/pflag"
 	genericserveroptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/apiserver/pkg/storage/storagebackend"
@@ -32,10 +31,18 @@ type EtcdOptions struct {
 	*genericserveroptions.EtcdOptions
 }
 
+const (
+	// DefaultEtcdPathPrefix is the default prefix that is prepended to all
+	// resource paths in etcd.  It is intended to allow an operator to
+	// differentiate the storage of different API servers from one another in
+	// a single etcd.
+	DefaultEtcdPathPrefix = "/registry"
+)
+
 // NewEtcdOptions creates a new, empty, EtcdOptions instance
 func NewEtcdOptions() *EtcdOptions {
 	return &EtcdOptions{
-		EtcdOptions: genericserveroptions.NewEtcdOptions(storagebackend.NewDefaultConfig(uuid.New(), api.Scheme, nil)),
+		EtcdOptions: genericserveroptions.NewEtcdOptions(storagebackend.NewDefaultConfig(DefaultEtcdPathPrefix, api.Scheme, nil)),
 	}
 }
 

cmd/apiserver/app/server/server.go

@@ -24,7 +24,6 @@ import (
 	"github.com/golang/glog"
 	"github.com/kubernetes-incubator/service-catalog/pkg"
 	"github.com/kubernetes-incubator/service-catalog/pkg/kubernetes/pkg/util/interrupt"
-	"github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server"
 	"github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/broker/authsarcheck"
 	"github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/namespace/lifecycle"
 	siclifecycle "github.com/kubernetes-incubator/service-catalog/plugin/pkg/admission/servicebindings/lifecycle"
@@ -40,14 +39,6 @@ const (
 	// k8s core API server.
 	certDirectory = "/var/run/kubernetes-service-catalog"
 
-	// I made this up to match some existing paths. I am not sure if there
-	// are any restrictions on the format or structure beyond text
-	// separated by slashes.
-	etcdPathPrefix = "/k8s.io/service-catalog"
-
-	// GroupName I made this up. Maybe we'll need it.
-	GroupName = "service-catalog.k8s.io"
-
 	storageTypeFlagName = "storageType"
 )
 
@@ -94,14 +85,6 @@ func NewCommandServer(
 		glog.Fatalf("invalid storage type '%s' (%s)", storageType, err)
 		return nil, err
 	}
-	if storageType == server.StorageTypeEtcd {
-		glog.Infof("using etcd for storage")
-		// Store resources in etcd under our special prefix
-		opts.EtcdOptions.StorageConfig.Prefix = etcdPathPrefix
-	} else {
-		// This should never happen, catch for potential bugs
-		panic("Unexpected storage type: " + storageType)
-	}
 
 	cmd.Run = func(c *cobra.Command, args []string) {
 		h := interrupt.New(nil, func() {

pkg/apiserver/etcd_config.go

@@ -88,6 +88,8 @@ func (c completedEtcdConfig) NewServer() (*ServiceCatalogAPIServer, error) {
 
 	roFactory := etcdRESTOptionsFactory{
 		deleteCollectionWorkers: c.extraConfig.deleteCollectionWorkers,
+		// TODO https://github.com/kubernetes/kubernetes/issues/44507
+		// we still need to enable it so finalizers are respected.
 		enableGarbageCollection: true,
 		storageFactory:          c.extraConfig.storageFactory,
 		storageDecorator:        generic.UndecoratedStorage,

pkg/apiserver/etcd_rest_options_factory.go

@@ -45,6 +45,6 @@ func (f etcdRESTOptionsFactory) GetRESTOptions(resource schema.GroupResource) (g
 		Decorator:               f.storageDecorator,
 		DeleteCollectionWorkers: f.deleteCollectionWorkers,
 		EnableGarbageCollection: f.enableGarbageCollection,
-		ResourcePrefix:          f.storageFactory.ResourcePrefix(resource),
+		ResourcePrefix:          resource.Group + "/" + resource.Resource,
 	}, nil
 }

pkg/registry/servicecatalog/binding/storage.go

@@ -139,6 +139,11 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage, error) {
 		DestroyFunc: dFunc,
 	}
 
+	options := &generic.StoreOptions{RESTOptions: opts.EtcdOptions.RESTOptions, AttrFunc: GetAttrs}
+	if err := store.CompleteWithOptions(options); err != nil {
+		panic(err) // TODO: Propagate error up
+	}
+
 	statusStore := store
 	statusStore.UpdateStrategy = bindingStatusUpdateStrategy
 

pkg/registry/servicecatalog/broker/storage.go

@@ -139,6 +139,11 @@ func NewStorage(opts server.Options) (brokers, brokersStatus rest.Storage) {
 		DestroyFunc: dFunc,
 	}
 
+	options := &generic.StoreOptions{RESTOptions: opts.EtcdOptions.RESTOptions, AttrFunc: GetAttrs}
+	if err := store.CompleteWithOptions(options); err != nil {
+		panic(err) // TODO: Propagate error up
+	}
+
 	statusStore := store
 	statusStore.UpdateStrategy = brokerStatusUpdateStrategy
 

pkg/registry/servicecatalog/instance/storage.go

@@ -137,6 +137,10 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage, rest.Storage)
 		Storage:     storageInterface,
 		DestroyFunc: dFunc,
 	}
+	options := &generic.StoreOptions{RESTOptions: opts.EtcdOptions.RESTOptions, AttrFunc: GetAttrs}
+	if err := store.CompleteWithOptions(options); err != nil {
+		panic(err) // TODO: Propagate error up
+	}
 
 	statusStore := store
 	statusStore.UpdateStrategy = instanceStatusUpdateStrategy

pkg/registry/servicecatalog/rest/storage_servicecatalog_test.go

@@ -36,7 +36,8 @@ type GetRESTOptionsHelper struct {
 
 func (g GetRESTOptionsHelper) GetRESTOptions(resource schema.GroupResource) (generic.RESTOptions, error) {
 	return generic.RESTOptions{
-		StorageConfig: &storagebackend.Config{},
+		ResourcePrefix: resource.Group + "/" + resource.Resource,
+		StorageConfig:  &storagebackend.Config{},
 		Decorator: generic.StorageDecorator(func(
 			copier runtime.ObjectCopier,
 			config *storagebackend.Config,

pkg/registry/servicecatalog/serviceclass/storage.go

@@ -146,6 +146,11 @@ func NewStorage(opts server.Options) (rest.Storage, rest.Storage) {
 		DestroyFunc:    dFunc,
 	}
 
+	options := &generic.StoreOptions{RESTOptions: opts.EtcdOptions.RESTOptions, AttrFunc: GetAttrs}
+	if err := store.CompleteWithOptions(options); err != nil {
+		panic(err) // TODO: Propagate error up
+	}
+
 	statusStore := store
 	statusStore.UpdateStrategy = serviceClassStatusUpdateStrategy