Open Arnavion opened 6 years ago
Hey. Apologies for the delay. I've been very slow at getting back into my OSS stuff post-holiday.
YES! I'd absolutely love to see all the resources generated via codegen. Skimming your repo, it looks like a pretty straight-forward drop-in replacement. We could incorporate the codegen into this crate, but I think I'd actually lean toward making this crate depend on and re-export your generated resource types if you intend to manage it as a crate (which alternate clients or other tools could also use). I'd happily accept a PR for this, or I might try and carve out some time in the next couple weeks to make it happen.
I haven't dug in enough to grok client generation using the paths
. I'd love to better grasp what that would look like. But in general, I'm very in favor of implementing as much as possible via codegen as long as it doesn't unnecessarily hinder the external ergonomics of using the client.
My initial feeling was that the codegen would be manually fixed up a bit first instead of being used directly. You can see it in the example in the OP as well - it converts the dotted definition names into Rust modules so the generated types are very nested. For example your crate has ::resources::PodSpec
in resources/pod.rs
, whereas the tool's codegen has ::resources::io::k8s::api::core::v1::PodSpec
in resources/io/k8s/api/core/v1/pod_spec.rs
So I initially had this process in mind:
PodSpec
from the codegen to your crate.$codegen_out_dir/resources/io/k8s/api/core/v1/pod_spec.rs
to $kubeclient-rs/resources/pod_spec.rs
pub affinity: Option<::io::k8s::api::core::v1::Affinity>
field, so you copy $codegen_out_dir/resources/io/k8s/api/core/v1/affinity.rs
to $kubeclient-rs/resources/affinity.rs
, and change the field to pub affinity: Option<Affinity>
Pros:
resources::PodSpec
compared to resources::io::k8s::api::core::v1::PodSpec
PodSpec
by hand from scratch, the basic work of field names and types, serde annotations, comments, etc is already done.Cons:
apps::v1beta1::StatefulSet
/ apps::v1beta2::StatefulSet
/ apps::v1::StatefulSet
from coexisting.But now that I think about it, all the definitions do start with io.k8s.
, so I could have the tool strip that part of the namespace atleast. So you would end up with ::resources::api::core::v1::PodSpec
in resources/api/core/v1/pod_spec.rs
, which is more palatable and doesn't need manual fixup. And if it's going to be in a separate crate, it can be shortened even more to just ::api::core::v1::PodSpec
What do you think? If you're okay with <some_crate_name>::api::core::v1::PodSpec
, etc (both your crate as well as users of your crate would have to type it, even if just in a use
statement), I can change the codegen to output that and publish it as a crate.
Re: generating the client using paths
, I've been experimenting with it (not published to github) and it doesn't seem very useful.
The generated code is very un-Rust-like, eg
impl Client {
fn list_core_v1_namespaced_pod(...) -> ListCoreV1NamespacedPodResult {
}
}
enum ListCoreV1NamespacedPodResult {
Ok(::api::core::v1::PodList),
Unauthorized,
Other(::http::StatusCode),
}
instead of
impl Pod {
fn list(client: Client, ...) -> Result<::api::core::v1::PodList, ::http::StatusCode> {
}
}
The definitions are also not very good, eg one definition has two parameters both named path
, so the codegen has to rename one of them to path_
.
I think it's best to keep manually-generated clients for now.
<some_crate_name>::api::core::v1::PodSpec
seems pretty reasonable. Is everything in the api
namespace? If so, I can imagine stripping that too, but definitely not necessary.
Perhaps the biggest downfall I see of using codegen resources from an external crate is exemplified by the Secret
resource
Being focused on ergonomics, Secret
handles the encode on insert
and decode on get
, and I intentionally didn't make the data
attribute pub
. I'm chewing through the possibilities:
kubeclient::prelude
and accept that fields like data
are public (secret.insert
and secret.data.insert
do slightly different things)Pod
and Secret
with the API I want, but their inner fields (like PodSpec
) would come from your codegen types. The downside is that that resource types come from different crates (e.g. kubeclient::resources::Pod
contains <your_crate>::api::core::v1::PodSpec
, but I suspect most uses of kubeclient
wouldn't need to specify inner types too often).I'm not 100% decided, so definitely open to opinions here, but I lean slightly to starting with the 3rd approach (which is probably the quickest to implement), and consider the other approaches once I have a better feel for integrating the codegen types. In all cases, this will help fill out a lot of incomplete definitions!
Is everything in the
api
namespace?
io.k8s.apiextensions-apiserver
and io.k8s.apimachinery
have custom resource- and schema validation-related definitions.io.k8s.kube-aggregator
has aggregated API server-related definitions.io.k8s.kubernetes.pkg
has definitions that were deprecated and moved to io.k8s.api
, so they're just refs now. Currently the codegen compiles them to type aliases but I'm planning to ignore them completely.I'm chewing through the possibilities:
I think option 2 will make it error-prone to use. Options 1 and 3 can both work. I agree that option 3 is probably better.
I'll drop the codegen into its own GH repo over the next few days so you can pull it as a git crate dependency and play around with it.
awesome and thanks!
Done.
# Cargo.toml
[dependencies]
k8s-openapi = { git = "https://github.com/Arnavion/k8s-openapi-codegen", branch = "master" }
// main.rs
extern crate k8s_openapi;
fn main() {
let pod_spec: k8s_openapi::api::core::v1::PodSpec = Default::default();
println!("{:#?}", pod_spec);
}
PodSpec {
active_deadline_seconds: None,
affinity: None,
automount_service_account_token: None,
containers: [],
dns_config: None,
dns_policy: None,
host_aliases: None,
host_ipc: None,
host_network: None,
host_pid: None,
hostname: None,
image_pull_secrets: None,
init_containers: None,
node_name: None,
node_selector: None,
priority: None,
priority_class_name: None,
restart_policy: None,
scheduler_name: None,
security_context: None,
service_account: None,
service_account_name: None,
subdomain: None,
termination_grace_period_seconds: None,
tolerations: None,
volumes: None
}
@Arnavion In your example, the default value for containers
is []
, but other arrays (like volumes
or init_containers
) are None
. Why is containers
different?
EDIT: Sorry, I just saw that containers
is defined as required somewhere else in the spec.
Master now uses several generated types from your repo. I'm definitely open to further ideas around codegen, and eventually, we should sync up about publishing these crates, but for now, I have a project that should give me a chance to get some more experience with this client and its codegen'd internals.
Any plans to progress on this or it's an abandoned track? Thanks
@maximih
It's not abandoned [yet?], but definitely sidelined at the moment. I started an ncurses-based tool for using this (think htop
for kubernetes) to get a feel for the API and verify much of the functionality of this client, but a work project has my attention for at least a few more weeks. I do hope to get back to this soon - but really can't promise/commit to anything yet. I'd happily commit to discussing design changes and reviewing PRs in the meantime.
https://crates.io/crates/k8s-openapi/ is now published, with multiple supported versions of Kubernetes (version-specific modules gated by features) and a client API based on the http
crate that works with both sync and async HTTP clients.
I've made a tool k8s-openapi-codegen that generates Rust types from the Kubernetes OpenAPI spec. For example this is what the generated
PodSpec
looks like:I've tested that the generated code compiles but I haven't used it against actual apiserver responses. It might make sense to also generate clients using the
paths
part of the OpenAPI spec.You can run it yourself to check out all the generated types. Are you interested in incorporating the generated types into kubeclient?