Merge pull request #60702 from hzxuzhonghu/kube-apiserver-cleanup

Automatic merge from submit-queue (batch tested with PRs 60363, 59208, 59465, 60581, 60702). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

apiserver clean code

**What this PR does / why we need it**:

1. clean up some redundant code in kube-apiserver startup

1. comment on `preparedGenericAPIServer`, which is just a wrapper of `GenericAPIServer`.



**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

cmd/kube-apiserver/app/apiextensions.go

@@ -61,10 +61,5 @@ func createAPIExtensionsConfig(kubeAPIServerConfig genericapiserver.Config, exte
 }
 
 func createAPIExtensionsServer(apiextensionsConfig *apiextensionsapiserver.Config, delegateAPIServer genericapiserver.DelegationTarget) (*apiextensionsapiserver.CustomResourceDefinitions, error) {
-	apiextensionsServer, err := apiextensionsConfig.Complete().New(delegateAPIServer)
-	if err != nil {
-		return nil, err
-	}
-
-	return apiextensionsServer, nil
+	return apiextensionsConfig.Complete().New(delegateAPIServer)
 }

cmd/kube-apiserver/app/options/validation.go

@@ -50,36 +50,36 @@ func validateServiceNodePort(options *ServerRunOptions) []error {
 }
 
 // Validate checks ServerRunOptions and return a slice of found errors.
