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

Tracking issue for bringing back API operations-related code if there is demand #149

Open
Arnavion opened this issue Sep 7, 2023 · 6 comments

Comments

@Arnavion
Copy link
Owner

Arnavion commented Sep 7, 2023

API operations-related code such as the Pod::list() fn were dropped in the v0.20 release.

Upstream's OpenAPI spec changed a few things related to API operations in v1.28 that would've required non-trivial changes in k8s-openapi-codegen-common. I implemented some of those changes and could've implemented the rest of them given more time and effort, but it made me re-evaluate whether there was any point in keeping this code in the first place. AFAIK everyone (*) uses k8s-openapi through kube and thus only uses k8s-openapi for the resource types, not their associated methods, since kube has its own API for operations.

If you did use the methods provided by k8s-openapi, then:

  1. Do not update to k8s-openapi v0.20 or to kube releases that require k8s-openapi v0.20

  2. Comment here to let me know that you exist. Also tell me if there's any reason you can't use kube instead.


Depending on how much demand there is for the API operations-related code in k8s-openapi, there are a few possible outcomes:

  • I might implement the changes to the code generator needed to support API operations with v1.28+ and bring back the API operations-related code.

  • I might maintain patch releases of v0.19.x for some time, that continue to get updates other than v1.28+ support. ie v0.19.x patch releases will be limited to Kubernetes 1.27 and earlier, but they will get other bugfixes.

  • I might implement a separate API that is generic like kube's is but also sans-io like the original k8s-openapi one. (I already did this partially for the sake of this repository's tests, so it would be a matter of adding more methods to it, cleaning it up for public consumption, and maintaining it.)

  • Something else.


(*): I queried users at $dayjob, and asked on the Rust IRC channel which got a few responses.

@erebe
Copy link

erebe commented Sep 20, 2023

Hello,

We are using this method in order to query the stats of the node directly.

let node_summary_request = Node::connect_get_proxy_with_path(node_name, "stats/summary", Default::default()).unwrap();

It seems this method have been removed by the change you describe.

Could you let me know if it is correct ? And if yes, if you see any alternative in the latest version to achieve this ?
At the moment, we pinned the version to v0.19.0 to avoid the breaking change.

Regards

@Arnavion
Copy link
Owner Author

Arnavion commented Sep 20, 2023

Right, that is an API operation and so it has been removed. Also kube does not implement the full proxy API to .../proxy yet.

But since k8s-openapi never implemented the rest of the proxy response / stream handling, you presumably already have code to do that yourself. So you don't need support from kube for it anyway, just the http::Request that k8s-openapi used to give you. You can create it yourself as k8s-openapi used to: https://docs.rs/k8s-openapi/0.19.0/src/k8s_openapi/v1_27/api/core/v1/node.rs.html#178-199

@erebe
Copy link

erebe commented Sep 21, 2023

Thank you for the swift answer @Arnavion.
Indeed, we have a custom parser for the raw bytes returned by this call.
We will try to use http::Request directly as you suggest :)

Thank you for Kube-rs

@chubei
Copy link

chubei commented Sep 26, 2023

First of all thank you for your amazing work!

Responding to the comment request, we're using Pod::log_stream because Api::<Pod>::LogStream doesn't check (or expose) the status code of the HTTP response.

@Arnavion
Copy link
Owner Author

Yes, comparing Client::request_text (used by other CRUD API) with Client::request_stream (used by Pod::log_stream), the latter is missing the handle_api_errors or equivalent that would act on res.status(). Once it did that, that would solve the "check" problem, and it would also result in the status code appearing as Err(Error::Api(...)) which would solve the "expose" problem.

I didn't find an open issue on their tracker about this, so please open one and link it here.

@chubei
Copy link

chubei commented Sep 27, 2023

Thanks for the research! Opened kube-rs/kube#1300.

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

No branches or pull requests

3 participants