project-codeflare / appwrapper

AppWrapper controller for Kueue
https://project-codeflare.github.io/appwrapper/
Apache License 2.0
4 stars 4 forks source link

Remove dependency on `k8s.io/kubernetes` #142

Closed astefanutti closed 1 month ago

astefanutti commented 1 month ago

WHY

k8s.io/kubernetes is not meant to be depended on as explain in kubernetes/kubernetes#79384.

WHAT

This causes issues with some tools.

HOW

Replace the dependency:

https://github.com/project-codeflare/appwrapper/blob/37ce35b914eb89715cf5f8c01e928e3124ac3abc/pkg/utils/utils.go#L30

With the right one from k8s.io/api/core/v1.

astefanutti commented 1 month ago

/cc @dgrove-oss

dgrove-oss commented 1 month ago

I'm pretty sure k8s.io/api/core/v1 isn't enough (that was what I initially tried last week when fixing #131). Just depending on core/v1 doesn't get the "right" AddToScheme function implementation that causes the generated defaulting functions to be registered.

I'll try to figure out some how to get those defaulters without the direct dependency.

dgrove-oss commented 1 month ago

Looks like we can't get the defaulters through one of the published k8s modules so we will have to replicate the needed functionality instead. This will be a bit fragile, but appears to be unavoidable if we can't access the generated functions.

tardieu commented 1 month ago

Could we run defaulter-gen ourselves?

astefanutti commented 1 month ago

It's annoying but not blocking, so we can leave it as if we think there isn't any "good" solution.

Ideally these defaulters would be published as part of https://github.com/kubernetes/api.

dgrove-oss commented 1 month ago

In the short term, I'm going to copy in the generated defaulter (and associated helpers) I'm using into our utils package as package-private functions. That will let us clean the dependency. Longer term, I'd like to kick off a discussion with the Kueue maintainers about whether they really need to use such a picky and fragile notion of equality.

astefanutti commented 1 month ago

Sounds good, thanks.