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

Emit datapoints into CRDs to auto generate constants #39

Open
aryan9600 opened this issue May 23, 2024 · 5 comments
Open

Emit datapoints into CRDs to auto generate constants #39

aryan9600 opened this issue May 23, 2024 · 5 comments
Labels
enhancement New feature or request priority/medium

Comments

@aryan9600
Copy link
Contributor

There are a lot of constants related to the Condition object that are a part of the Go API, but they're not reflected in the CRD since the Condition API is imported from upstream (kubernetes/client-go). Currently, we maintain a list of these constants in our repository manually, which means that we need to make sure they are in sync with the Go codebase.
To ease the burden of maintenance, we should figure out a way to emit datapoints into CRDs, so that a tool like kopium can parse them generate those constants in Rust.

@shaneutt
Copy link
Member

@clux, kinda curious if you have any thoughts on this one? 🤔

@clux
Copy link
Member

clux commented Oct 24, 2024

Are we talking about the constants you are defining in constants.rs. I don't see them in client-go.

If there are generic names that exists somewhere that have established meanings we could add them to kube-core, but if it's only properties in the CRDs, then you can try to wrangle the schema into something that looks more like an enum, e.g. oneOf with actual options (because then kopium can in theory create the right rust enum for you).

@clux
Copy link
Member

clux commented Oct 24, 2024

Although, if you are re-using the schema of Condition, in which the condition strings are untyped, I guess you would always lose type information (if there's probably no good ways to signal to that parent struct's serialization in golang to only allow a subset of strings).

..unless you want to patch the yaml schema by including oneOfs (which you perhaps could, the schema has no $ref's and it doesn't have to be the same as metav1 condition)

@shaneutt
Copy link
Member

Just to add some more context to this issue:

In upstream Gateway API there are types for specific status conditions, such as Programmed for the Gateway resource:

https://github.com/kubernetes-sigs/gateway-api/blob/5273415dcc628cb6e5b98e0de29e8d63a4e76ec5/apis/v1/gateway_types.go#L810

These types are available as a library to integrations that are written in Golang, for the convenience and conformance of using well-known status conditions.

To make these available to kube-rs users what we're doing currently is in our update.sh script (which automates updates to new Gateway API versions with kopium) we have hard-coded lists of them:

GATEWAY_CONDITION_CONSTANTS="GatewayConditionType=Programmed,Accepted,Ready"

Then our xtask collects conditions from the environment:

fn gen_condition_constants() -> Result<(), DynError> {

And we do our own codegen to generate those:

fn gen_const_enums(scope: &mut Scope, constants: String) {

It's a little messy at the moment, but the main implication is that for each Gateway API release we have to manually identify and add any new status conditions, and we maintain some manual generator code to emit them like so:

The root of the problem is these standard APIs in upstream not being built in a language agnostic sort of way (but with Gateway API at least that ship has sailed for now and might be a while before a chance for it to come back into port) so for now we'd like to find a way to automate this which is as least crunchy as possible.

Sanskar is suggesting that perhaps we can start adding metadata to the CRDs in upstream to enumerate these possibilities, which is reminiscent of an option we considered for solving #38 as well (though with #38 I think we're leaning more towards adding automatic identification directly into kopium because that may be more generally helpful for all APIs generated with kopium).

At a personal level, I would like to dig at the root problem: the standard API being non-language-agnostic. This however is an problem that's fundamental across upstream Kubernetes, and digging into it would probably be... engrossing to say the least. Sanskar's suggestion currently seems like the least crunchy at the moment to me, but I was curious if you had any other expertise or insights in your experience that might influence us here @clux? 🤔

@clux
Copy link
Member

clux commented Oct 26, 2024

Appreciate all the context! Yeah, I agree better language support would be the best in the end, but it indeed sound difficult. Like in this case, it's almost like Kubernetes needs its metav1 Condition to be generic over user types, rather than a static type with string fields. Don't know if this is something that could be expressed easily in go.


We could add some annotation injection for enums initially though, yeah. For simple enums, this feels doable in a single annotation ala codegen.kubernetes.io/enum-EnumName: VariantA,VariantB" (or something similar), and it's something kopium could parse and inject an enum for each without too much difficulty (with some care taken to respect derive requests etc), maybe another annotation for codegen.kubernetes.io/enum-default-EnumName: VariantB to solve the 95% use case (i kind of have to do these 2 types of injections myself a lot, so it would save me time as well).

The obvious downside is that all the normal schema benefits become manual; descriptions, defaults (as seen), more complicated enum serializations (e.g. non string types) would all require manual hacking. Could become complex. Then again, go doesn't do complex enums afaik, we could just do default + variants, generate some documentation, and solve the biggest issue this way (and it could be presumably be upstreamed less controversially into other user crds if we don't tie the annotation group name to rust - if kubernetes org is amenable to it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority/medium
Projects
Status: Next
Development

No branches or pull requests

3 participants