Comments (19)
So, the other "nice" thing that the reader does it that it auto-adds new watches if you request a new object. This might be too magic, but so far it seems to have worked out well
FWIW, coming in as a (relatively new) user of controller-runtime, this is something that just caught me off-guard: if you Get
any object, a List
watch will be set up behind the scenes for me.
Doesn't this have implications if there are a lot of objects (e.g Pods), and I only want to:
- Get a single one: I end up watching every Pod (in all namespaces, unless I namespace the controller?)
- Watch a subset (e.g only pods labelled
foo: bar
): I end up watching as above (see example below for a possible start of a workaround)
If an informer has been auto added, there is also no way for me to indicate that it is no longer needed, so my operator will possibly keep watching events for resources that might no longer be relevant,.
Perhaps it's just me, but I'm currently struggling a little finding how to set a watch with a selector, and I'm not sure how it would work out with the current cache behaviors (e.g only one informer per GVK, so there seems to be a strict 1-to-1 between a gvk and an informer? unless I create my own listwatch, informer and cache reader manually, meaning I have to be careful and manage reads correctly between the default client and my own cache reader?)
See example code (not complete) for the manual lifting required (click to expand)
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, so I have to fall back to using the good ole ClientSet
?
What I (as an operator developer) would like to be able to do (excluding the current patterns of setting up namespace-wide watches and auto-adding watches for all resources of a given gvk when getting):
- Get + watch for single objects (and a way to stop watching). (Use case: user can provide a reference to a named resource I should use when reconciling, e.g a config map containing some config, but I don't want to watch /all/ config maps in the namespace)
- List + watch for objects with filters (field selectors, label selectors) (and a way to stop watching) (Use case: having multiple controllers not all having to watch all the resources of each watched kind)
- Get without watching (no caching) (Use case: one-off getting a resource, e.g where the gets are rare and unlikely to be repeated or updates through watches would cause more load than just sporadic gets)
- List without watching (w or w/o selectors, no caching) (Use case: same as above, but for selected resources)
Apologies if this is a bit long and more suitable to its own issue(s), but it felt very relevant to the caching behavior.
EDIT: I've taken this comment and tried splitting some of the functionality asks into separate issues (#244, #245, #246, #247), but they are all related to this issue in one way or another, and I'm not sure about the main maintainers would like this to be handled.
from controller-runtime.
We're trying to make it as easy as possible to use the "correct" Kubernetes pattern. So you don't have to manage two separate objects -- you just get the correct behavior off the bat. This does mean that for things like loading configuration from specific objects, you do need to manually instantiate the client.
You do raise a valid point here, and it's difficult to balance the "can we make it really easy so the user just doesn't have to care" and "is this magic going to trip people up". Did this trip you up or cause significant confusion? I'm happy to have a conversation on this feature, and/or to see if we can make the pitfalls of the default client clearer. I definitely don't want using controller-runtime to be confusing or painful :-)
from controller-runtime.
/kind design
from controller-runtime.
the major confusing part came from manager exposed both getClient & getCache(both of them provided reader interface), which make me think naturally that one provides uncached behavior and one provides cached behavior.
If we can merge them together, then it's less confusing.
Also, i think loading configuration from configMaps before controller loops seems a pretty general use case. Maybe we can make waitForCacheSync happen inside manager.New
(or another explicit method call) instead of manager.Start
? ( i think it's not ideal for users to manually init an k8s client just to fulfill this use case)
from controller-runtime.
@droot thoughts here?
Yeah, I definitely see how it's a common use case. We can prob make the naming a bit clearer, and introducing an explicit GetRemoteOnlyClient
helper (terrible name, just a placeholder) might make this clearer too.
from controller-runtime.
The problem with starting the caches in manager.New
is that it makes it impossible to safely add indexes (at least, how we do it now, IIRC), which can be super useful in clearing up bits of code. It's a bit of an advanced use case, but nice to have.
from controller-runtime.
another tricky solution i can think of is make the manager.GetClient read directly at first, and change to delegated cache implementation once cache is synced πΈ . (maybe via a flag in the delegating client). Then all use case are satisfied and transparent to users.
from controller-runtime.
So, the other "nice" thing that the reader does it that it auto-adds new watches if you request a new object. This might be too magic, but so far it seems to have worked out well. Your suggestion, while not necessarily bad in the absence of this logic, cause the behavior of the client to become inconsistent when paired with the auto-add.
from controller-runtime.
Just checked the auto-add implementation. I don't think the behavior of client will be inconsistent.
Since auto-add behavior is not necessary for Read/List operations from user's perspective.(It's an implementation detail needed when read/list is performed by cache).
And when they need watch behavior, they have to use cache.GetInformers, which setup auto-add correctly. πΈ
from controller-runtime.
This might be a really bad idea but what if before starting the controllers, we use a direct client, and then during start change the client to a cache client.
Allows for the setup and then does the right things for the controllers?
from controller-runtime.
happy to contribute if we are going for the solution of switch read client
πΈ .
But the problem of both manager.GetClient and manager.GetCache exposed reader still exists. I'm a big fan of the python zen There should be one-- and preferably only one --obvious way to do it
π
from controller-runtime.
I am going to take the other side of the argument here. I think that exposing the cache helps advanced users, and making the client βsmartβ and have a default that is to use the cache on reads makes sense 90% of the time. This reduces the cognitive overhead for new users and helps them get the right outcome.
What about something like for the status writer. A client.NoCache().Get(...)?
from controller-runtime.
the major confusing part came from manager exposed both getClient & getCache(both of them provided reader interface), which make me think naturally that one provides uncached behavior and one provides cached behavior.
This is good feedback. Now I read it, it is confusing.
For our next major revision, we can probably try to do the following:
- Remove
GetCache
API from manager - Introduce method on
Client
interfaceNoCache
(as @shawn-hurley suggested above)
from controller-runtime.
@droot Where would I get these methods: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/cache/cache.go#L51-L55 then if we remove the GetCache() interface?
Maybe a GetInformer()
method is added and an Informer
interface is exposed?
from controller-runtime.
I think your concern is relevant here. This is one of the things that made me a bit nervous initially, but it is a bit of a corner case, and generally the auto-caching follows the behavior we encourage in Kubernetes (always cache, except in really rare cases). Let's deal with the cases in the issues you've opened up -- I think splitting the use cases out was a good idea.
from controller-runtime.
(and yes, there's currently not a good way to limit/filter watches. it's something we need to solve)
from controller-runtime.
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
from controller-runtime.
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
from controller-runtime.
closing this master issue in favor of the sub-issues
from controller-runtime.
Related Issues (20)
- fake client: fatal error: concurrent map read and map write HOT 5
- Proposal: Naming consistency for leader election lock options HOT 5
- Request for v0.17.0 HOT 6
- Remove deprecated webhook interfaces HOT 11
- Support startup probes HOT 9
- proper versioning for setup-envtest HOT 12
- Controller Runtime Outputs `klog` in non-standard format HOT 7
- admission.Webhook always returns HTTP 200, making failurePolicy=Ignore moot HOT 15
- Create a separate registry for work-queue metrics HOT 2
- Add "controllerutil" helpers for applyconfigurations (SSA) HOT 4
- Proposal: Generics for CustomValidator & CustomDefaulter HOT 4
- Fakeclient: Patching an object that has allowUnconditionalUpdate=false where the patched version has no RV should succeed HOT 3
- client.Object occasionally returns empty string when calling for GroupVersionKind HOT 5
- Predicates added with For() affects Owns() HOT 18
- unable to list/watch resources without cluster-wide permissions HOT 2
- In favor of what is `EventBroadcaster` manager option deprecated? HOT 3
- admission.Decoder.Decode is panicking after version bump HOT 8
- How to iterate over ObjectList HOT 2
- Cache issue when using receiver
- Instance of CR in the work queue are not unique HOT 7
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from controller-runtime.