From 62b9605da7e8e9b3d081c03c8afc46c9d0536d20 Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Fri, 28 Sep 2018 16:34:59 +0100 Subject: [PATCH] Deprecate storage-type flag (#2266) --- .../templates/apiserver-deployment.yaml | 4 - cmd/apiserver/app/server/options.go | 32 ++- cmd/apiserver/app/server/run_server.go | 13 +- cmd/apiserver/app/server/server.go | 32 --- cmd/service-catalog/server/apiserver.go | 2 +- contrib/hack/start-server.sh | 2 +- pkg/apiserver/etcd_config.go | 3 +- pkg/apiserver/util.go | 6 +- .../rest/storage_servicecatalog.go | 9 - .../rest/storage_servicecatalog_test.go | 2 - pkg/registry/servicecatalog/server/options.go | 96 ++------- .../servicecatalog/server/options_test.go | 39 ---- .../settings/rest/storage_settings.go | 2 - test/integration/clientset_test.go | 186 ++++++------------ test/integration/controller_test.go | 5 +- test/integration/framework.go | 21 +- test/integration/podpreset_test.go | 31 ++- 17 files changed, 111 insertions(+), 374 deletions(-) delete mode 100644 cmd/apiserver/app/server/server.go delete mode 100644 pkg/registry/servicecatalog/server/options_test.go diff --git a/charts/catalog/templates/apiserver-deployment.yaml b/charts/catalog/templates/apiserver-deployment.yaml index bec671c939f..24a4e9eefa4 100644 --- a/charts/catalog/templates/apiserver-deployment.yaml +++ b/charts/catalog/templates/apiserver-deployment.yaml @@ -44,12 +44,8 @@ spec: - "NamespaceLifecycle,DefaultServicePlan,ServiceBindingsLifecycle,ServicePlanChangeValidator,BrokerAuthSarCheck" - --secure-port - "8443" - - --storage-type - - {{ .Values.apiserver.storage.type }} - {{- if eq .Values.apiserver.storage.type "etcd" }} - --etcd-servers - {{ .Values.apiserver.storage.etcd.servers }} - {{- end }} - -v - "{{ .Values.apiserver.verbosity }}" {{- if .Values.apiserver.tls.requestHeaderCA }} diff --git a/cmd/apiserver/app/server/options.go b/cmd/apiserver/app/server/options.go index e5f8ba9e862..05c2ededff6 100644 --- a/cmd/apiserver/app/server/options.go +++ b/cmd/apiserver/app/server/options.go @@ -23,8 +23,6 @@ import ( "github.com/spf13/pflag" utilerrors "k8s.io/apimachinery/pkg/util/errors" genericserveroptions "k8s.io/apiserver/pkg/server/options" - - "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" ) const ( @@ -32,14 +30,13 @@ const ( // k8s core API server. certDirectory = "/var/run/kubernetes-service-catalog" - storageTypeFlagName = "storageType" + storageTypeFlagName = "storage-type" ) // ServiceCatalogServerOptions contains the aggregation of configuration structs for // the service-catalog server. It contains everything needed to configure a basic API server. // It is public so that integration tests can access it. type ServiceCatalogServerOptions struct { - StorageTypeString string // the runtime configuration of our server GenericServerRunOptions *genericserveroptions.ServerRunOptions // the admission options @@ -86,12 +83,15 @@ func NewServiceCatalogServerOptions() *ServiceCatalogServerOptions { // AddFlags adds to the flag set the flags to configure the API Server. func (s *ServiceCatalogServerOptions) AddFlags(flags *pflag.FlagSet) { - flags.StringVar( - &s.StorageTypeString, - "storage-type", - "etcd", + // storage-type flag is deprecated so let's mark it as so but keep it visible in usage + // to make it more obvious that it will be removed in the near future. + _ = flags.String( + storageTypeFlagName, + "", "The type of backing storage this API server should use", ) + flags.MarkDeprecated(storageTypeFlagName, "The value of this flag is now unused and will be removed in the near future") + flags.Lookup(storageTypeFlagName).Hidden = false flags.BoolVar( &s.DisableAuth, @@ -122,12 +122,6 @@ func (s *ServiceCatalogServerOptions) AddFlags(flags *pflag.FlagSet) { s.AuditOptions.AddFlags(flags) } -// StorageType returns the storage type configured on s, or a non-nil error if s holds an -// invalid storage type -func (s *ServiceCatalogServerOptions) StorageType() (server.StorageType, error) { - return server.StorageTypeFromString(s.StorageTypeString) -} - // Validate checks all subOptions flags have been set and that they // have not been set in a conflictory manner. func (s *ServiceCatalogServerOptions) Validate() error { @@ -139,13 +133,11 @@ func (s *ServiceCatalogServerOptions) Validate() error { errors = append(errors, s.AuthenticationOptions.Validate()...) errors = append(errors, s.AuthorizationOptions.Validate()...) // etcd options - if "etcd" == s.StorageTypeString { - etcdErrs := s.EtcdOptions.Validate() - if len(etcdErrs) > 0 { - glog.Errorln("Error validating etcd options, do you have `--etcd-servers localhost` set?") - } - errors = append(errors, etcdErrs...) + etcdErrs := s.EtcdOptions.Validate() + if len(etcdErrs) > 0 { + glog.Errorln("Error validating etcd options, do you have `--etcd-servers localhost` set?") } + errors = append(errors, etcdErrs...) // TODO add alternative storage validation // errors = append(errors, s.CRDOptions.Validate()...) // TODO uncomment after 1.8 rebase expecting diff --git a/cmd/apiserver/app/server/run_server.go b/cmd/apiserver/app/server/run_server.go index 7381a620cab..921cd8b40da 100644 --- a/cmd/apiserver/app/server/run_server.go +++ b/cmd/apiserver/app/server/run_server.go @@ -27,31 +27,22 @@ import ( "github.com/golang/glog" "github.com/kubernetes-incubator/service-catalog/pkg/apiserver" "github.com/kubernetes-incubator/service-catalog/pkg/apiserver/options" - registryserver "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" ) // RunServer runs an API server with configuration according to opts func RunServer(opts *ServiceCatalogServerOptions, stopCh <-chan struct{}) error { - storageType, err := opts.StorageType() - if err != nil { - return err - } if stopCh == nil { /* the caller of RunServer should generate the stop channel if there is a need to stop the API server */ stopCh = make(chan struct{}) } - err = opts.Validate() + err := opts.Validate() if nil != err { return err } - if storageType == registryserver.StorageTypeEtcd { - return runEtcdServer(opts, stopCh) - } - // This should never happen, catch for potential bugs - panic("Unexpected storage type: " + storageType) + return runEtcdServer(opts, stopCh) } func runEtcdServer(opts *ServiceCatalogServerOptions, stopCh <-chan struct{}) error { diff --git a/cmd/apiserver/app/server/server.go b/cmd/apiserver/app/server/server.go deleted file mode 100644 index d00ec69d4b0..00000000000 --- a/cmd/apiserver/app/server/server.go +++ /dev/null @@ -1,32 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package server - -import ( - "github.com/golang/glog" -) - -// Run runs the specified APIServer. This should never exit. -func Run(opts *ServiceCatalogServerOptions, stopCh <-chan struct{}) error { - storageType, err := opts.StorageType() - if err != nil { - glog.Fatalf("invalid storage type '%s' (%s)", storageType, err) - return err - } - - return RunServer(opts, stopCh) -} diff --git a/cmd/service-catalog/server/apiserver.go b/cmd/service-catalog/server/apiserver.go index a901fa7e691..502ad763f05 100644 --- a/cmd/service-catalog/server/apiserver.go +++ b/cmd/service-catalog/server/apiserver.go @@ -32,7 +32,7 @@ func NewAPIServer() *hyperkube.Server { SimpleUsage: "apiserver", Long: "The main API entrypoint and interface to the storage system. The API server is also the focal point for all authorization decisions.", Run: func(_ *hyperkube.Server, args []string, stopCh <-chan struct{}) error { - return server.Run(s, stopCh) + return server.RunServer(s, stopCh) }, RespectsStopCh: true, } diff --git a/contrib/hack/start-server.sh b/contrib/hack/start-server.sh index 1a65a0d1876..a8c04b152a6 100755 --- a/contrib/hack/start-server.sh +++ b/contrib/hack/start-server.sh @@ -45,7 +45,7 @@ docker run -d --name apiserver \ --net container:etcd-svc-cat \ scbuildimage \ bin/service-catalog apiserver -v 10 --logtostderr --etcd-servers http://localhost:2379 \ - --storage-type=etcd --disable-auth \ + --disable-auth \ --feature-gates "PodPreset=true" \ --feature-gates "NamespacedServiceBroker=true" \ --feature-gates "ServicePlanDefaults=true" diff --git a/pkg/apiserver/etcd_config.go b/pkg/apiserver/etcd_config.go index 1697a9cd074..33cf4da3088 100644 --- a/pkg/apiserver/etcd_config.go +++ b/pkg/apiserver/etcd_config.go @@ -18,7 +18,6 @@ package apiserver import ( "github.com/golang/glog" - "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/generic/registry" "k8s.io/apiserver/pkg/registry/rest" @@ -99,7 +98,7 @@ func (c completedEtcdConfig) NewServer(stopCh <-chan struct{}) (*ServiceCatalogA glog.V(4).Infoln("Installing API groups") // default namespace doesn't matter for etcd - providers := restStorageProviders("" /* default namespace */, server.StorageTypeEtcd, nil) + providers := restStorageProviders("" /* default namespace */, nil) for _, provider := range providers { groupInfo, err := provider.NewRESTStorage(c.apiResourceConfigSource, roFactory) if IsErrAPIGroupDisabled(err) { diff --git a/pkg/apiserver/util.go b/pkg/apiserver/util.go index f45d410bc19..8d4f5ce87e7 100644 --- a/pkg/apiserver/util.go +++ b/pkg/apiserver/util.go @@ -19,7 +19,6 @@ package apiserver import ( "github.com/kubernetes-incubator/service-catalog/pkg/api" servicecatalogrest "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/rest" - "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" settingsrest "github.com/kubernetes-incubator/service-catalog/pkg/registry/settings/rest" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/client-go/pkg/version" @@ -32,18 +31,15 @@ const ( func restStorageProviders( defaultNamespace string, - storageType server.StorageType, restClient restclient.Interface, ) []RESTStorageProvider { return []RESTStorageProvider{ servicecatalogrest.StorageProvider{ DefaultNamespace: defaultNamespace, - StorageType: storageType, RESTClient: restClient, }, settingsrest.StorageProvider{ - StorageType: storageType, - RESTClient: restClient, + RESTClient: restClient, }, } } diff --git a/pkg/registry/servicecatalog/rest/storage_servicecatalog.go b/pkg/registry/servicecatalog/rest/storage_servicecatalog.go index e37c77a8c5e..764aa94096b 100644 --- a/pkg/registry/servicecatalog/rest/storage_servicecatalog.go +++ b/pkg/registry/servicecatalog/rest/storage_servicecatalog.go @@ -45,7 +45,6 @@ import ( // the servicecatalog API group. It implements (./pkg/apiserver).RESTStorageProvider type StorageProvider struct { DefaultNamespace string - StorageType server.StorageType RESTClient restclient.Interface } @@ -88,7 +87,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: clusterservicebroker.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) clusterServiceClassRESTOptions, err := restOptionsGetter.GetRESTOptions(servicecatalog.Resource("clusterserviceclasses")) @@ -105,7 +103,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: clusterserviceclass.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) clusterServicePlanRESTOptions, err := restOptionsGetter.GetRESTOptions(servicecatalog.Resource("clusterserviceplans")) @@ -122,7 +119,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: clusterserviceplan.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) instanceClassRESTOptions, err := restOptionsGetter.GetRESTOptions(servicecatalog.Resource("serviceinstances")) @@ -139,7 +135,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: instance.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) bindingClassRESTOptions, err := restOptionsGetter.GetRESTOptions(servicecatalog.Resource("servicebindings")) @@ -156,7 +151,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: binding.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) clusterServiceBrokerStorage, clusterServiceBrokerStatusStorage := clusterservicebroker.NewStorage(*clusterServiceBrokerOpts) @@ -198,7 +192,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: serviceclass.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) serviceBrokerRESTOptions, err := restOptionsGetter.GetRESTOptions(servicecatalog.Resource("servicebrokers")) @@ -216,7 +209,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: servicebroker.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) servicePlanRESTOptions, err := restOptionsGetter.GetRESTOptions(servicecatalog.Resource("serviceplans")) @@ -234,7 +226,6 @@ func (p StorageProvider) v1beta1Storage( GetAttrsFunc: serviceplan.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) serviceClassStorage, serviceClassStatusStorage := serviceclass.NewStorage(*serviceClassOpts) diff --git a/pkg/registry/servicecatalog/rest/storage_servicecatalog_test.go b/pkg/registry/servicecatalog/rest/storage_servicecatalog_test.go index cedac6afebb..5b945925942 100644 --- a/pkg/registry/servicecatalog/rest/storage_servicecatalog_test.go +++ b/pkg/registry/servicecatalog/rest/storage_servicecatalog_test.go @@ -38,7 +38,6 @@ import ( "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/clusterserviceclass" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/clusterserviceplan" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/instance" - "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/servicebroker" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/serviceclass" "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/serviceplan" @@ -83,7 +82,6 @@ var _ = Describe("ensure that our storage types implement the appropriate interf provider := StorageProvider{ DefaultNamespace: "test-default", - StorageType: server.StorageTypeEtcd, RESTClient: nil, } configSource := serverstorage.NewResourceConfig() diff --git a/pkg/registry/servicecatalog/server/options.go b/pkg/registry/servicecatalog/server/options.go index 0cb990722d4..81fd437fecf 100644 --- a/pkg/registry/servicecatalog/server/options.go +++ b/pkg/registry/servicecatalog/server/options.go @@ -18,7 +18,6 @@ package server import ( "context" - "fmt" "github.com/kubernetes-incubator/service-catalog/pkg/storage/etcd" "k8s.io/apimachinery/pkg/runtime" @@ -28,63 +27,18 @@ import ( "k8s.io/apiserver/pkg/storage/storagebackend/factory" ) -type errUnsupportedStorageType struct { - t StorageType -} - -func (e errUnsupportedStorageType) Error() string { - return fmt.Sprintf("unsupported storage type %s", e.t) -} - -// StorageType represents the type of storage a storage interface should use -type StorageType string - -// StorageTypeFromString converts s to a valid StorageType. Returns StorageType("") and a non-nil -// error if s names an invalid or unsupported storage type -func StorageTypeFromString(s string) (StorageType, error) { - switch s { - case StorageTypeEtcd.String(): - return StorageTypeEtcd, nil - default: - return StorageType(""), errUnsupportedStorageType{t: StorageType(s)} - } -} - -func (s StorageType) String() string { - return string(s) -} - -const ( - // StorageTypeEtcd indicates a storage interface should use etcd - StorageTypeEtcd StorageType = "etcd" -) - // Options is the extension of a generic.RESTOptions struct, complete with service-catalog // specific things type Options struct { EtcdOptions etcd.Options - storageType StorageType } // NewOptions returns a new Options with the given parameters func NewOptions( etcdOpts etcd.Options, - sType StorageType, ) *Options { return &Options{ EtcdOptions: etcdOpts, - storageType: sType, - } -} - -// StorageType returns the storage type the rest server should use, or an error if an unsupported -// storage type is indicated -func (o Options) StorageType() (StorageType, error) { - switch o.storageType { - case StorageTypeEtcd: - return o.storageType, nil - default: - return StorageType(""), errUnsupportedStorageType{t: o.storageType} } } @@ -98,17 +52,9 @@ func (o Options) ResourcePrefix() string { // by combining the namespace in the context with the given prefix func (o Options) KeyRootFunc() func(context.Context) string { prefix := o.ResourcePrefix() - sType, err := o.StorageType() - if err != nil { - return nil + return func(ctx context.Context) string { + return registry.NamespaceKeyRootFunc(ctx, prefix) } - if sType == StorageTypeEtcd { - return func(ctx context.Context) string { - return registry.NamespaceKeyRootFunc(ctx, prefix) - } - } - // This should never happen, catch for potential bugs - panic("Unexpected storage type: " + sType) } // KeyFunc returns the appropriate key function for the storage type in o. @@ -116,19 +62,12 @@ func (o Options) KeyRootFunc() func(context.Context) string { // by combining the namespace in the context with the given prefix func (o Options) KeyFunc(namespaced bool) func(context.Context, string) (string, error) { prefix := o.ResourcePrefix() - sType, err := o.StorageType() - if err != nil { - return nil - } - if sType == StorageTypeEtcd { - return func(ctx context.Context, name string) (string, error) { - if namespaced { - return registry.NamespaceKeyFunc(ctx, prefix, name) - } - return registry.NoNamespaceKeyFunc(ctx, prefix, name) + return func(ctx context.Context, name string) (string, error) { + if namespaced { + return registry.NamespaceKeyFunc(ctx, prefix, name) } + return registry.NoNamespaceKeyFunc(ctx, prefix, name) } - panic("Unexpected storage type: " + o.storageType) } // GetStorage returns the storage from the given parameters @@ -140,17 +79,14 @@ func (o Options) GetStorage( getAttrsFunc storage.AttrFunc, trigger storage.TriggerPublisherFunc, ) (storage.Interface, factory.DestroyFunc) { - if o.storageType == StorageTypeEtcd { - etcdRESTOpts := o.EtcdOptions.RESTOptions - return etcdRESTOpts.Decorator( - etcdRESTOpts.StorageConfig, - objectType, - resourcePrefix, - nil, /* keyFunc for decorator -- looks to be unused everywhere */ - newListFunc, - getAttrsFunc, - trigger, - ) - } - panic("Unexpected storage type: " + o.storageType) + etcdRESTOpts := o.EtcdOptions.RESTOptions + return etcdRESTOpts.Decorator( + etcdRESTOpts.StorageConfig, + objectType, + resourcePrefix, + nil, /* keyFunc for decorator -- looks to be unused everywhere */ + newListFunc, + getAttrsFunc, + trigger, + ) } diff --git a/pkg/registry/servicecatalog/server/options_test.go b/pkg/registry/servicecatalog/server/options_test.go deleted file mode 100644 index baa4b726ff8..00000000000 --- a/pkg/registry/servicecatalog/server/options_test.go +++ /dev/null @@ -1,39 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package server - -import ( - "testing" - - "github.com/kubernetes-incubator/service-catalog/pkg/storage/etcd" -) - -func TestNewOptions(t *testing.T) { - origStorageType := StorageTypeEtcd - opts := NewOptions(etcd.Options{}, origStorageType) - retStorageType, err := opts.StorageType() - if err != nil { - t.Fatalf("getting storage type (%s)", err) - } - if origStorageType != retStorageType { - t.Fatalf("expected storage type %s, got %s", - origStorageType, - retStorageType, - ) - } - -} diff --git a/pkg/registry/settings/rest/storage_settings.go b/pkg/registry/settings/rest/storage_settings.go index 3c815e03dc4..dd3a97472e5 100644 --- a/pkg/registry/settings/rest/storage_settings.go +++ b/pkg/registry/settings/rest/storage_settings.go @@ -36,7 +36,6 @@ import ( // the servicecatalog API group. It implements (./pkg/apiserver).RESTStorageProvider type StorageProvider struct { DefaultNamespace string - StorageType server.StorageType RESTClient restclient.Interface } @@ -82,7 +81,6 @@ func (p StorageProvider) v1alpha1Storage( GetAttrsFunc: podpreset.GetAttrs, Trigger: storage.NoTriggerPublisher, }, - p.StorageType, ) version := settingsapiv1alpha1.SchemeGroupVersion diff --git a/test/integration/clientset_test.go b/test/integration/clientset_test.go index e4571050b7d..8c3b285e21b 100644 --- a/test/integration/clientset_test.go +++ b/test/integration/clientset_test.go @@ -27,7 +27,6 @@ import ( "strings" "testing" - "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" // our versioned types @@ -64,10 +63,6 @@ const ( ` ) -var storageTypes = []server.StorageType{ - server.StorageTypeEtcd, -} - // Used for testing binding parameters type bpStruct struct { Foo string `json:"foo"` @@ -76,27 +71,17 @@ type bpStruct struct { // TestGroupVersion is trivial. func TestGroupVersion(t *testing.T) { - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ClusterServiceBroker{} - }) - defer shutdownServer() - if err := testGroupVersion(client); err != nil { - t.Fatal(err) - } - } - } - for _, sType := range storageTypes { - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) - } + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { + return &servicecatalog.ClusterServiceBroker{} + }) + defer shutdownServer() + if err := testGroupVersion(client); err != nil { + t.Fatal(err) } } func TestEtcdHealthCheckerSuccess(t *testing.T) { serverConfig := NewTestServerConfig() - serverConfig.storageType = server.StorageTypeEtcd _, clientconfig, shutdownServer := withConfigGetFreshApiserverAndClient(t, serverConfig) t.Log(clientconfig.Host) tr := &http.Transport{ @@ -135,22 +120,12 @@ func testGroupVersion(client servicecatalogclient.Interface) error { // TestNoName checks that all creates fail for objects that have no // name given. func TestNoName(t *testing.T) { - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ClusterServiceBroker{} - }) - defer shutdownServer() - if err := testNoName(client); err != nil { - t.Fatal(err) - } - } - } - - for _, sType := range storageTypes { - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) - } + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { + return &servicecatalog.ClusterServiceBroker{} + }) + defer shutdownServer() + if err := testNoName(client); err != nil { + t.Fatal(err) } } @@ -189,25 +164,16 @@ func testNoName(client servicecatalogclient.Interface) error { // TestClusterServiceBrokerClient exercises the ClusterServiceBroker client. func TestClusterServiceBrokerClient(t *testing.T) { const name = "test-broker" - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ClusterServiceBroker{} - }) - defer shutdownServer() - if err := testClusterServiceBrokerClient(sType, client, name); err != nil { - t.Fatal(err) - } - } - } - for _, sType := range storageTypes { - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) - } + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { + return &servicecatalog.ClusterServiceBroker{} + }) + defer shutdownServer() + if err := testClusterServiceBrokerClient(client, name); err != nil { + t.Fatal(err) } } -func testClusterServiceBrokerClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { +func testClusterServiceBrokerClient(client servicecatalogclient.Interface, name string) error { brokerClient := client.Servicecatalog().ClusterServiceBrokers() broker := &v1beta1.ClusterServiceBroker{ ObjectMeta: metav1.ObjectMeta{Name: name}, @@ -360,34 +326,27 @@ func testClusterServiceBrokerClient(sType server.StorageType, client servicecata return fmt.Errorf("broker should be deleted (%v)", brokerDeleted) } return nil -} // TestNamespacedServiceBrokerClient exercises the namespaced ServiceBroker client. +} + +// TestNamespacedServiceBrokerClient exercises the namespaced ServiceBroker client. func TestNamespacedServiceBrokerClient(t *testing.T) { const name = "test-broker" const namespace = "test-namespace" - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - resetFeaturesFunc, err := enableNamespacedResources() - if err != nil { - t.Fatal(err) - } - defer resetFeaturesFunc() - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ClusterServiceBroker{} - }) - defer shutdownServer() - if err := testNamespacedServiceBrokerClient(sType, client, namespace, name); err != nil { - t.Fatal(err) - } - } + resetFeaturesFunc, err := enableNamespacedResources() + if err != nil { + t.Fatal(err) } - for _, sType := range storageTypes { - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) - } + defer resetFeaturesFunc() + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { + return &servicecatalog.ClusterServiceBroker{} + }) + defer shutdownServer() + if err := testNamespacedServiceBrokerClient(client, namespace, name); err != nil { + t.Fatal(err) } } -func testNamespacedServiceBrokerClient(sType server.StorageType, client servicecatalogclient.Interface, namespace, name string) error { +func testNamespacedServiceBrokerClient(client servicecatalogclient.Interface, namespace, name string) error { brokerClient := client.Servicecatalog().ServiceBrokers(namespace) broker := &v1beta1.ServiceBroker{ ObjectMeta: metav1.ObjectMeta{Name: name}, @@ -550,18 +509,17 @@ func testNamespacedServiceBrokerClient(sType server.StorageType, client servicec // TestClusterServiceClassClient exercises the ClusterServiceClass client. func TestClusterServiceClassClient(t *testing.T) { const name = "test-serviceclass" - sType := server.StorageTypeEtcd - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { return &servicecatalog.ClusterServiceClass{} }) defer shutdownServer() - if err := testClusterServiceClassClient(sType, client, name); err != nil { + if err := testClusterServiceClassClient(client, name); err != nil { t.Fatal(err) } } -func testClusterServiceClassClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { +func testClusterServiceClassClient(client servicecatalogclient.Interface, name string) error { serviceClassClient := client.Servicecatalog().ClusterServiceClasses() serviceClass := &v1beta1.ClusterServiceClass{ @@ -760,18 +718,17 @@ func TestNamespacedServiceClassClient(t *testing.T) { t.Fatal(err) } defer resetFeaturesFunc() - sType := server.StorageTypeEtcd - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { return &servicecatalog.ServiceClass{} }) defer shutdownServer() - if err := testNamespacedServiceClassClient(sType, client, namespace, name); err != nil { + if err := testNamespacedServiceClassClient(client, namespace, name); err != nil { t.Fatal(err) } } -func testNamespacedServiceClassClient(sType server.StorageType, client servicecatalogclient.Interface, namespace, name string) error { +func testNamespacedServiceClassClient(client servicecatalogclient.Interface, namespace, name string) error { serviceClassClient := client.Servicecatalog().ServiceClasses(namespace) serviceClass := &v1beta1.ServiceClass{ @@ -973,18 +930,17 @@ func testNamespacedServiceClassClient(sType server.StorageType, client serviceca // TestClusterServicePlanClient exercises the ClusterServicePlan client. func TestClusterServicePlanClient(t *testing.T) { const name = "test-serviceplan" - sType := server.StorageTypeEtcd - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { return &servicecatalog.ClusterServicePlan{} }) defer shutdownServer() - if err := testClusterServicePlanClient(sType, client, name); err != nil { + if err := testClusterServicePlanClient(client, name); err != nil { t.Fatal(err) } } -func testClusterServicePlanClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { +func testClusterServicePlanClient(client servicecatalogclient.Interface, name string) error { servicePlanClient := client.Servicecatalog().ClusterServicePlans() bindable := true @@ -1192,18 +1148,17 @@ func TestNamespacedServicePlanClient(t *testing.T) { t.Fatal(err) } defer resetFeaturesFunc() - sType := server.StorageTypeEtcd - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { return &servicecatalog.ServicePlan{} }) defer shutdownServer() - if err := testNamespacedServicePlanClient(sType, client, namespace, name); err != nil { + if err := testNamespacedServicePlanClient(client, namespace, name); err != nil { t.Fatal(err) } } -func testNamespacedServicePlanClient(sType server.StorageType, client servicecatalogclient.Interface, namespace, name string) error { +func testNamespacedServicePlanClient(client servicecatalogclient.Interface, namespace, name string) error { servicePlanClient := client.Servicecatalog().ServicePlans(namespace) bindable := true @@ -1413,26 +1368,17 @@ func testNamespacedServicePlanClient(sType server.StorageType, client servicecat // TestInstanceClient exercises the Instance client. func TestInstanceClient(t *testing.T) { - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - const name = "test-instance" - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ServiceInstance{} - }) - defer shutdownServer() - if err := testInstanceClient(sType, client, name); err != nil { - t.Fatal(err) - } - } - } - for _, sType := range storageTypes { - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) - } + const name = "test-instance" + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { + return &servicecatalog.ServiceInstance{} + }) + defer shutdownServer() + if err := testInstanceClient(client, name); err != nil { + t.Fatal(err) } } -func testInstanceClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { +func testInstanceClient(client servicecatalogclient.Interface, name string) error { const ( osbGUID = "9737b6ed-ca95-4439-8219-c53fcad118ab" dashboardURL = "http://test-dashboard.example.com" @@ -1684,28 +1630,18 @@ func testInstanceClient(sType server.StorageType, client servicecatalogclient.In // TestBindingClient exercises the Binding client. func TestBindingClient(t *testing.T) { - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - const name = "test-binding" - client, _, shutdownServer := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &servicecatalog.ServiceBinding{} - }) - defer shutdownServer() - - if err := testBindingClient(sType, client, name); err != nil { - t.Fatal(err) - } - } - } - for _, sType := range storageTypes { - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%q test failed", sType) - } + const name = "test-binding" + client, _, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { + return &servicecatalog.ServiceBinding{} + }) + defer shutdownServer() + if err := testBindingClient(client, name); err != nil { + t.Fatal(err) } } -func testBindingClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { +func testBindingClient(client servicecatalogclient.Interface, name string) error { bindingClient := client.Servicecatalog().ServiceBindings("test-namespace") binding := &v1beta1.ServiceBinding{ diff --git a/test/integration/controller_test.go b/test/integration/controller_test.go index 4cce0f4c80f..1c27e1b40ce 100644 --- a/test/integration/controller_test.go +++ b/test/integration/controller_test.go @@ -51,7 +51,6 @@ import ( informers "github.com/kubernetes-incubator/service-catalog/pkg/client/informers_generated/externalversions/servicecatalog/v1beta1" "github.com/kubernetes-incubator/service-catalog/pkg/controller" scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" - "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" "github.com/kubernetes-incubator/service-catalog/test/util" ) @@ -752,7 +751,7 @@ func newControllerTestTestController(ct *controllerTest) ( fakeKubeClient.Unlock() // create an sc client and running server - catalogClient, catalogClientConfig, shutdownServer := getFreshApiserverAndClient(t, server.StorageTypeEtcd.String(), func() runtime.Object { + catalogClient, catalogClientConfig, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { return &servicecatalog.ClusterServiceBroker{} }) @@ -912,7 +911,7 @@ func newTestController(t *testing.T) ( fakeKubeClient.Unlock() // create an sc client and running server - catalogClient, catalogClientConfig, shutdownServer := getFreshApiserverAndClient(t, server.StorageTypeEtcd.String(), func() runtime.Object { + catalogClient, catalogClientConfig, shutdownServer := getFreshApiserverAndClient(t, func() runtime.Object { return &servicecatalog.ClusterServiceBroker{} }) diff --git a/test/integration/framework.go b/test/integration/framework.go index 0de05a6a9e1..e8669dd899c 100644 --- a/test/integration/framework.go +++ b/test/integration/framework.go @@ -40,7 +40,6 @@ import ( _ "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/install" _ "github.com/kubernetes-incubator/service-catalog/pkg/apis/settings/install" servicecatalogclient "github.com/kubernetes-incubator/service-catalog/pkg/client/clientset_generated/clientset" - serverstorage "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" "k8s.io/apimachinery/pkg/runtime" ) @@ -52,7 +51,6 @@ func init() { type TestServerConfig struct { etcdServerList []string - storageType serverstorage.StorageType emptyObjFunc func() runtime.Object } @@ -77,15 +75,10 @@ func withConfigGetFreshApiserverAndClient( secureServingOptions := genericserveroptions.NewSecureServingOptions() var etcdOptions *server.EtcdOptions - if serverstorage.StorageTypeEtcd == serverConfig.storageType { - etcdOptions = server.NewEtcdOptions() - etcdOptions.StorageConfig.ServerList = serverConfig.etcdServerList - etcdOptions.EtcdOptions.StorageConfig.Prefix = fmt.Sprintf("%s-%08X", server.DefaultEtcdPathPrefix, rand.Int31()) - } else { - t.Log("no storage type specified") - } + etcdOptions = server.NewEtcdOptions() + etcdOptions.StorageConfig.ServerList = serverConfig.etcdServerList + etcdOptions.EtcdOptions.StorageConfig.Prefix = fmt.Sprintf("%s-%08X", server.DefaultEtcdPathPrefix, rand.Int31()) options := &server.ServiceCatalogServerOptions{ - StorageTypeString: serverConfig.storageType.String(), GenericServerRunOptions: genericserveroptions.NewServerRunOptions(), AdmissionOptions: genericserveroptions.NewAdmissionOptions(), SecureServingOptions: genericserveroptions.WithLoopback(secureServingOptions), @@ -145,18 +138,10 @@ func withConfigGetFreshApiserverAndClient( func getFreshApiserverAndClient( t *testing.T, - storageTypeStr string, newEmptyObj func() runtime.Object, ) (servicecatalogclient.Interface, *restclient.Config, func()) { - var serverStorageType serverstorage.StorageType - serverStorageType, err := serverstorage.StorageTypeFromString(storageTypeStr) - if nil != err { - t.Fatal("non supported storage type") - } - serverConfig := &TestServerConfig{ etcdServerList: []string{"http://localhost:2379"}, - storageType: serverStorageType, emptyObjFunc: newEmptyObj, } client, clientConfig, shutdownFunc := withConfigGetFreshApiserverAndClient(t, serverConfig) diff --git a/test/integration/podpreset_test.go b/test/integration/podpreset_test.go index c82816e0964..af87fe35dab 100644 --- a/test/integration/podpreset_test.go +++ b/test/integration/podpreset_test.go @@ -22,7 +22,6 @@ import ( "testing" "github.com/kubernetes-incubator/service-catalog/pkg/features" - "github.com/kubernetes-incubator/service-catalog/pkg/registry/servicecatalog/server" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -39,24 +38,16 @@ func TestPodPresetClient(t *testing.T) { // enable PodPreset APIs since they aren't enabled by default enablePodPresetFeature() defer disablePodPresetFeature() - rootTestFunc := func(sType server.StorageType) func(t *testing.T) { - return func(t *testing.T) { - const name = "test-podpreset" - - client, _, shutdown := getFreshApiserverAndClient(t, sType.String(), func() runtime.Object { - return &settingsapi.PodPreset{} - }) - defer shutdown() - - if err := testPodPresetClient(sType, client, name); err != nil { - t.Fatal(err) - } - } - } - for _, sType := range []server.StorageType{server.StorageTypeEtcd} { - if !t.Run(sType.String(), rootTestFunc(sType)) { - t.Errorf("%s test failed", sType) - } + + const name = "test-podpreset" + + client, _, shutdown := getFreshApiserverAndClient(t, func() runtime.Object { + return &settingsapi.PodPreset{} + }) + defer shutdown() + + if err := testPodPresetClient(client, name); err != nil { + t.Fatal(err) } } @@ -68,7 +59,7 @@ func disablePodPresetFeature() { utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", features.PodPreset)) } -func testPodPresetClient(sType server.StorageType, client servicecatalogclient.Interface, name string) error { +func testPodPresetClient(client servicecatalogclient.Interface, name string) error { testNamespace := "test-namespace" podPresetName := "test-podpreset"