Merge pull request #33842 from deads2k/api-19-clean-structs

Automatic merge from submit-queue

clean api server cruft

Some cruft has developed over refactors.  Remove that cruft.

@liggitt probably last in the chain so far

cmd/kube-apiserver/app/server.go

@@ -365,6 +365,6 @@ func Run(s *options.APIServer) error {
 	}
 
 	sharedInformers.Start(wait.NeverStop)
-	m.Run()
+	m.GenericAPIServer.Run()
 	return nil
 }

pkg/genericapiserver/config.go

@@ -365,7 +365,6 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 
 	s := &GenericAPIServer{
 		ServiceClusterIPRange: c.ServiceClusterIPRange,
-		ServiceNodePortRange:  c.ServiceNodePortRange,
 		LoopbackClientConfig:  c.LoopbackClientConfig,
 		legacyAPIPrefix:       c.APIPrefix,
 		apiPrefix:             c.APIGroupPrefix,
@@ -381,11 +380,8 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 		InsecureServingInfo:  c.InsecureServingInfo,
 		ExternalAddress:      c.ExternalHost,
 		ClusterIP:            c.PublicAddress,
-		PublicReadWritePort:  c.ReadWritePort,
 		ServiceReadWriteIP:   c.ServiceReadWriteIP,
 		ServiceReadWritePort: c.ServiceReadWritePort,
-		ExtraServicePorts:    c.ExtraServicePorts,
-		ExtraEndpointPorts:   c.ExtraEndpointPorts,
 
 		KubernetesServiceNodePort: c.KubernetesServiceNodePort,
 		apiGroupsForDiscovery:     map[string]unversioned.APIGroup{},

pkg/genericapiserver/genericapiserver.go

@@ -90,10 +90,6 @@ type GenericAPIServer struct {
 	// truth for its value.
 	ServiceClusterIPRange *net.IPNet
 
-	// ServiceNodePortRange is only used for `master.go` to construct its RESTStorage for the legacy API group
-	// TODO refactor this closer to the point of use.
-	ServiceNodePortRange utilnet.PortRange
-
 	// LoopbackClientConfig is a config for a privileged loopback connection to the API server
 	LoopbackClientConfig *restclient.Config
 
@@ -166,11 +162,8 @@ type GenericAPIServer struct {
 	openAPIDefinitions        *common.OpenAPIDefinitions
 	MasterCount               int
 	KubernetesServiceNodePort int // TODO(sttts): move into master
-	PublicReadWritePort       int
 	ServiceReadWriteIP        net.IP
 	ServiceReadWritePort      int
-	ExtraServicePorts         []api.ServicePort
-	ExtraEndpointPorts        []api.EndpointPort
 }
 
 func init() {

pkg/genericapiserver/genericapiserver_test.go

@@ -90,7 +90,6 @@ func TestNew(t *testing.T) {
 	serviceReadWriteIP, _ := ipallocator.GetIndexedIP(serviceClusterIPRange, 1)
 	assert.Equal(s.ServiceReadWriteIP, serviceReadWriteIP)
 	assert.Equal(s.ExternalAddress, net.JoinHostPort(config.PublicAddress.String(), "6443"))
-	assert.Equal(s.PublicReadWritePort, 6443)
 
 	// These functions should point to the same memory location
 	serverDialer, _ := utilnet.Dialer(s.ProxyTransport)
@@ -312,7 +311,6 @@ func TestInstallSwaggerAPI(t *testing.T) {
 	server.HandlerContainer = genericmux.NewAPIContainer(mux, nil)
 	server.ExternalAddress = ""
 	server.ClusterIP = net.IPv4(10, 10, 10, 10)
-	server.PublicReadWritePort = 1010
 	server.InstallSwaggerAPI()
 	if assert.NotEqual(0, len(ws), "SwaggerAPI not installed.") {
 		assert.Equal("/swaggerapi/", ws[0].RootPath(), "SwaggerAPI did not install to the proper path. %s != /swaggerapi", ws[0].RootPath())

pkg/master/master.go

@@ -120,9 +120,7 @@ type EndpointReconcilerConfig struct {
 
 // Master contains state for a Kubernetes cluster master/api server.
 type Master struct {
-	*genericapiserver.GenericAPIServer
+	GenericAPIServer *genericapiserver.GenericAPIServer
-
-	legacyRESTStorageProvider corerest.LegacyRESTStorageProvider
 
 	deleteCollectionWorkers int
 
@@ -217,8 +215,17 @@ func (c completedConfig) New() (*Master, error) {
 			enableGarbageCollection: c.GenericConfig.EnableGarbageCollection,
 			storageFactory:          c.StorageFactory,
 		},
+	}
 
-		legacyRESTStorageProvider: corerest.LegacyRESTStorageProvider{
+	if c.EnableWatchCache {
+		m.restOptionsFactory.storageDecorator = registry.StorageWithCacher
+	} else {
+		m.restOptionsFactory.storageDecorator = generic.UndecoratedStorage
+	}
+
+	// install legacy rest storage
+	// because of other hacks, this always has to come first
+	legacyRESTStorageProvider := corerest.LegacyRESTStorageProvider{
 		StorageFactory:            c.StorageFactory,
 		ProxyTransport:            s.ProxyTransport,
 		KubeletClient:             c.KubeletClient,
@@ -227,13 +234,6 @@ func (c completedConfig) New() (*Master, error) {
 		ServiceNodePortRange:      c.GenericConfig.ServiceNodePortRange,
 		ComponentStatusServerFunc: func() map[string]apiserver.Server { return getServersToValidate(c.StorageFactory) },
 		LoopbackClientConfig:      c.GenericConfig.LoopbackClientConfig,
-		},
-	}
-
-	if c.EnableWatchCache {
-		m.restOptionsFactory.storageDecorator = registry.StorageWithCacher
-	} else {
-		m.restOptionsFactory.storageDecorator = generic.UndecoratedStorage
 	}
 
 	// Add some hardcoded storage for now.  Append to the map.
@@ -253,12 +253,37 @@ func (c completedConfig) New() (*Master, error) {
 	c.RESTStorageProviders[policy.GroupName] = policyrest.RESTStorageProvider{}
 	c.RESTStorageProviders[rbac.GroupName] = &rbacrest.RESTStorageProvider{AuthorizerRBACSuperUser: c.GenericConfig.AuthorizerRBACSuperUser}
 	c.RESTStorageProviders[storage.GroupName] = storagerest.RESTStorageProvider{}
-	m.InstallAPIs(c.Config)
+	m.InstallAPIs(c.Config, legacyRESTStorageProvider)
+
+	m.InstallGeneralEndpoints(c.Config)
 
 	return m, nil
 }
 
-func (m *Master) InstallAPIs(c *Config) {
+// TODO this needs to be refactored so we have a way to add general health checks to genericapiserver
+// TODO profiling should be generic
+func (m *Master) InstallGeneralEndpoints(c *Config) {
+	// Run the tunneler.
+	healthzChecks := []healthz.HealthzChecker{}
+	if c.Tunneler != nil {
+		c.Tunneler.Run(m.getNodeAddresses)
+		healthzChecks = append(healthzChecks, healthz.NamedCheck("SSH Tunnel Check", genericapiserver.TunnelSyncHealthChecker(c.Tunneler)))
+		prometheus.NewGaugeFunc(prometheus.GaugeOpts{
+			Name: "apiserver_proxy_tunnel_sync_latency_secs",
+			Help: "The time since the last successful synchronization of the SSH tunnels for proxy requests.",
+		}, func() float64 { return float64(c.Tunneler.SecondsSinceSync()) })
+	}
+	healthz.InstallHandler(&m.GenericAPIServer.HandlerContainer.NonSwaggerRoutes, healthzChecks...)
+
+	if c.GenericConfig.EnableProfiling {
+		routes.MetricsWithReset{}.Install(m.GenericAPIServer.HandlerContainer)
+	} else {
+		routes.DefaultMetrics{}.Install(m.GenericAPIServer.HandlerContainer)
+	}
+
+}
+
+func (m *Master) InstallAPIs(c *Config, legacyRESTStorageProvider corerest.LegacyRESTStorageProvider) {
 	restOptionsGetter := func(resource unversioned.GroupResource) generic.RESTOptions {
 		return m.restOptionsFactory.NewFor(resource)
 	}
@@ -267,7 +292,7 @@ func (m *Master) InstallAPIs(c *Config) {
 
 	// Install v1 unless disabled.
 	if c.GenericConfig.APIResourceConfigSource.AnyResourcesForVersionEnabled(apiv1.SchemeGroupVersion) {
-		legacyRESTStorage, apiGroupInfo, err := m.legacyRESTStorageProvider.NewLegacyRESTStorage(restOptionsGetter)
+		legacyRESTStorage, apiGroupInfo, err := legacyRESTStorageProvider.NewLegacyRESTStorage(restOptionsGetter)
 		if err != nil {
 			glog.Fatalf("Error building core storage: %v", err)
 		}
@@ -282,24 +307,6 @@ func (m *Master) InstallAPIs(c *Config) {
 		apiGroupsInfo = append(apiGroupsInfo, apiGroupInfo)
 	}
 
-	// Run the tunneler.
-	healthzChecks := []healthz.HealthzChecker{}
-	if c.Tunneler != nil {
-		c.Tunneler.Run(m.getNodeAddresses)
-		healthzChecks = append(healthzChecks, healthz.NamedCheck("SSH Tunnel Check", genericapiserver.TunnelSyncHealthChecker(c.Tunneler)))
-		prometheus.NewGaugeFunc(prometheus.GaugeOpts{
-			Name: "apiserver_proxy_tunnel_sync_latency_secs",
-			Help: "The time since the last successful synchronization of the SSH tunnels for proxy requests.",
-		}, func() float64 { return float64(c.Tunneler.SecondsSinceSync()) })
-	}
-	healthz.InstallHandler(&m.HandlerContainer.NonSwaggerRoutes, healthzChecks...)
-
-	if c.GenericConfig.EnableProfiling {
-		routes.MetricsWithReset{}.Install(m.HandlerContainer)
-	} else {
-		routes.DefaultMetrics{}.Install(m.HandlerContainer)
-	}
-
 	// Install third party resource support if requested
 	// TODO seems like this bit ought to be unconditional and the REST API is controlled by the config
 	if c.GenericConfig.APIResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("thirdpartyresources")) {
@@ -340,7 +347,7 @@ func (m *Master) InstallAPIs(c *Config) {
 	}
 
 	for i := range apiGroupsInfo {
-		if err := m.InstallAPIGroup(&apiGroupsInfo[i]); err != nil {
+		if err := m.GenericAPIServer.InstallAPIGroup(&apiGroupsInfo[i]); err != nil {
 			glog.Fatalf("Error in registering group versions: %v", err)
 		}
 	}
@@ -423,7 +430,7 @@ func (m *Master) removeThirdPartyStorage(path, resource string) error {
 	delete(entry.storage, resource)
 	if len(entry.storage) == 0 {
 		delete(m.thirdPartyResources, path)
-		m.RemoveAPIGroupForDiscovery(extensionsrest.GetThirdPartyGroupName(path))
+		m.GenericAPIServer.RemoveAPIGroupForDiscovery(extensionsrest.GetThirdPartyGroupName(path))
 	} else {
 		m.thirdPartyResources[path] = entry
 	}
@@ -443,11 +450,11 @@ func (m *Master) RemoveThirdPartyResource(path string) error {
 		return err
 	}
 
-	services := m.HandlerContainer.RegisteredWebServices()
+	services := m.GenericAPIServer.HandlerContainer.RegisteredWebServices()
 	for ix := range services {
 		root := services[ix].RootPath()
 		if root == path || strings.HasPrefix(root, path+"/") {
-			m.HandlerContainer.Remove(services[ix])
+			m.GenericAPIServer.HandlerContainer.Remove(services[ix])
 		}
 	}
 	return nil
@@ -523,7 +530,7 @@ func (m *Master) addThirdPartyResourceStorage(path, resource string, storage *th
 	}
 	entry.storage[resource] = storage
 	if !found {
-		m.AddAPIGroupForDiscovery(apiGroup)
+		m.GenericAPIServer.AddAPIGroupForDiscovery(apiGroup)
 	}
 }
 
@@ -562,13 +569,13 @@ func (m *Master) InstallThirdPartyResource(rsrc *extensions.ThirdPartyResource)
 	// the group with the new API
 	if m.hasThirdPartyGroupStorage(path) {
 		m.addThirdPartyResourceStorage(path, plural.Resource, thirdparty.Storage[plural.Resource].(*thirdpartyresourcedataetcd.REST), apiGroup)
-		return thirdparty.UpdateREST(m.HandlerContainer.Container)
+		return thirdparty.UpdateREST(m.GenericAPIServer.HandlerContainer.Container)
 	}
 
-	if err := thirdparty.InstallREST(m.HandlerContainer.Container); err != nil {
+	if err := thirdparty.InstallREST(m.GenericAPIServer.HandlerContainer.Container); err != nil {
 		glog.Errorf("Unable to setup thirdparty api: %v", err)
 	}
-	m.HandlerContainer.Add(apiserver.NewGroupWebService(api.Codecs, path, apiGroup))
+	m.GenericAPIServer.HandlerContainer.Add(apiserver.NewGroupWebService(api.Codecs, path, apiGroup))
 
 	m.addThirdPartyResourceStorage(path, plural.Resource, thirdparty.Storage[plural.Resource].(*thirdpartyresourcedataetcd.REST), apiGroup)
 	return nil
@@ -611,9 +618,9 @@ func (m *Master) thirdpartyapi(group, kind, version, pluralResource string) *api
 		Serializer:     thirdpartyresourcedata.NewNegotiatedSerializer(api.Codecs, kind, externalVersion, internalVersion),
 		ParameterCodec: thirdpartyresourcedata.NewThirdPartyParameterCodec(api.ParameterCodec),
 
-		Context: m.RequestContextMapper(),
+		Context: m.GenericAPIServer.RequestContextMapper(),
 
-		MinRequestTimeout: m.MinRequestTimeout(),
+		MinRequestTimeout: m.GenericAPIServer.MinRequestTimeout(),
 
 		ResourceLister: dynamicLister{m, extensionsrest.MakeThirdPartyPath(group)},
 	}

pkg/master/master_test.go

@@ -161,23 +161,22 @@ func TestNew(t *testing.T) {
 	defer etcdserver.Terminate(t)
 
 	// Verify many of the variables match their config counterparts
-	assert.Equal(master.RequestContextMapper(), config.GenericConfig.RequestContextMapper)
+	assert.Equal(master.GenericAPIServer.RequestContextMapper(), config.GenericConfig.RequestContextMapper)
-	assert.Equal(master.ClusterIP, config.GenericConfig.PublicAddress)
+	assert.Equal(master.GenericAPIServer.ClusterIP, config.GenericConfig.PublicAddress)
 
 	// these values get defaulted
 	_, serviceClusterIPRange, _ := net.ParseCIDR("10.0.0.0/24")
 	serviceReadWriteIP, _ := ipallocator.GetIndexedIP(serviceClusterIPRange, 1)
-	assert.Equal(master.MasterCount, 1)
+	assert.Equal(master.GenericAPIServer.MasterCount, 1)
-	assert.Equal(master.PublicReadWritePort, 6443)
+	assert.Equal(master.GenericAPIServer.ServiceReadWriteIP, serviceReadWriteIP)
-	assert.Equal(master.ServiceReadWriteIP, serviceReadWriteIP)
 
 	// These functions should point to the same memory location
-	masterDialer, _ := utilnet.Dialer(master.ProxyTransport)
+	masterDialer, _ := utilnet.Dialer(master.GenericAPIServer.ProxyTransport)
 	masterDialerFunc := fmt.Sprintf("%p", masterDialer)
 	configDialerFunc := fmt.Sprintf("%p", config.GenericConfig.ProxyDialer)
 	assert.Equal(masterDialerFunc, configDialerFunc)
 
-	assert.Equal(master.ProxyTransport.(*http.Transport).TLSClientConfig, config.GenericConfig.ProxyTLSClientConfig)
+	assert.Equal(master.GenericAPIServer.ProxyTransport.(*http.Transport).TLSClientConfig, config.GenericConfig.ProxyTLSClientConfig)
 }
 
 // TestVersion tests /version
@@ -187,7 +186,7 @@ func TestVersion(t *testing.T) {
 
 	req, _ := http.NewRequest("GET", "/version", nil)
 	resp := httptest.NewRecorder()
-	s.InsecureHandler.ServeHTTP(resp, req)
+	s.GenericAPIServer.InsecureHandler.ServeHTTP(resp, req)
 	if resp.Code != 200 {
 		t.Fatalf("expected http 200, got: %d", resp.Code)
 	}
@@ -302,7 +301,7 @@ func TestAPIVersionOfDiscoveryEndpoints(t *testing.T) {
 	master, etcdserver, _, assert := newMaster(t)
 	defer etcdserver.Terminate(t)
 
-	server := httptest.NewServer(master.HandlerContainer.ServeMux)
+	server := httptest.NewServer(master.GenericAPIServer.HandlerContainer.ServeMux)
 
 	// /api exists in release-1.1
 	resp, err := http.Get(server.URL + "/api")
@@ -376,7 +375,7 @@ func TestDiscoveryAtAPIS(t *testing.T) {
 	master, etcdserver, _, assert := newLimitedMaster(t)
 	defer etcdserver.Terminate(t)
 
-	server := httptest.NewServer(master.HandlerContainer.ServeMux)
+	server := httptest.NewServer(master.GenericAPIServer.HandlerContainer.ServeMux)
 	resp, err := http.Get(server.URL + "/apis")
 	if !assert.NoError(err) {
 		t.Errorf("unexpected error: %v", err)
@@ -515,14 +514,14 @@ func TestValidOpenAPISpec(t *testing.T) {
 	}
 
 	// make sure swagger.json is not registered before calling install api.
-	server := httptest.NewServer(master.HandlerContainer.ServeMux)
+	server := httptest.NewServer(master.GenericAPIServer.HandlerContainer.ServeMux)
 	resp, err := http.Get(server.URL + "/swagger.json")
 	if !assert.NoError(err) {
 		t.Errorf("unexpected error: %v", err)
 	}
 	assert.Equal(http.StatusNotFound, resp.StatusCode)
 
-	master.InstallOpenAPI()
+	master.GenericAPIServer.InstallOpenAPI()
 	resp, err = http.Get(server.URL + "/swagger.json")
 	if !assert.NoError(err) {
 		t.Errorf("unexpected error: %v", err)

test/integration/framework/master_utils.go

@@ -175,7 +175,7 @@ func startMasterOrDie(masterConfig *master.Config, incomingServer *httptest.Serv
 		s = incomingServer
 	} else {
 		s = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-			m.Handler.ServeHTTP(w, req)
+			m.GenericAPIServer.Handler.ServeHTTP(w, req)
 		}))
 	}
 

test/integration/kubectl/kubectl_test.go

@@ -47,7 +47,7 @@ func TestKubectlValidation(t *testing.T) {
 	ctx := clientcmdapi.NewContext()
 	cfg := clientcmdapi.NewConfig()
 	// Enable swagger api on master.
-	components.KubeMaster.InstallSwaggerAPI()
+	components.KubeMaster.GenericAPIServer.InstallSwaggerAPI()
 	cluster := clientcmdapi.NewCluster()
 
 	cluster.Server = components.ApiServer.URL

test/integration/quota/quota_test.go

@@ -58,7 +58,7 @@ func TestQuota(t *testing.T) {
 	h := &framework.MasterHolder{Initialized: make(chan struct{})}
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		<-h.Initialized
-		h.M.Handler.ServeHTTP(w, req)
+		h.M.GenericAPIServer.Handler.ServeHTTP(w, req)
 	}))
 	defer s.Close()
 

test/integration/scheduler_perf/util.go

@@ -51,7 +51,7 @@ func mustSetupScheduler() (schedulerConfigFactory *factory.ConfigFactory, destro
 		panic("error in brining up the master: " + err.Error())
 	}
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		m.Handler.ServeHTTP(w, req)
+		m.GenericAPIServer.Handler.ServeHTTP(w, req)
 	}))
 
 	c := client.NewOrDie(&restclient.Config{

test/integration/serviceaccount/service_account_test.go

@@ -342,7 +342,7 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie
 	h := &framework.MasterHolder{Initialized: make(chan struct{})}
 	apiServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		<-h.Initialized
-		h.M.Handler.ServeHTTP(w, req)
+		h.M.GenericAPIServer.Handler.ServeHTTP(w, req)
 	}))
 
 	// Anonymous client config