Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TTL for DeferredDiscoveryRESTMapper #75383

Open
timuthy opened this Issue Mar 14, 2019 · 5 comments

Comments

Projects
None yet
5 participants
@timuthy
Copy link

timuthy commented Mar 14, 2019

What would you like to be added:
The DeferredDiscoveryRESTMapper holds some sort of cache for once discovered Resource / Kind objects via its delegate instance.
Unfortunately this cache must be invalidates explicitly by its user by calling the Reset() method.

It would be nice to make the DeferredDiscoveryRESTMapper aware of a TTL mechanism which invalidates the cache or calls Reset() internally after the TTL is expired.

Why is this needed:
Controllers require to have an up-to-date RESTMapper instance, especially when an API group becomes available after a controller is already running. The Kube-controller-manager has a separate Goroutine for that.

Imho, it's inconvenient to reset the cached discovery information of a DeferredDiscoveryRESTMapper instance only from outside as well as susceptible to be missed by users.
Due to the combination of RESTMapper and DiscoveryClient it is not quite obvious what instance caches which information and how invalidations can be handled.

For example, the function below takes an instance of CachedDiscoveryInterface which is used by the returned DeferredDiscoveryRESTMapper.

func NewDeferredDiscoveryRESTMapper(cl discovery.CachedDiscoveryInterface) *DeferredDiscoveryRESTMapper

On the other hand, the passed CachedDiscoveryInterface can be configured with some caching options. But the caching options of the underlying DiscoveryClient don't affect the discovery information in the DeferredRESTMapper.

func NewCachedDiscoveryClientForConfig(config *restclient.Config, discoveryCacheDir, httpCacheDir string, ttl time.Duration) (*CachedDiscoveryClient, error)
@timuthy

This comment has been minimized.

Copy link
Author

timuthy commented Mar 14, 2019

/sig api-machinery

@timuthy

This comment has been minimized.

Copy link
Author

timuthy commented Mar 14, 2019

@hzxuzhonghu

This comment has been minimized.

Copy link
Member

hzxuzhonghu commented Mar 16, 2019

I think it is resonable.

@caesarxuchao

This comment has been minimized.

Copy link
Member

caesarxuchao commented Mar 18, 2019

@timuthy

This comment has been minimized.

Copy link
Author

timuthy commented Mar 19, 2019

I'm willing to file a PR for this but just wanted to know if you have any concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.