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

Controller<T> event duplication #279

Closed
clux opened this issue Jul 20, 2020 · 7 comments
Closed

Controller<T> event duplication #279

clux opened this issue Jul 20, 2020 · 7 comments
Labels
help wanted Not immediately prioritised, please help! question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related

Comments

@clux
Copy link
Member

clux commented Jul 20, 2020

Noticing an issue where we are receiving events for the patch operations we did to our own crd's status.

example reconciler from controller-rs

This is the dumbest reconciler we have in any example;

  • foo crd touched
  • patch its status object with a bool

But the last step now triggers the reconciler again because the crd was touched (by ourselves).
It perhaps possible that we can put an "ignore" on events coming in from the event that we "expect" to see updates, but that seems error prone (as well as difficult).

Not sure this is necessarily a bad thing, but I expect this is counter-intuitive.

Because you were so helpful last time around @alexeldeib , do you have any hints from controller-runtime for this?

@clux clux added help wanted Not immediately prioritised, please help! question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related labels Jul 20, 2020
@nightkr
Copy link
Member

nightkr commented Jul 21, 2020

This is to be expected, I'm afraid. The alternative would be pretty racy and unexpected (store the last-written resourceVersion somewhere?).

IMO this also generally fits the expected behaviour. If you have a FooClaim controller that manages Foos (with controllers for both in the same project), then you want the Foo controller to reconcile when the object is updated by the FooClaim controller.

@clux
Copy link
Member Author

clux commented Jul 21, 2020

I guess some controllers store spec hashes somewhere. Maybe that's to avoid doing the work multiple times. If controllers can do that then at least there are solutions.

@alexeldeib
Copy link
Contributor

Yeah, controller-runtime has a few predicates to help with event filtering on the reconcilers (looks similar to trigger_with);
https://godoc.org/sigs.k8s.io/controller-runtime/pkg/predicate#GenerationChangedPredicate
https://godoc.org/sigs.k8s.io/controller-runtime/pkg/predicate#ResourceVersionChangedPredicate

@clux
Copy link
Member Author

clux commented Aug 25, 2020

I think the difficulty here is whether or not we should force this behavior on the Controller. We would have to use a lot more reflectors (or something smarter) at the cost of memory use and code readability, whereas if users just cached what they have seen from object X, then they could just short-circuit the reconcile loop. This might be the smarter pattern, given that we probably want reconcile to trigger every 5 minutes for every object anyway; optimizing away a handful of them isn't going to be super useful.

Not sure what people think.

I think at least we could have some helpers for how to do it manually; like showing how to track it in observedGeneration maps via the Context and doing early exit in the reconciler.

Side note; was looking at the mutation_detector, it's got a memory leak by design from looking at the comments.

@nightkr
Copy link
Member

nightkr commented Aug 26, 2020

FWIW, you can actually do that kind of filtering without memory leaks, by putting the filter as a stage between watcher and reflector. But I can't really see a good way to expose that for the Controller :/

@clux
Copy link
Member Author

clux commented Dec 31, 2020

Coming back to this, this is probably the next thing I'd like to improve.

Want to investigate whether we can inject some kind of event filter at the very end of the applier loop (so that potentially reconciles won't get called if some injectable selector_fn: (Fn: K -> P) (where P: some property on K) returns different values for the one from the reflector and the current (and we can provide selector functions for generation and resource version behind some enum).

This feels similar to what users would have to write in their apps if they want to have a "do constant work" type reconciler (e.g. something like #354) that perhaps updates status objects even if nothing technically changed on the object itself.

This might be awkward for events that do not come from the root type, but by types that are owned by the root type - I am hoping that the filter can be avoided in those cases.

@clux clux changed the title Controller<T> event duplication? Controller<T> event duplication Nov 3, 2021
@clux clux linked a pull request May 13, 2022 that will close this issue
@clux
Copy link
Member Author

clux commented Apr 7, 2023

This can now be iltered out via predicates using the controller stream interface. See kube-rs/controller-rs#50 for how this can look in practice.

@clux clux closed this as completed Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Not immediately prioritised, please help! question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related
Projects
Status: Done
Development

No branches or pull requests

3 participants