Controller-runtime: Why DelegatingClient is needed?

Created on 25 Oct 2018 · 19Comments · Source: kubernetes-sigs/controller-runtime

From the code,
the Manager.GetClient returns an DelegatingClient which read from cache for structured object, but read directly for unstructured object.

This forbids us to use the client before controller loops(since cache needs to be synced). (Our use case is load configurations from configMap before controller loops).

From my perspective, users should use manager.GetCache if they need caching behavior(and we can do the delegation there to read directly for unstructured object).
For manager.GetClient, it's better to let it always read directly without cache.

kindesign lifecyclrotten

Source

M00nF1sh

Most helpful comment

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.

nkvoll on 11 Dec 2018

👍3

All 19 comments

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 :-)