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

move kube -> kube-client and try to make a facade crate #652

Merged
merged 28 commits into from Oct 22, 2021
Merged

Conversation

clux
Copy link
Member

@clux clux commented Oct 13, 2021

quick play around with #651

kind of just moves kube the entire crate and avoids the error nesting problem by re-exporting kube_client::Error as kube::Error, not sure if that's the best solution. kind of delays the error story out further, but that's also good that we don't have to tackle it here.

big changes are:

  • kube/cargo.toml -> kube-client/cargo.toml
  • new kube/cargo.toml has features but just defers to dependency set in kube-client
  • kube/src/lib.rs -> kube-client/src/lib.rs (diff but removes kube-derive re-export)
  • new kube/src/lib.rs is now exporting the same stuff but via modules from kube-client (and kube-derive)
  • runtime mod reexport from new kube
  • kube-derive points at kube_core for its trait definitions

@clux clux linked an issue Oct 13, 2021 that may be closed by this pull request
@clux clux marked this pull request as ready for review October 14, 2021 21:19
@clux
Copy link
Member Author

clux commented Oct 14, 2021

ok, everything is passing at least now so this is indeed viable.

a bit of a faff internally with re-exporting features yet another time from the facade into kube into kube-core (and if we're coming from examples it's one extra step <- examples simplified now as well), but generally pretty smooth.

kube/src/lib.rs Outdated Show resolved Hide resolved
kube-client/README.md Outdated Show resolved Hide resolved
kube/src/lib.rs Show resolved Hide resolved
@clux clux linked an issue Oct 17, 2021 that may be closed by this pull request
@clux clux mentioned this pull request Oct 17, 2021
@clux clux force-pushed the super-crate branch 2 times, most recently from 1345143 to 34420ea Compare October 17, 2021 20:35
@clux
Copy link
Member Author

clux commented Oct 17, 2021

Few things left:

Will try to finish the first 2 tomorrow. In the mean time, any quick look-overs of the new lib.rs examples and architecture.md would be appreciated.

kube-derive/src/lib.rs Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
kube/src/lib.rs Outdated Show resolved Hide resolved
kube/src/lib.rs Outdated Show resolved Hide resolved
kube/src/lib.rs Outdated Show resolved Hide resolved
kube/src/lib.rs Outdated Show resolved Hide resolved
kube/src/lib.rs Outdated Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
clux and others added 11 commits October 18, 2021 19:26
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
also add a standalone readme
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>
Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Co-authored-by: kazk <kazk.dev@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
so that we can use same paths equally depending on where we require them

Signed-off-by: clux <sszynrae@gmail.com>
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>
they need to look like they came from the facade crate

Signed-off-by: clux <sszynrae@gmail.com>
clux and others added 3 commits October 18, 2021 19:26
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
…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>
@clux clux force-pushed the super-crate branch 2 times, most recently from fa1ee36 to 293a11c Compare October 18, 2021 18:27
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>
@nightkr
Copy link
Member

nightkr commented Oct 18, 2021

Lovely how even trivial docs changes get tripped up by the DCO bot :/

On a more practical note, the constant force-pushing that this leads to is really annoying when trying to follow links from email notifications.

@clux
Copy link
Member Author

clux commented Oct 18, 2021

Yeah, it's almost easier to search and replace doc changes yourself on this scale. Thankfully, it was time to do a rebase anyway.

@clux
Copy link
Member Author

clux commented Oct 18, 2021

On a more practical note, the constant force-pushing that this leads to is really annoying when trying to follow links from email notifications.

Ah, yeah, that is true. I tried to limit most the force pushing here to the very end, but maybe I should wait until PRs are complete ready to go / approved in the future (although then we have to deal with emails from failing DCO bot)..

* 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>
@clux
Copy link
Member Author

clux commented Oct 20, 2021

I'm pretty happy with how documentation looks now all over. After the last cleanup in #662 is done, i'll probably want to merge both and do a release.

Are we otherwise happy with this PR? Happy to spend some more time on it if there's something.

Copy link
Member

@kazk kazk left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Have you tried using this branch from outside? I remember #525 (comment) worked fine in this workspace, but didn't outside. Just wanted to make sure we don't have anything like that.

@clux
Copy link
Member Author

clux commented Oct 20, 2021

Ah. That's a good point. I'll verify in controller-rs first.

@clux
Copy link
Member Author

clux commented Oct 20, 2021

Ok. It does seem to work fine from controller-rs and version-rs, but going to hold off until tomorrow. Want to POC the EventRecorder in controller-rs and the api still has some awkwardness to it that I would like to iron out before making a release.

architecture.md Outdated Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
architecture.md Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
architecture.md Outdated Show resolved Hide resolved
ws = ["client", "tokio-tungstenite", "rand", "kube-core/ws"]
oauth = ["client", "tame-oauth"]
gzip = ["client", "tower-http/decompression-gzip"]
client = ["config", "__non_core", "hyper", "http-body", "tower", "tower-http", "hyper-timeout", "pin-project", "chrono", "jsonpath_lib", "bytes", "futures", "tokio", "tokio-util", "either"]
Copy link
Member

Choose a reason for hiding this comment

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

What does it mean to build kube-client without client?

Copy link
Member Author

Choose a reason for hiding this comment

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

you can still get the config out of it. effectively it's similar to old kube, but could possibly strip the kube-client a bit more now.

kube-client/src/lib.rs Outdated Show resolved Hide resolved
clux and others added 4 commits October 21, 2021 19:59
- 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>
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>
Signed-off-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Oct 21, 2021

Bah. DCO bot is not super reliable in its suggestions for how to fix missing signoffs across big prs... Doing a big force push again.

* 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>
@clux
Copy link
Member Author

clux commented Oct 22, 2021

Ok. Last tweak to events api + rust 2021 upgrade is in here. Will add CHANGELOG entries and release tonight if all goes as planned :-)

@clux clux merged commit 8effb4f into master Oct 22, 2021
@clux clux deleted the super-crate branch October 22, 2021 18:18
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.

kube super-crate write an architecture.md
3 participants