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

Update key dependencies: kube-rs, tonic, and tokio #223

Closed
kate-goldenring opened this issue Jan 25, 2021 · 3 comments · Fixed by #361
Closed

Update key dependencies: kube-rs, tonic, and tokio #223

kate-goldenring opened this issue Jan 25, 2021 · 3 comments · Fixed by #361
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kate-goldenring
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We are currently using an outdated version of tokio, kube-rs, and tonic, and they are all interdependent because the latter two use tokio, and there cannot be two versions of tokio running in an application or tokio (a runtime for async apps) will error.
To get everything to tokio 1.0, kube-rs will need to be updated to v0.47.0+ and reqwest to v0.11.0.

This task also provides an opportunity to do pruning of dependencies. Async in Rust was fairly new when Akri was first being developed, so many crates were interwoven to accomplish tasks. Now, tokio may be able to accomplish what we were using std:::futures for in some scenarios.

I took a look at some of the work that will need to be done by the lucky updater:
Some changes that will need to be made due to upgrading tokio:

  • tokio_core is now a part of tokio, so the new/equivalent functions/structs will need to be used from tokio. Our signal module depends on this. This module may be able to be removed/ changed. Only the udev broker depends on it and its functionality may be uneccesary.
  • time::delay -> time::sleep
  • ONVIF discovery implementation needs to use oneshot instead of mpsc channels.

Some changes that will need to be made due to upgrading kube-rs:

Some changes that will need to be made due to upgrading tonic:

Other dependencies that will likely need to be updated:

  • hyper
  • h2 (will need rebase our patch branch)
  • tower
  • futures crates (e.g. stream is now a submodule of futures)
  • others probably

Some dependencies will probably be able to be removed such as:

  • tokio_core (now embedded in tokio)
  • futures-old (only need one futures crate now)

Note: This task of upgrading dependencies will cause a lot of code changes. It is best to not due this while another large feature change is underway, namely, modularizing discovery protocols #198.

@kate-goldenring kate-goldenring added enhancement New feature or request help wanted Extra attention is needed labels Feb 2, 2021
@bfjelds
Copy link
Collaborator

bfjelds commented Feb 3, 2021

Considerable changes to their custom resources interface. Looks like they added a kube_derive crate that can be utilized from kube::api::Resource::dynamic. Looks like they only support custom resources for K8s >= v1.17, so this update may limit us from supporting K8s 1.16. See the crd prefixed examples.

It looks like the Object<P,U> is still supported (https://github.com/clux/kube-rs/blob/d450509554ef3f601a0e2ba90524b9fae43c1a7b/kube/src/api/object.rs#L80), whicih is what we use for CRDs now. Maybe we can continue to use that and not be subjected to the K8s >- v1.17 requirement?

@clux
Copy link

clux commented Jul 16, 2021

Some changes that will need to be made due to upgrading kube-rs:

Just leaving a comment here for some clarification (since it's not often people do such a big upgrade jump, and just spotted this):

  • RawApi is actually Request now (but if you can use Api, that's obviously easier in terms of serialization)
  • CRDs are supported pre 1.17, but you have to use the #[kube(apiextensions = "v1beta1")] derive attr (although 1.17 is probably EOL'd soon now anyway?)

If you have more peripheral use cases (such as Object<P, U>, and using the straight no-client interface to the api (Request)) then we'd appreciate hearing the reasons behind that, and if there's anything we can do to improve it in kube-rs.

@kate-goldenring
Copy link
Contributor Author

@clux thanks for the comment! It bumped me to add needed context to this issue. I started the work to update kube-rs and switched to the new CustomResource model; however, since our CRDs contain Kubernetes types (PodSpec and ServiceSpec) as fields, I am currently blocked by this issue on k8s-openapi Arnavion/k8s-openapi#86. Patching k8s-openapi with the fork of the PR with the fix Arnavion/k8s-openapi#104 resolved the issue, so I am waiting on that to merge to proceed with the update. There doesn't seem to be anything else blocking us updating kube-rs and I will look into using #[kube(apiextensions = "v1beta1")] for backwards compat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants