kube

kube copied to clipboard

A semi-mirror the predicates from controller-runtime that allows filtering watcher streams. Removed scope regarding controllers.

    if !generation.cmp_update(&mut *ctx.predicate_cache.write().await, &doc) {
        info!("ignoring generationally equivalent reconcile for {}", doc.name());
        return Ok(Action::requeue(Duration::from_secs(30 * 60)));
    }

(related discord thread)

May 13 '22 19:05 clux

Codecov Report

Merging #911 (4a6f820) into main (4f633ee) will increase coverage by 0.78%. The diff coverage is 75.86%.

@@            Coverage Diff             @@
##             main     #911      +/-   ##
==========================================
+ Coverage   72.75%   73.53%   +0.78%     
==========================================
  Files          67       68       +1     
  Lines        5201     5351     +150     
==========================================
+ Hits         3784     3935     +151     
+ Misses       1417     1416       -1     

May 13 '22 20:05 codecov-commenter

Previous potential problems noted about using predicates for controllers that came up before and wanted to note them. This is ultimately not directly related to the interface in this PR because this is only the watcher interface, which is still useful for people running straight watcher loops.

That said, I no longer think they are objections worth blocking this for. Here are the problems that came up:

1. It inhibits requeues / retriggers / only accounts for the last objects in the controller's queue stream

The system is meant to requeue based on time, and allow users to re-trigger in case something bad happened, so it is essential that this is not filtered out at the controller level. The solution is to not do this filtering inside the controller; let users filter outside (via stream sharing), and always propagate requeues/retriggers.

2. It potentially encourages non-idempotent modes of operation of a reconciler

I.e. less redundant work scheduled, leads to people being lazier. Yes, true, but isn't that a good thing? Ultimately people want to write less code. This allows them to do that. It is an advanced feature, but i have several controllers where we i effectively have to filter out generation changes out of band, and awkwardly, so this would be very useful.

It's also not like we are doing filtering across all streams with the same brush. This is intended to be setup in the way you want for each stream.

EDIT: edited after more clarity, since it was the most matching comment.

May 13 '22 20:05 clux

Have reduced this in scope to not be about controllers. It's currently now just something we can use to filter flattened watcher streams:

let stream = watcher(node, lp).applied_objects().filter_predicates(predicates::annotations);
pin_mut!(stream);
while let Some(node) = rf.try_next().await? {
    info!("saw node {} with hitherto unseen annotations", node.name());
}

It does the filtering by maintaining a reasonably optimised cache (only storing hashed values - so limits predicate fns instead), which is important because objects coming and going at the usual kubernetes rate is going to leave this cache with lots of potential to grow. Ideally, the map should be some timelimited cache where keys drop out of scope after some time.

May 15 '22 12:05 clux

Opening this up for review because I think this has value as is when plugged through watcher directly, and have removed/hidden references to controller ideas for it (which are a little more controversial).

Aug 08 '22 18:08 clux

Wrapped this in an unstable feature and added better docs. Hoping to get this in so we can iterate if we find corner cases. I still have not found good ways to solve this, and other people keep asking about it. I think this can actually be useful with something like #1163 as an early place to experiment with stream sharing with stuff like #1163. cc @Dav1dde

Deny will fail because of a dev-dep duplicate (it's fixed in #1171)

Mar 29 '23 17:03 clux

Going to send this through under the unstable feature to let it stew for a while. This is a simplistic solution and may benefit from some TTL style decay (or moving it before the flatteners to get access to raw watcher::Events) in the future. But, in the mean time, would like to run this in some of my controllers for a while to see how well it works, and if that type of over-engineering is necessary.

Apr 03 '23 15:04 clux