-func (options *ServerRunOptions) Validate() []error {
+func (s *ServerRunOptions) Validate() []error {
 	var errors []error
-	if errs := options.Etcd.Validate(); len(errs) > 0 {
+	if errs := s.Etcd.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := validateClusterIPFlags(options); len(errs) > 0 {
+	if errs := validateClusterIPFlags(s); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := validateServiceNodePort(options); len(errs) > 0 {
+	if errs := validateServiceNodePort(s); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := options.SecureServing.Validate(); len(errs) > 0 {
+	if errs := s.SecureServing.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := options.Authentication.Validate(); len(errs) > 0 {
+	if errs := s.Authentication.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := options.Audit.Validate(); len(errs) > 0 {
+	if errs := s.Audit.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := options.Admission.Validate(); len(errs) > 0 {
+	if errs := s.Admission.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if errs := options.InsecureServing.Validate("insecure-port"); len(errs) > 0 {
+	if errs := s.InsecureServing.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
-	if options.MasterCount <= 0 {
-		errors = append(errors, fmt.Errorf("--apiserver-count should be a positive number, but value '%d' provided", options.MasterCount))
+	if s.MasterCount <= 0 {
+		errors = append(errors, fmt.Errorf("--apiserver-count should be a positive number, but value '%d' provided", s.MasterCount))
 	}
-	if errs := options.APIEnablement.Validate(legacyscheme.Registry, apiextensionsapiserver.Registry, aggregatorscheme.Registry); len(errs) > 0 {
+	if errs := s.APIEnablement.Validate(legacyscheme.Registry, apiextensionsapiserver.Registry, aggregatorscheme.Registry); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
 

cmd/kube-apiserver/app/server.go

@@ -27,7 +27,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"reflect"
 	"strconv"
 	"strings"
 	"time"
@@ -154,7 +153,6 @@ func CreateServerChain(runOptions *options.ServerRunOptions, stopCh <-chan struc
 		return nil, err
 	}
 
-	// TPRs are enabled and not yet beta, since this these are the successor, they fall under the same enablement rule
 	// If additional API servers are added, they should be gated.
 	apiExtensionsConfig, err := createAPIExtensionsConfig(*kubeAPIServerConfig.GenericConfig, versionedInformers, runOptions)
 	if err != nil {
@@ -195,8 +193,6 @@ func CreateServerChain(runOptions *options.ServerRunOptions, stopCh <-chan struc
 	if err != nil {
 		return nil, err
 	}
-	aggregatorConfig.ExtraConfig.ProxyTransport = proxyTransport
-	aggregatorConfig.ExtraConfig.ServiceResolver = serviceResolver
 	aggregatorServer, err := createAggregatorServer(aggregatorConfig, kubeAPIServer.GenericAPIServer, apiExtensionsServer.Informers)
 	if err != nil {
 		// we don't need special handling for innerStopCh because the aggregator server doesn't create any go routines
@@ -480,7 +476,7 @@ func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transp
 		)
 	}
 
-	genericConfig.Authentication.Authenticator, genericConfig.OpenAPIConfig.SecurityDefinitions, err = BuildAuthenticator(s, storageFactory, client, clientgoExternalClient, sharedInformers)
+	genericConfig.Authentication.Authenticator, genericConfig.OpenAPIConfig.SecurityDefinitions, err = BuildAuthenticator(s, clientgoExternalClient, sharedInformers)
 	if err != nil {
 		return nil, nil, nil, nil, nil, fmt.Errorf("invalid authentication config: %v", err)
 	}
@@ -564,19 +560,18 @@ func BuildAdmissionPluginInitializers(s *options.ServerRunOptions, client intern
 }
 
 // BuildAuthenticator constructs the authenticator
-func BuildAuthenticator(s *options.ServerRunOptions, storageFactory serverstorage.StorageFactory, client internalclientset.Interface, extclient clientgoclientset.Interface, sharedInformers informers.SharedInformerFactory) (authenticator.Request, *spec.SecurityDefinitions, error) {
+func BuildAuthenticator(s *options.ServerRunOptions, extclient clientgoclientset.Interface, sharedInformers informers.SharedInformerFactory) (authenticator.Request, *spec.SecurityDefinitions, error) {
 	authenticatorConfig := s.Authentication.ToAuthenticationConfig()
 	if s.Authentication.ServiceAccounts.Lookup {
 		authenticatorConfig.ServiceAccountTokenGetter = serviceaccountcontroller.NewGetterFromClient(extclient)
 	}
-	if client == nil || reflect.ValueOf(client).IsNil() {
-		// TODO: Remove check once client can never be nil.
-		glog.Errorf("Failed to setup bootstrap token authenticator because the loopback clientset was not setup properly.")
-	} else {
+	kubeAPIVersions := os.Getenv("KUBE_API_VERSIONS")
+	if len(kubeAPIVersions) == 0 {
 		authenticatorConfig.BootstrapTokenAuthenticator = bootstrap.NewTokenAuthenticator(
 			sharedInformers.Core().InternalVersion().Secrets().Lister().Secrets(v1.NamespaceSystem),
 		)
 	}
+
 	return authenticatorConfig.New()
 }
 

pkg/kubeapiserver/options/serving.go

@@ -81,7 +81,7 @@ func NewInsecureServingOptions() *InsecureServingOptions {
 	}
 }
 
-func (s InsecureServingOptions) Validate(portArg string) []error {
+func (s InsecureServingOptions) Validate() []error {
 	errors := []error{}
 
 	if s.BindPort < 0 || s.BindPort > 65535 {

staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go

@@ -236,12 +236,7 @@ func (s *GenericAPIServer) RequestContextMapper() apirequest.RequestContextMappe
 	return s.requestContextMapper
 }
 
-// MinRequestTimeout is exposed so that third party resource storage can be build in a different location.
-// TODO refactor third party resource storage
-func (s *GenericAPIServer) MinRequestTimeout() time.Duration {
-	return s.minRequestTimeout
-}
-
+// preparedGenericAPIServer is a private wrapper that enforces a call of PrepareRun() before Run can be invoked.
 type preparedGenericAPIServer struct {
 	*GenericAPIServer
 }
@@ -259,12 +254,6 @@ func (s *GenericAPIServer) PrepareRun() preparedGenericAPIServer {
 
 	s.installHealthz()
 
-	return preparedGenericAPIServer{s}
-}
-
-// Run spawns the secure http server. It only returns if stopCh is closed
-// or the secure port cannot be listened on initially.
-func (s preparedGenericAPIServer) Run(stopCh <-chan struct{}) error {
 	// Register audit backend preShutdownHook.
 	if s.AuditBackend != nil {
 		s.AddPreShutdownHook("audit-backend", func() error {
@@ -273,6 +262,12 @@ func (s preparedGenericAPIServer) Run(stopCh <-chan struct{}) error {
 		})
 	}
 
+	return preparedGenericAPIServer{s}
+}
+
+// Run spawns the secure http server. It only returns if stopCh is closed
+// or the secure port cannot be listened on initially.
+func (s preparedGenericAPIServer) Run(stopCh <-chan struct{}) error {
 	err := s.NonBlockingRun(stopCh)
 	if err != nil {
 		return err
@@ -349,7 +344,7 @@ func (s *GenericAPIServer) installAPIResources(apiPrefix string, apiGroupInfo *A
 		}
 
 		if err := apiGroupVersion.InstallREST(s.Handler.GoRestfulContainer); err != nil {
-			return fmt.Errorf("Unable to setup API %v: %v", apiGroupInfo, err)
+			return fmt.Errorf("unable to setup API %v: %v", apiGroupInfo, err)
 		}
 	}
 
@@ -372,6 +367,7 @@ func (s *GenericAPIServer) InstallLegacyAPIGroup(apiPrefix string, apiGroupInfo
 	// Install the version handler.
 	// Add a handler at /<apiPrefix> to enumerate the supported api versions.
 	s.Handler.GoRestfulContainer.Add(discovery.NewLegacyRootAPIHandler(s.discoveryAddresses, s.Serializer, apiPrefix, apiVersions, s.requestContextMapper).WebService())
+
 	return nil
 }