Backoff watcher cleanup #763

nightkr · 2021-12-21T10:18:55Z

Sub-PR of #703, to get the basic backoff functionality shipping, while putting off Observer (and its traitification) for a later PR.

Signed-off-by: clux <[email protected]> Signed-off-by: kazk <[email protected]>

Signed-off-by: kazk <[email protected]>

Remove `Error::SslError` Signed-off-by: kazk <[email protected]>

Signed-off-by: kazk <[email protected]>

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

…bjref Include the object that failed in controller::Error::ReconcilerFailed

to let cargo geiger show us as green Signed-off-by: clux <[email protected]>

* dedicate a root file to adopters Signed-off-by: clux <[email protected]> * Update ADOPTERS.md Co-authored-by: Teo Klestrup Röijezon <[email protected]> Co-authored-by: Teo Klestrup Röijezon <[email protected]>

* add sandbox host line + cii best practices badge Signed-off-by: clux <[email protected]> * Update README.md Co-authored-by: kazk <[email protected]> * Update README.md Co-authored-by: kazk <[email protected]> * Update README.md Co-authored-by: Teo Klestrup Röijezon <[email protected]> Co-authored-by: kazk <[email protected]> Co-authored-by: Teo Klestrup Röijezon <[email protected]>

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

To align with the old implementation Signed-off-by: Teo Klestrup Röijezon <[email protected]>

…meta-fields Remove TypeMeta fields from derived kind objects

* code coverage improvement testing Signed-off-by: clux <[email protected]> * codecov action Signed-off-by: clux <[email protected]> * try newer k3d action Signed-off-by: clux <[email protected]> * wtf Signed-off-by: clux <[email protected]> * get coverage running above 55 still not getting all the results i want because some ignored tests are not running Signed-off-by: clux <[email protected]> * remove duplicate instructions and leftovers Signed-off-by: clux <[email protected]> * fmt + comment Signed-off-by: clux <[email protected]> * simplify (tarpaulin does not inject features in workspaces anyway) Signed-off-by: clux <[email protected]> * fix test feature requirement Signed-off-by: clux <[email protected]> * need to move cfg limit before module Signed-off-by: clux <[email protected]> * try running doc tests as well Signed-off-by: clux <[email protected]> * try using nightly for tarpaulin doc test runner Signed-off-by: clux <[email protected]> * try with two runs of tarpaulin Signed-off-by: clux <[email protected]> * simplified with one-pass on stable Signed-off-by: clux <[email protected]> * test 3 core methods Signed-off-by: clux <[email protected]> * import parts of dynamic api example as discovery tests Signed-off-by: clux <[email protected]> * remove accidentally left-in flags Signed-off-by: clux <[email protected]> * revert pointless change in core_methods Signed-off-by: clux <[email protected]>

Signed-off-by: clux <[email protected]>

`warp` depends on old `tokio-tungstenite`. Signed-off-by: kazk <[email protected]>

Change the bounds of body's error type for better compatibility with middleware like `DecompressionLayer`. Signed-off-by: kazk <[email protected]>

