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

Allow awaiting a reflector Store for readiness #1226

Closed
clux opened this issue Jun 7, 2023 · 0 comments · Fixed by #1243
Closed

Allow awaiting a reflector Store for readiness #1226

clux opened this issue Jun 7, 2023 · 0 comments · Fixed by #1243
Labels
question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related

Comments

@clux
Copy link
Member

clux commented Jun 7, 2023

What problem are you trying to solve?

Consumers of reflector caches might occasionally notice that a Store is not guaranteed to be fully initialized before a webhook server or controller starts, leading to startup errors and readiness checks being passed too early. This is because the stream has to be awaited "a little bit" before this happens, to populate the Store fully. Unfortunately, there is no easy mechanism for users to wait for this.

Describe the solution you'd like

Something that can be awaited by something wrapping reflector, Store or WatchStreamExt. This is purposefully vague because I am looking for good suggestions.

The big problem with actually implementing this is that the watcher stream (which populates the Store) and the Store are separate objects (connected by this line), so we cannot just await store initialization without actually polling the stream (to start the watcher machinery). However, polling the stream is usually done by the controller/app itself.

IF we had something simple to .await, then fixing webhook servers would be much more ergonomic to setup, and it could also be used to prevent early Controller starts in both the native setup (easy Controller mode; it owns the stream/store, it can await) and the custom setup (hard unstable-controller for #1080; user owns the store/stream, users can await).

Describe alternatives you've considered

There is a larger proposal for a SafeStore using CancellationToken in https://github.com/kube-rs/kube/pull/1128/files#diff-5bf1be34972f407369ee8a0252b3abf07720efb03bca6b67043b0721a83240a4 . This might actually be the way to go, but opening this up more generally in the hopes to get more movement here.

(Personally, i am a little curious if this full machinery is necessary; stores are designed in such a way that they keep its state until re-inits are coming in so they should always have state. On the other hand, this type of setup would allow for a store that could potentially invalidate its own cache down the line if it does get stuck after one init.)

Webhook servers can currently work around the problem by looping through raw watcher events before chaining through reflectors and setting some global state for a /health check, but this is obviously ugly.

Documentation, Adoption, Migration Strategy

easy if it can be done additively, but it might not.

Target crate for feature

kube-runtime

@clux clux added question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related labels Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant