Apply review comments

Author: kawych

pkg/apiserver/apiserver.go

@@ -27,8 +27,8 @@ import (
 	genericapiserver "k8s.io/apiserver/pkg/server"
 
 	"github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider"
-	cm_install "k8s.io/metrics/pkg/apis/custom_metrics/install"
-	em_install "k8s.io/metrics/pkg/apis/external_metrics/install"
+	cminstall "k8s.io/metrics/pkg/apis/custom_metrics/install"
+	eminstall "k8s.io/metrics/pkg/apis/external_metrics/install"
 )
 
 var (
@@ -39,8 +39,8 @@ var (
 )
 
 func init() {
-	cm_install.Install(groupFactoryRegistry, registry, Scheme)
-	em_install.Install(groupFactoryRegistry, registry, Scheme)
+	cminstall.Install(groupFactoryRegistry, registry, Scheme)
+	eminstall.Install(groupFactoryRegistry, registry, Scheme)
 
 	// we need to add the options to empty v1
 	// TODO fix the server code to avoid this
@@ -63,8 +63,9 @@ type Config struct {
 
 // CustomMetricsAdapterServer contains state for a Kubernetes cluster master/api server.
 type CustomMetricsAdapterServer struct {
-	GenericAPIServer *genericapiserver.GenericAPIServer
-	Provider         provider.MetricsProvider
+	GenericAPIServer        *genericapiserver.GenericAPIServer
+	customMetricsProvider   provider.CustomMetricsProvider
+	externalMetricsProvider provider.ExternalMetricsProvider
 }
 
 type completedConfig struct {
@@ -82,23 +83,26 @@ func (c *Config) Complete() completedConfig {
 
 // New returns a new instance of CustomMetricsAdapterServer from the given config.
 // name is used to differentiate for logging.
-func (c completedConfig) New(name string, metricsProvider provider.MetricsProvider, enableCustomMetricsAPI bool, enableExternalMetricsAPI bool) (*CustomMetricsAdapterServer, error) {
+// Each of the arguments: customMetricsProvider, externalMetricsProvider can be set either to
+// a provider implementation, or to nil to disable one of the APIs.
+func (c completedConfig) New(name string, customMetricsProvider provider.CustomMetricsProvider, externalMetricsProvider provider.ExternalMetricsProvider) (*CustomMetricsAdapterServer, error) {
 	genericServer, err := c.CompletedConfig.New(name, genericapiserver.EmptyDelegate) // completion is done in Complete, no need for a second time
 	if err != nil {
 		return nil, err
 	}
 
 	s := &CustomMetricsAdapterServer{
-		GenericAPIServer: genericServer,
-		Provider:         metricsProvider,
+		GenericAPIServer:        genericServer,
+		customMetricsProvider:   customMetricsProvider,
+		externalMetricsProvider: externalMetricsProvider,
 	}
 
-	if enableCustomMetricsAPI {
+	if customMetricsProvider != nil {
 		if err := s.InstallCustomMetricsAPI(); err != nil {
 			return nil, err
 		}
 	}
-	if enableExternalMetricsAPI {
+	if externalMetricsProvider != nil {
 		if err := s.InstallExternalMetricsAPI(); err != nil {
 			return nil, err
 		}

pkg/apiserver/cmapis.go

@@ -62,7 +62,7 @@ func (s *CustomMetricsAdapterServer) InstallCustomMetricsAPI() error {
 }
 
 func (s *CustomMetricsAdapterServer) cmAPI(groupMeta *apimachinery.GroupMeta, groupVersion *schema.GroupVersion) *specificapi.MetricsAPIGroupVersion {
-	resourceStorage := metricstorage.NewREST(s.Provider)
+	resourceStorage := metricstorage.NewREST(s.customMetricsProvider)
 
 	return &specificapi.MetricsAPIGroupVersion{
 		DynamicStorage: resourceStorage,
@@ -85,7 +85,7 @@ func (s *CustomMetricsAdapterServer) cmAPI(groupMeta *apimachinery.GroupMeta, gr
 			OptionsExternalVersion: &schema.GroupVersion{Version: "v1"},
 		},
 
-		ResourceLister: provider.NewCustomMetricResourceLister(s.Provider),
+		ResourceLister: provider.NewCustomMetricResourceLister(s.customMetricsProvider),
 		Handlers:       &specificapi.CMHandlers{},
 	}
 }

pkg/apiserver/emapis.go

@@ -63,7 +63,7 @@ func (s *CustomMetricsAdapterServer) InstallExternalMetricsAPI() error {
 }
 
 func (s *CustomMetricsAdapterServer) emAPI(groupMeta *apimachinery.GroupMeta, groupVersion *schema.GroupVersion) *specificapi.MetricsAPIGroupVersion {
-	resourceStorage := metricstorage.NewREST(s.Provider)
+	resourceStorage := metricstorage.NewREST(s.externalMetricsProvider)
 
 	return &specificapi.MetricsAPIGroupVersion{
 		DynamicStorage: resourceStorage,
@@ -85,7 +85,7 @@ func (s *CustomMetricsAdapterServer) emAPI(groupMeta *apimachinery.GroupMeta, gr
 			MinRequestTimeout:      s.GenericAPIServer.MinRequestTimeout(),
 			OptionsExternalVersion: &schema.GroupVersion{Version: "v1"},
 		},
-		ResourceLister: provider.NewExternalMetricResourceLister(s.Provider),
+		ResourceLister: provider.NewExternalMetricResourceLister(s.externalMetricsProvider),
 		Handlers:       &specificapi.EMHandlers{},
 	}
 }

pkg/apiserver/installer/installer.go

@@ -56,10 +56,10 @@ type MetricsAPIGroupVersion struct {
 
 	ResourceLister discovery.APIResourceLister
 
-	Handlers Handlers
+	Handlers apiHandlers
 }
 
-type Handlers interface {
+type apiHandlers interface {
 	registerResourceHandlers(a *MetricsAPIInstaller, ws *restful.WebService) error
 }
 
@@ -103,7 +103,7 @@ type MetricsAPIInstaller struct {
 	group             *MetricsAPIGroupVersion
 	prefix            string // Path prefix where API resources are to be registered.
 	minRequestTimeout time.Duration
-	handlers          Handlers
+	handlers          apiHandlers
 
 	// TODO: do we want to embed a normal API installer here so we can serve normal
 	// endpoints side by side with dynamic ones (from the same API group)?

pkg/sample-cmd/server/start.go

@@ -116,8 +116,17 @@ func (o SampleAdapterServerOptions) RunCustomMetricsAdapterServer(stopCh <-chan
 	}
 
 	metricsProvider := provider.NewFakeProvider(clientPool, dynamicMapper)
+	customMetricsProvider := metricsProvider
+	externalMetricsProvider := metricsProvider
+	if !o.EnableCustomMetricsAPI {
+		customMetricsProvider = nil
+	}
+	if !o.EnableExternalMetricsAPI {
+		externalMetricsProvider = nil
+	}
 
-	server, err := config.Complete().New("sample-custom-metrics-adapter", metricsProvider, o.EnableCustomMetricsAPI, o.EnableExternalMetricsAPI)
+	// In this example, the same provider implements both Custom Metrics API and External Metrics API
+	server, err := config.Complete().New("sample-custom-metrics-adapter", customMetricsProvider, externalMetricsProvider)
 	if err != nil {
 		return err
 	}