Merge pull request #103900 from ash2k/ash2k/cache-mapper-and-client

Cache rest mapper and discovery client

staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go

@@ -103,10 +103,17 @@ type ConfigFlags struct {
 	WrapConfigFn func(*rest.Config) *rest.Config
 
 	clientConfig     clientcmd.ClientConfig
-	lock         sync.Mutex
-	// If set to true, will use persistent client config and
-	// propagate the config to the places that need it, rather than
-	// loading the config multiple times
+	clientConfigLock sync.Mutex
+
+	restMapper     meta.RESTMapper
+	restMapperLock sync.Mutex
+
+	discoveryClient     discovery.CachedDiscoveryInterface
+	discoveryClientLock sync.Mutex
+
+	// If set to true, will use persistent client config, rest mapper, discovery client, and
+	// propagate them to the places that need them, rather than
+	// instantiating them multiple times.
 	usePersistentConfig bool
 	// Allows increasing burst used for discovery, this is useful
 	// in clusters with many registered resources
@@ -216,8 +223,8 @@ func (f *ConfigFlags) toRawKubeConfigLoader() clientcmd.ClientConfig {
 // toRawKubePersistentConfigLoader binds config flag values to config overrides
 // Returns a persistent clientConfig for propagation.
 func (f *ConfigFlags) toRawKubePersistentConfigLoader() clientcmd.ClientConfig {
-	f.lock.Lock()
-	defer f.lock.Unlock()
+	f.clientConfigLock.Lock()
+	defer f.clientConfigLock.Unlock()
 
 	if f.clientConfig == nil {
 		f.clientConfig = f.toRawKubeConfigLoader()
@@ -230,6 +237,27 @@ func (f *ConfigFlags) toRawKubePersistentConfigLoader() clientcmd.ClientConfig {
 // Expects the AddFlags method to have been called.
 // Returns a CachedDiscoveryInterface using a computed RESTConfig.
 func (f *ConfigFlags) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
+	if f.usePersistentConfig {
+		return f.toPersistentDiscoveryClient()
+	}
+	return f.toDiscoveryClient()
+}
+
+func (f *ConfigFlags) toPersistentDiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
+	f.discoveryClientLock.Lock()
+	defer f.discoveryClientLock.Unlock()
+
+	if f.discoveryClient == nil {
+		discoveryClient, err := f.toDiscoveryClient()
+		if err != nil {
+			return nil, err
+		}
+		f.discoveryClient = discoveryClient
+	}
+	return f.discoveryClient, nil
+}
+
+func (f *ConfigFlags) toDiscoveryClient() (discovery.CachedDiscoveryInterface, error) {
 	config, err := f.ToRESTConfig()
 	if err != nil {
 		return nil, err
@@ -255,6 +283,27 @@ func (f *ConfigFlags) ToDiscoveryClient() (discovery.CachedDiscoveryInterface, e
 
 // ToRESTMapper returns a mapper.
 func (f *ConfigFlags) ToRESTMapper() (meta.RESTMapper, error) {
+	if f.usePersistentConfig {
+		return f.toPersistentRESTMapper()
+	}
+	return f.toRESTMapper()
+}
+
+func (f *ConfigFlags) toPersistentRESTMapper() (meta.RESTMapper, error) {
+	f.restMapperLock.Lock()
+	defer f.restMapperLock.Unlock()
+
+	if f.restMapper == nil {
+		restMapper, err := f.toRESTMapper()
+		if err != nil {
+			return nil, err
+		}
+		f.restMapper = restMapper
+	}
+	return f.restMapper, nil
+}
+
+func (f *ConfigFlags) toRESTMapper() (meta.RESTMapper, error) {
 	discoveryClient, err := f.ToDiscoveryClient()
 	if err != nil {
 		return nil, err
@@ -324,7 +373,6 @@ func (f *ConfigFlags) AddFlags(flags *pflag.FlagSet) {
 	if f.Timeout != nil {
 		flags.StringVar(f.Timeout, flagTimeout, *f.Timeout, "The length of time to wait before giving up on a single server request. Non-zero values should contain a corresponding time unit (e.g. 1s, 2m, 3h). A value of zero means don't timeout requests.")
 	}
-
 }
 
 // WithDeprecatedPasswordFlag enables the username and password config flags
@@ -377,7 +425,7 @@ func stringptr(val string) *string {
 }
 
 // overlyCautiousIllegalFileCharacters matches characters that *might* not be supported.  Windows is really restrictive, so this is really restrictive
-var overlyCautiousIllegalFileCharacters = regexp.MustCompile(`[^(\w/\.)]`)
+var overlyCautiousIllegalFileCharacters = regexp.MustCompile(`[^(\w/.)]`)
 
 // computeDiscoverCacheDir takes the parentDir and the host and comes up with a "usually non-colliding" name.
 func computeDiscoverCacheDir(parentDir, host string) string {

staging/src/k8s.io/cli-runtime/pkg/resource/builder.go

@@ -212,7 +212,7 @@ func NewBuilder(restClientGetter RESTClientGetter) *Builder {
 
 	return newBuilder(
 		restClientGetter.ToRESTConfig,
-		(&cachingRESTMapperFunc{delegate: restClientGetter.ToRESTMapper}).ToRESTMapper,
+		restClientGetter.ToRESTMapper,
 		(&cachingCategoryExpanderFunc{delegate: categoryExpanderFn}).ToCategoryExpander,
 	)
 }
@@ -1187,28 +1187,6 @@ func HasNames(args []string) (bool, error) {
 	return hasCombinedTypes || len(args) > 1, nil
 }
 
-type cachingRESTMapperFunc struct {
-	delegate RESTMapperFunc
-
-	lock   sync.Mutex
-	cached meta.RESTMapper
-}
-
-func (c *cachingRESTMapperFunc) ToRESTMapper() (meta.RESTMapper, error) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-	if c.cached != nil {
-		return c.cached, nil
-	}
-
-	ret, err := c.delegate()
-	if err != nil {
-		return nil, err
-	}
-	c.cached = ret
-	return c.cached, nil
-}
-
 type cachingCategoryExpanderFunc struct {
 	delegate CategoryExpanderFunc
 

staging/src/k8s.io/client-go/restmapper/discovery.go

@@ -205,7 +205,7 @@ func (d *DeferredDiscoveryRESTMapper) getDelegate() (meta.RESTMapper, error) {
 	}
 
 	d.delegate = NewDiscoveryRESTMapper(groupResources)
-	return d.delegate, err
+	return d.delegate, nil
 }
 
 // Reset resets the internally cached Discovery information and will