Updates the requirements on [tokio-tungstenite](https://github.com/snapview/tokio-tungstenite) to permit the latest version. - [Release notes](https://github.com/snapview/tokio-tungstenite/releases) - [Changelog](https://github.com/snapview/tokio-tungstenite/blob/master/CHANGELOG.md) - [Commits](snapview/tokio-tungstenite@v0.15.0...v0.16.1) --- updated-dependencies: - dependency-name: tokio-tungstenite dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* more coverage experiments Signed-off-by: clux <[email protected]> * ping hooks - 2021-12-06 Signed-off-by: clux <[email protected]> * custom client example into kube-client/lib might need to add this to actions, but testing first Signed-off-by: clux <[email protected]> * stick to native tls testing Signed-off-by: clux <[email protected]> * port subresource tests from crd_api Signed-off-by: clux <[email protected]> * some coverage tests for discovery Signed-off-by: clux <[email protected]> * add some more test for dynamic objects Signed-off-by: clux <[email protected]> * do iteration Signed-off-by: clux <[email protected]> * do a few more discovery bits Signed-off-by: clux <[email protected]> * Has traits Signed-off-by: clux <[email protected]> * dont skip core group, has special casings Signed-off-by: clux <[email protected]> * more core cases Signed-off-by: clux <[email protected]> * add parallel integration tests for pods, ws, evict, eventrecorder should push it to 70% Signed-off-by: clux <[email protected]> * write a policy on testing, and classify into 3 types this forces our integration folder to be moved to e2e as it is the heaviest type of test. but crucially it leaves a path for us to run our integration tests in a matrix against various clusters. Signed-off-by: clux <[email protected]> * missing rename in root toml Signed-off-by: clux <[email protected]> * final rename piece Signed-off-by: clux <[email protected]> * missing rename Signed-off-by: clux <[email protected]> * markdown fixes plus review feedback on contributing Signed-off-by: clux <[email protected]> * fix test names and wrong feature requirements on them Signed-off-by: clux <[email protected]> * keep log + evict in same test Signed-off-by: clux <[email protected]> * move import around.. Signed-off-by: clux <[email protected]>

No need to add `#[doc(hidden)]` because they're not accessible and doesn't show up in docs. Signed-off-by: kazk <[email protected]>

* release changelog 0.65 Signed-off-by: clux <[email protected]> * stick with convention Signed-off-by: clux <[email protected]>

Updates the requirements on [dashmap](https://github.com/xacrimon/dashmap) to permit the latest version. - [Release notes](https://github.com/xacrimon/dashmap/releases) - [Commits](xacrimon/dashmap@v4.0.1...v5.0.0) --- updated-dependencies: - dependency-name: dashmap dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>

…5.0.0 Update dashmap requirement from 4.0.1 to 5.0.0

kube-rs#757) * Add DeleteParams constructors for setting PropagationPolicy Signed-off-by: Kate Goldenring <[email protected]> * Use JSON instead of string comparison in test Signed-off-by: Kate Goldenring <[email protected]> * Add entry to CHANGELOG Signed-off-by: Kate Goldenring <[email protected]> * rustfmt for jobs_api.rs Signed-off-by: Kate Goldenring <[email protected]> * Add setters for remaining fields of DeleteParams Signed-off-by: Kate Goldenring <[email protected]> * fix fmt + deny Signed-off-by: clux <[email protected]> Co-authored-by: clux <[email protected]>

* clarify contributing guidelines with doctests slightly also split by sentence as is done elsewhere. Signed-off-by: clux <[email protected]> * coverage automation tweaks Signed-off-by: clux <[email protected]> * add test for runtime::conditions Signed-off-by: clux <[email protected]> * newline option got turned off Signed-off-by: clux <[email protected]> * remove reduntant line in contributing Signed-off-by: clux <[email protected]> * fix and-ed Condition test Signed-off-by: clux <[email protected]> * more targetted test of objectlist iter (others were covered) Signed-off-by: clux <[email protected]> * better tests to ObjectList iterators and mutable accessors Signed-off-by: clux <[email protected]> * few more kube-core test asserts Signed-off-by: clux <[email protected]> * watch debug test Signed-off-by: clux <[email protected]> * pr feedback Signed-off-by: clux <[email protected]> * typo + retrigger ci Signed-off-by: clux <[email protected]> * minor tweaks to avoid leaving orphaned pods around when testing locally Signed-off-by: clux <[email protected]> * is it me, or is it a lot slower without caching? Signed-off-by: clux <[email protected]> * retrigger CI with caching on to get data Signed-off-by: clux <[email protected]> * ok it is faster, try also without limiting test-threads (works locally) Signed-off-by: clux <[email protected]> * avoid race conditions on foo crd installation in parallel tests - ensures there is only one test function that need api access to crds - those functions always install at start (via apply), and uninstall at the end Signed-off-by: clux <[email protected]> * one final re-run for confidence (it's faster and parallel now) * another re-run before going out Signed-off-by: clux <[email protected]>

Signed-off-by: clux <[email protected]>

* fix clippy must_uses must_use all the things Signed-off-by: clux <[email protected]> * more must_use catches and fix make clippy Signed-off-by: clux <[email protected]>

kube-rs#760) * Add the example to demonstrate the use of field-selector and jsonpath Signed-off-by: ChinYing-Li <[email protected]> * Modify the dynamic_jsonpath example to have a more advanced jsonpath Signed-off-by: ChinYing-Li <[email protected]> Co-authored-by: Eirik A <[email protected]>

kube-runtime/src/lib.rs

nightkr · 2021-12-21T10:22:47Z

kube-runtime/src/controller/mod.rs

@@ -749,7 +759,7 @@ where
            error_policy,
            context,
            self.reader,
-            self.trigger_selector
+            StreamBackoff::new(self.trigger_selector, self.trigger_backoff)


This applies a unified backoff timer to all triggers, do we want to maintain separate timers per trigger instead?

Given that we are basing our default_backoff on the client-go use case (backing off because we believe the api-server is unhealthy) i think it's reasonable to keep a single, shared trigger.

Fair enough. My counter would be that there are some cases where the error only concerns one of the watches (CRD not found, permission denied, and so on), in which case keeping the other streams going would still be a good thing.

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

And we must all pay our due respects, or pay the price. Signed-off-by: Teo Klestrup Röijezon <[email protected]>

nightkr · 2021-12-21T10:37:41Z

Are we ok adding a deny exception for itoa? It's a pretty tiny and isolated dependency, and we don't have much control over how quickly it percolates down the dependency tree.

clux · 2021-12-21T11:28:57Z

Are we ok adding a deny exception for itoa? It's a pretty tiny and isolated dependency, and we don't have much control over how quickly it percolates down the dependency tree.

Yeah. This is actually done in master. I am seeing some #[must_use] things in there that is also on master, maybe one of us needs to merge master into our branch.

…eanup Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

nightkr · 2021-12-21T11:40:15Z

Ok, merged in master now. Bypassed DCO-bot, since the failing commits are from the master merge.

clux · 2021-12-21T14:32:57Z

examples/node_watcher.rs

+    let obs = try_flatten_applied(StreamBackoff::new(
+        watcher(nodes, ListParams::default().labels("beta.kubernetes.io/os=linux")),
+        ExponentialBackoff::default(),
+    ));


Since we've been having chats about this outside of github, I thought I would highlight a temporary compromise here for now for anyone else reading it.

This api is not what we want long term

the flow of the program should be understood by reading left -> right (not like here where we have to start in the middle, and then descend leftwards, while keeping track of arguments and closing parens)

the individual pieces presented in this pr are still sensible

we can fix the flow in the future with stream extensions

The long term goal is to arrive at something like watcher(api, lp).backoff(backoff).watch_applies()

clux

have reviewed the actual commits herein - hoping the rest goes away once we merge this into my branch

clux and others added 30 commits November 22, 2021 15:51

Upgrade rustls

6fced4e

Signed-off-by: clux <[email protected]> Signed-off-by: kazk <[email protected]>

Refactor rustls_client_config

9e481da

Signed-off-by: kazk <[email protected]>

Refine errors for tls::rustls_tls

77255cc

Remove `Error::SslError` Signed-off-by: kazk <[email protected]>

Use native roots when root certs are not configured

6875ccd

Signed-off-by: kazk <[email protected]>

Add changelog entries

d501303

Signed-off-by: kazk <[email protected]>

Allow old rustls version from examples

988705f

Signed-off-by: kazk <[email protected]>

Merge pull request kube-rs#704 from kazk/chore/rustls020

6b23251

Possibly fix "the binary target name examples is forbidden" error

5f08c48

Signed-off-by: kazk <[email protected]>

Merge pull request kube-rs#730 from kazk/maybe-fix-dependabot

7e2c102

Include the object that failed in controller::Error::ReconcilerFailed

de7cb4c

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Merge pull request kube-rs#733 from teozkr/feature/controller-error-o…

fba749d

…bjref Include the object that failed in controller::Error::ReconcilerFailed

Add changelog link to kube-rs#733

cc055e1

deny(unsafe_code) -> forbid(unsafe_code) (kube-rs#734)

fa27811

to let cargo geiger show us as green Signed-off-by: clux <[email protected]>

dedicate a root file to adopters (kube-rs#735)

6537cf0

* dedicate a root file to adopters Signed-off-by: clux <[email protected]> * Update ADOPTERS.md Co-authored-by: Teo Klestrup Röijezon <[email protected]> Co-authored-by: Teo Klestrup Röijezon <[email protected]>

Remove TypeMeta fields from derived kind objects

fb4dc21

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Changelog

eb1162d

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Only serialize status if it is set

f4b7dab

To align with the old implementation Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Merge pull request kube-rs#739 from teozkr/feature/derive-remove-type…

20642bb

…meta-fields Remove TypeMeta fields from derived kind objects

upgrade subscriber and swap out eyre (kube-rs#744)

b112388

Signed-off-by: clux <[email protected]>

Remove unused features from warp dependency in examples (kube-rs#749)

32d2b61

`warp` depends on old `tokio-tungstenite`. Signed-off-by: kazk <[email protected]>

Update tower-http to 0.2.0 (kube-rs#748)

c543603

Change the bounds of body's error type for better compatibility with middleware like `DecompressionLayer`. Signed-off-by: kazk <[email protected]>

Replace RefreshTokenLayer with AsyncFilterLayer (kube-rs#752)

3f85b54

No need to add `#[doc(hidden)]` because they're not accessible and doesn't show up in docs. Signed-off-by: kazk <[email protected]>

release changelog 0.65 (kube-rs#753)

c71ef42

* release changelog 0.65 Signed-off-by: clux <[email protected]> * stick with convention Signed-off-by: clux <[email protected]>

release 0.65.0

8d27aad

Merge pull request kube-rs#755 from kube-rs/dependabot/cargo/dashmap-…

080c6ae

…5.0.0 Update dashmap requirement from 4.0.1 to 5.0.0

kate-goldenring and others added 5 commits December 16, 2021 09:46

upgrade make k3d to use k3d 5 - closes kube-rs#759

1c36768

Signed-off-by: clux <[email protected]>

fix clippy must_uses (kube-rs#761)

17ebf42

* fix clippy must_uses must_use all the things Signed-off-by: clux <[email protected]> * more must_use catches and fix make clippy Signed-off-by: clux <[email protected]>

nightkr self-assigned this Dec 21, 2021

nightkr added the runtime controller runtime related label Dec 21, 2021

nightkr commented Dec 21, 2021

View reviewed changes

nightkr requested a review from clux December 21, 2021 10:22

nightkr added 7 commits December 21, 2021 11:33

Use released backoff

927d630

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Factor out default Backoff

676f085

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Add note to watcher about backoff

c142ca1

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Added backoff to Controller

6aa0d79

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Changelog

2805d14

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

Revert Observer for now

0402c0b

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

The clippyman comes for us all, eventually

426a30a

And we must all pay our due respects, or pay the price. Signed-off-by: Teo Klestrup Röijezon <[email protected]>

nightkr force-pushed the backoff-watcher-/cleanup branch from 50f12e7 to 426a30a Compare December 21, 2021 10:33

Merge remote-tracking branch 'origin/master' into backoff-watcher-/cl…

33083f3

…eanup Signed-off-by: Teo Klestrup Röijezon <[email protected]>

nightkr force-pushed the backoff-watcher-/cleanup branch from b16f975 to 33083f3 Compare December 21, 2021 11:35

Fix build warnings

4fb49a8

Signed-off-by: Teo Klestrup Röijezon <[email protected]>

This comment has been minimized.

Sign in to view

clux reviewed Dec 21, 2021

View reviewed changes

clux approved these changes Dec 21, 2021

View reviewed changes

nightkr merged commit d02b1ec into kube-rs:backoff-watcher Dec 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backoff watcher cleanup #763

Backoff watcher cleanup #763

nightkr commented Dec 21, 2021

nightkr Dec 21, 2021

clux Dec 21, 2021

nightkr Dec 21, 2021

nightkr commented Dec 21, 2021 •

edited

Loading

clux commented Dec 21, 2021

nightkr commented Dec 21, 2021 •

edited

Loading

This comment has been minimized.

clux Dec 21, 2021

clux left a comment

Backoff watcher cleanup #763

Backoff watcher cleanup #763

Conversation

nightkr commented Dec 21, 2021

nightkr Dec 21, 2021

Choose a reason for hiding this comment

clux Dec 21, 2021

Choose a reason for hiding this comment

nightkr Dec 21, 2021

Choose a reason for hiding this comment

nightkr commented Dec 21, 2021 • edited Loading

clux commented Dec 21, 2021

nightkr commented Dec 21, 2021 • edited Loading

This comment has been minimized.

clux Dec 21, 2021

Choose a reason for hiding this comment

clux left a comment

Choose a reason for hiding this comment

nightkr commented Dec 21, 2021 •

edited

Loading

nightkr commented Dec 21, 2021 •

edited

Loading