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

Consider making cache.internal.CacheReader accessible for users #247

Closed
nkvoll opened this issue Dec 12, 2018 · 6 comments
Closed

Consider making cache.internal.CacheReader accessible for users #247

nkvoll opened this issue Dec 12, 2018 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@nkvoll
Copy link

nkvoll commented Dec 12, 2018

The documentation at https://book.kubebuilder.io/beyond_basics/using_client_go_informers.html shows how to wire in informers from the client-go package, but due to the behavior of the client provided to controllers (creates a watch when get/list is used), it means the story gets complicated

See below for an example where a custom informer is used, but having the ability to create a CacheReader would be very useful when it comes to actually using it.

	clientSet, err := kubernetes.NewForConfig(mgr.GetConfig())
	if err != nil {
		return err
	}

	informer := informersCoreV1.NewFilteredSecretInformer(
		clientSet,
		"default",
		10 * time.Hour,
		cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
		func(options *v1.ListOptions) {
			options.LabelSelector = labels.Set{"foo": "bar"}.String()
		},
	)

	if err := c.Watch(
		&source.Informer{
			Informer: informer,
		},
		&handler.EnqueueRequestForObject{},
	); err != nil {
		return err
	}

	if err := mgr.Add(manager.RunnableFunc(func(stopCh <-chan struct{}) error {
		informer.Run(stopCh)
		return nil
	})); err != nil {
		return err
	}

	// TODO: not sure how to actually go from the above Informer to a cache.Cache?
	//   e.g actually make it usable using the controller-runtime client.Reader interface.

In this scenario I cannot use the mgr.GetClient() to do any reads of Secrets because that would set up a watch (thus why bother with the custom informer?), so the fall back would be working with the old informer api's instead of the easier to use client.Reader interfaces, which would be possible if I could do secretsCache := NewCacheReader(informer.GetIndexer(), groupVersionKind)

Being able to explicitly ask for caching / non-caching behavior as discussed in #180 combined with first-class support of filtered watches (#244) might not fully solve this case, as there could be multiple different informers for the same GVK, but with different filters (overlapping or not), meaning the choice of informer/cache perhaps should be more explicit than first thought? (this is also mentioned at the bottom of #244). By letting users easily create their own CacheReaders it at least makes it possible to do this today without gating it on similar/getting built-in support for this in the main client(s) provided by controller-runtime to the custom controllers, though having to use more than one client to achieve this is unfortunate.

@DirectXMan12
Copy link
Contributor

/kind feature
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Dec 21, 2018
@DirectXMan12
Copy link
Contributor

#327 addresses this from the opposite direction

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 4, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 4, 2019
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

DirectXMan12 pushed a commit that referenced this issue Jan 31, 2020
removed an extra 'dep ensure' step in init project
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

No branches or pull requests

4 participants