reflector api redesign

[clux]

Reflector as it stands in async land is awkward and not very user friendly. A few reasons for this:

Polling does not need to be done by the consumer

You must drive a Reflector with .poll() inside the app, but the app doesn't get anything back from this, as it only serves to drive internal state management. This is in contrast to the Informer whose poll() returns the literal Stream of WatchEvents (making it a lot more useful).

This means most Reflector get this leads to this type of logic: https://github.com/clux/version-rs/blob/f636f8d66eb2e2f2019b8b0a0467a77b2fd133a2/version.rs#L50-L62

It might be better to just hide the polling action inside the Reflector, even if this forces us to expose an error handling callback FnOnce or an ErrorHandlingPolicy enum with ExponentialBackoff (we do backoff) + SelfTerminate (k8s does backoff).

Hidden information / state getters insufficient

With the events hidden from the app, it's hard to keep any secondary states up to date. If you want to keep a different data structure as your cache you need to apply a transform yourself periodically. It might be better to expose the events, or the keys of the changed resources.

This leads to this type of logic: https://github.com/clux/version-rs/blob/f636f8d66eb2e2f2019b8b0a0467a77b2fd133a2/version.rs#L70-L79

Here we've badly tied the internal update to the watch timeout (300s by default now) because they already have to do one pointless timer. If they have to do a timer, they should really only have to do the ones that matter to them. But it's also possible we should expose a Reflector::transform function to pass through Modified events through so the cache inside the Reflector is always kept correct with the information the app always care about.

notes

These changes would mean that Reflector and Informer APIs deviate a bit more from each other, but they should serve different, and real purposes here. We're not tied to the golang api for the abstractions, they're just inspirations at this point.

[xguerin]

A pattern I find very useful in my applications is to both allow the application to react on incoming Kubernetes events while still maintaining the cache of Kubernetes objects. This combination is nicely laid out in this blog post and I believe is also available in the Go client, although since I never used it I cannot say for sure.

In essence, it would be the Informer/Reflector pattern merged into one. I gave it a shot here https://github.com/xguerin/kube-rs/tree/feature/controller. It's probably not the best implementation but it achieves the goal stated above.

[clux]

Late response, but appreciate your change here @xguerin ! I think we do need to expose some kind of combination for sure.

Will be trying to look at this this week and will investigate more whether to take more inspiration from the controller-runtime package that seems to have spun out of kubebuilder. The go ecosystem have matured quite a bit and there are nicer abstractions to take inspiration from that we can package up in some - hopefully a nicer - way in rust.

Might create a Controller type that just tells you when to reconcile as well.

[clux]

The new reflector from Theo's runtime (in #258) will address these concerns and is a complete rewrite.