Deprecate storage-type flag (kubernetes-retired#2266)

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 }}

cmd/apiserver/app/server/options.go

@@ -23,23 +23,20 @@ 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 (
 	// Store generated SSL certificates in a place that won't collide with the
 	// 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

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 {

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,
 	}

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"

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) {

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,
 		},
 	}
 }

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)

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()