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

conditions::is_accepted for crds with await_condition #659

Merged
merged 7 commits into from
Oct 20, 2021
Merged

Conversation

clux
Copy link
Member

@clux clux commented Oct 18, 2021

replaces some hacky wait logic with Api::watch (which we now official no longer recommend after the super-crate pr) so ought to do what we say.

should probably add a timeout here to the callers, but beyond that, it seems to work already.

Signed-off-by: clux <sszynrae@gmail.com>
@clux clux requested a review from nightkr October 18, 2021 18:57
@clux clux closed this Oct 18, 2021
@nightkr
Copy link
Member

nightkr commented Oct 18, 2021

That wasn't a very long review window.. :P

@clux
Copy link
Member Author

clux commented Oct 18, 2021

it has conflicts with the super crate pr (and i failed to spot that i based it off that)..

at any rate the change is fe94f59

will re-open after that PR is done, or alternatively push it into that branch if it seems ok.

@nightkr
Copy link
Member

nightkr commented Oct 18, 2021

Can always retarget the PR against that branch, to make review a bit simpler.

@clux clux reopened this Oct 18, 2021
@clux clux changed the base branch from master to super-crate October 18, 2021 19:02
@clux
Copy link
Member Author

clux commented Oct 18, 2021

Ah, lol. I actually didn't know you could do this.

@nightkr
Copy link
Member

nightkr commented Oct 18, 2021

Yeah, it's.. not exactly clear lol.

@clux
Copy link
Member Author

clux commented Oct 18, 2021

Yeah, maybe Established is better from that description 🤔
From the kubernetes source it looks like it's accepted -> established: https://github.com/kubernetes/kubernetes/blob/ea0764452222146c47ec826977f49d7001b0ea8c/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status/naming_controller_test.go

FWIW, NamesAccepted never had any race conditions in the tests before, but sounds like that was just luck.

In either case, neither of these conditions seem to have any use outside of crds, so thinking conditions::is_crd_established for the name.

@nightkr
Copy link
Member

nightkr commented Oct 18, 2021

Sounds good to me.

Signed-off-by: clux <sszynrae@gmail.com>
kube-runtime/src/wait.rs Outdated Show resolved Hide resolved
clux added 5 commits October 19, 2021 23:22
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Oct 20, 2021

i think i am done here. although clippy is being a bit temperamental today.

@clux
Copy link
Member Author

clux commented Oct 20, 2021

pulling this into the super-crate pr

@clux clux merged commit 55d7018 into super-crate Oct 20, 2021
@clux clux deleted the await-names branch October 20, 2021 20:05
clux added a commit that referenced this pull request Oct 22, 2021
* move kube -> kube-client and try to make a facade crate

Signed-off-by: clux <sszynrae@gmail.com>

* move relative imports from kube-runtime -> kube-client

Signed-off-by: clux <sszynrae@gmail.com>

* client: fd -e rs | xargs sed -i 's/kube\:\:/kube_client\:\:/g'

also add a standalone readme
Signed-off-by: clux <sszynrae@gmail.com>

* kube-derive path correcting (now always points at kube-core)

Signed-off-by: clux <sszynrae@gmail.com>

* kube-derive to point dev-deps at kube-core as well

Signed-off-by: clux <sszynrae@gmail.com>

* fix kube not re-exporting config when client feature set

Signed-off-by: clux <sszynrae@gmail.com>

* Update kube/src/lib.rs

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

* Update kube-client/README.md

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

* ensure kube::core == kube-client::core == kube-core

so that we can use same paths equally depending on where we require them

Signed-off-by: clux <sszynrae@gmail.com>

* add #[kube(kube_crate = "kube")] attr

defaults to kube and will append ::core if kube or kube_client
but users can also use it with kube_core directly

Signed-off-by: clux <sszynrae@gmail.com>

* revert change to kube-dervive dev-dependencies

they need to look like they came from the facade crate

Signed-off-by: clux <sszynrae@gmail.com>

* rename to #[kube(kube...)] and document

Signed-off-by: clux <sszynrae@gmail.com>

* doc tweaks and fixes

Signed-off-by: clux <sszynrae@gmail.com>

* start writing an architecture md

Signed-off-by: clux <sszynrae@gmail.com>

* fix imports from new events moudle after crate-split

Signed-off-by: clux <sszynrae@gmail.com>

* architecture mostly done

Signed-off-by: clux <sszynrae@gmail.com>

* kube root crate documentation and fmt

Signed-off-by: clux <sszynrae@gmail.com>

* maintain kube doc imports from kube-client

Signed-off-by: clux <sszynrae@gmail.com>

* use kube imports in examples

Signed-off-by: clux <sszynrae@gmail.com>

* Allow `#[derive(CustomResource)]` to take an arbitrary path to `kube_core` (#658)

* Allow `#[derive(CustomResource)]` to take an arbitrary path to `kube_core`

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>

* Update kube-derive/src/lib.rs

Co-authored-by: Eirik A <sszynrae@gmail.com>

Co-authored-by: Eirik A <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

* Update kube-derive/src/lib.rs

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

minor doc improvements

Signed-off-by: clux <sszynrae@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

Update examples/README.md

Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>

Update architecture.md

Co-authored-by: kazk <kazk.dev@gmail.com>

* conditions::is_accepted for crds with await_condition (#659)

* add conditions::is_accepted for crds - closes #655

Signed-off-by: clux <sszynrae@gmail.com>

* rename to established based on new information + timout guard

Signed-off-by: clux <sszynrae@gmail.com>

* simplify a bit

Signed-off-by: clux <sszynrae@gmail.com>

* traitify condition

Signed-off-by: clux <sszynrae@gmail.com>

* docs for Condition fn trait

Signed-off-by: clux <sszynrae@gmail.com>

* add example for await_condition as well

Signed-off-by: clux <sszynrae@gmail.com>

* fix pedantic lints

Signed-off-by: clux <sszynrae@gmail.com>

* add root docs for condition waiting and fill doc gaps in runtime

Signed-off-by: clux <sszynrae@gmail.com>

* runtime::events: remove client validation + simplify module (#662)

- simpler names for snappier docs
- removes client side validation - see #654
- everything in one file in root (200 lines after all)
- avoids clippy lint

Signed-off-by: clux <sszynrae@gmail.com>

* Update architecture.md

Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>

Update architecture.md

Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>

Update architecture.md

Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>
Signed-off-by: clux <sszynrae@gmail.com>

* change core example in kube-client root

Signed-off-by: clux <sszynrae@gmail.com>

* add overlaps and crate delineation at the end

Signed-off-by: clux <sszynrae@gmail.com>

* Events api improvements (#663)

* add Resource::object_ref helper to simplify event recording

Signed-off-by: clux <sszynrae@gmail.com>

* a few more simplifications to events recorder

Signed-off-by: clux <sszynrae@gmail.com>

* fmt

Signed-off-by: clux <sszynrae@gmail.com>

* clippy + broken doc links

Signed-off-by: clux <sszynrae@gmail.com>

* one line clarification

Signed-off-by: clux <sszynrae@gmail.com>

* remove remnants of derive feature from kube-client

Signed-off-by: clux <sszynrae@gmail.com>

* only clone what is necessary

Signed-off-by: clux <sszynrae@gmail.com>

* code review doc comments

Signed-off-by: clux <sszynrae@gmail.com>

* 2021 edition (#664)

Signed-off-by: clux <sszynrae@gmail.com>

Co-authored-by: kazk <kazk.dev@gmail.com>
Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

example: port wait for NamesAccepted logit to use kube_runtime await_condition
2 participants