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

Add implicit runner inside Reflector #151

Closed
clux opened this issue Feb 27, 2020 · 0 comments · Fixed by #218
Closed

Add implicit runner inside Reflector #151

clux opened this issue Feb 27, 2020 · 0 comments · Fixed by #218
Assignees
Labels
runtime controller runtime related

Comments

@clux
Copy link
Member

clux commented Feb 27, 2020

To avoid people having to do this type of task spawning just to ensure a reflector is up to date:

https://github.com/babylonhealth/shipcat/blob/fd78055e795017b276eb99de3ecc86a2f99eab86/raftcat/src/state.rs#L184-L207

There's no reason why people NEED to do this themselves. It should be encapsulated herein if possible.

This should also be integrated with signal handlers in the future. But making a separate issue for that.

@clux clux added the runtime controller runtime related label Feb 27, 2020
@clux clux added this to the kubecon-eu-2020 milestone Feb 29, 2020
clux added a commit that referenced this issue Apr 6, 2020
fixes #151 and fixes #152 with tokio::signal and a futures::select call.
drops a lot of Reflector logic to defer to Informer.

Seems to work pretty well, but may need to deal with desyncs better.
Ran into the unordered resourceVersion issue on a really old namespace.
Need to double check if that can happen with old version.
@clux clux self-assigned this Apr 6, 2020
@clux clux linked a pull request Apr 6, 2020 that will close this issue
clux added a commit that referenced this issue Apr 7, 2020
fixes #151 and fixes #152 with tokio::signal and a futures::select call.
drops a lot of Reflector logic to defer to Informer.

Seems to work pretty well, but may need to deal with desyncs better.
Ran into the unordered resourceVersion issue on a really old namespace.
Need to double check if that can happen with old version.
@clux clux closed this as completed in #218 Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtime controller runtime related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant