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

Track store readiness #1243

Merged
merged 9 commits into from Jul 6, 2023
Merged

Conversation

nightkr
Copy link
Member

@nightkr nightkr commented Jul 5, 2023

Motivation

Fixes #1226.

Solution

We add a new wait_until_ready() method to Store, which returns a Future that completes after any write has been done to the store. If the store has already been changed in the past then it completes immediately.

Further, controller::Runner is extended with a mechanism that allows us to gate all jobs until a trigger Future has completed. This is then used in controller::applier to gate reconciliation based on the store readiness introduced earlier.

@nightkr nightkr added runtime controller runtime related changelog-add changelog added category for prs labels Jul 5, 2023
kube-runtime/src/controller/mod.rs Show resolved Hide resolved
kube-runtime/src/controller/mod.rs Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Merging #1243 (047a73f) into main (bda3b90) will increase coverage by 0.70%.
The diff coverage is 95.37%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1243      +/-   ##
==========================================
+ Coverage   71.89%   72.60%   +0.70%     
==========================================
  Files          71       72       +1     
  Lines        5729     5899     +170     
==========================================
+ Hits         4119     4283     +164     
- Misses       1610     1616       +6     
Impacted Files Coverage Δ
kube-runtime/src/utils/mod.rs 64.15% <ø> (ø)
kube-runtime/src/controller/mod.rs 35.15% <85.71%> (+1.42%) ⬆️
kube-runtime/src/controller/runner.rs 94.36% <93.67%> (-1.09%) ⬇️
kube-runtime/src/utils/delayed_init.rs 97.40% <97.40%> (ø)
kube-runtime/src/reflector/store.rs 98.94% <100.00%> (+2.47%) ⬆️

Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial comments. Short story is that I really like the approach (even though there's a lot more here than I expected from a one-day nerd snipe). Very good tests.

Some questions mainly around complexity and public interfaces.

kube-runtime/src/utils/mod.rs Outdated Show resolved Hide resolved
kube-runtime/src/utils/delayed_init.rs Outdated Show resolved Hide resolved
kube-runtime/src/utils/delayed_init.rs Outdated Show resolved Hide resolved
kube-runtime/src/utils/delayed_init.rs Outdated Show resolved Hide resolved
kube-runtime/src/controller/runner.rs Show resolved Hide resolved
kube-runtime/src/controller/mod.rs Show resolved Hide resolved
kube-runtime/src/reflector/store.rs Show resolved Hide resolved
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Signed-off-by: Natalie Klestrup Röijezon <nat@nullable.se>
Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very transparent from a user POV with debug logs kube=debug (trimmed timestamps for the 0.1s this took):

INFO kube_runtime::controller: press ctrl+c to shut down gracefully
DEBUG kube_runtime::controller: applier runner held until store is ready
DEBUG HTTP{http.method=GET http.url=https://0.0.0.0:51145/apis/nullable.se/v1/configmapgenerators? otel.name="list" otel.kind="client"}: kube_client::client::builder: requesting
DEBUG HTTP{http.method=GET http.url=https://0.0.0.0:51145/api/v1/configmaps? otel.name="list_metadata" otel.kind="client"}: kube_client::client::builder: requesting
DEBUG kube_runtime::controller: store is ready, starting runner
DEBUG HTTP{http.method=GET http.url=https://0.0.0.0:51145/apis/nullable.se/v1/configmapgenerators?&watch=true&timeoutSeconds=290&allowWatchBookmarks=true&resourceVersion=779 otel.name="watch" otel.kind="client"}: kube_client::client::builder: requesting
DEBUG HTTP{http.method=GET http.url=https://0.0.0.0:51145/api/v1/configmaps?&watch=true&timeoutSeconds=290&allowWatchBookmarks=true&resourceVersion=779 otel.name="watch_metadata" otel.kind="client"}: kube_client::client::builder: requesting

@clux clux added this to the 0.84.0 milestone Jul 6, 2023
@nightkr nightkr merged commit db585dd into kube-rs:main Jul 6, 2023
16 checks passed
@nightkr nightkr deleted the feature/store-readiness branch July 6, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-add changelog added category for prs runtime controller runtime related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow awaiting a reflector Store for readiness
2 participants