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

Change a way to check if the GVK is registered #1054

Closed
tenzen-y opened this issue Aug 11, 2023 · 3 comments · Fixed by #1140
Closed

Change a way to check if the GVK is registered #1054

tenzen-y opened this issue Aug 11, 2023 · 3 comments · Fixed by #1140
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@tenzen-y
Copy link
Member

What would you like to be cleaned:

Once kubernetes-sigs/controller-runtime#2425 commit is released, we need to use only !meta.IsNoMatchError(err) to check if the GVK is registered.

kueue/main.go

Lines 223 to 233 in b95d0fd

if _, err := mgr.GetRESTMapper().RESTMapping(cb.GVK.GroupKind(), cb.GVK.Version); err != nil {
// TODO: If the below PR is released, we need to change a way to check if the GVK is registered.
// REF: https://github.com/kubernetes-sigs/controller-runtime/pull/2425
// if !meta.IsNoMatchError(err) {
// return err
// }
var NoMatchingErr *discovery.ErrGroupDiscoveryFailed
if !meta.IsNoMatchError(err) && !errors.As(err, &NoMatchingErr) {
return err
}
log.Info("No matching API server for job framework, skip to create controller and webhook")

Why is this needed:

The behavior of mgr.GetRESTMapper().RESTMapping() will be changed once kubernetes-sigs/controller-runtime#2425 is released.

Follow-ups: #1046

@tenzen-y tenzen-y added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Aug 11, 2023
@mimowo
Copy link
Contributor

mimowo commented Sep 12, 2023

I think this is unblocked after kubernetes-sigs/controller-runtime#2472 is merged.

@tenzen-y
Copy link
Member Author

I think this is unblocked after kubernetes-sigs/controller-runtime#2472 is merged.

Oh, right. Thank you for the notifications!
/assign

@tenzen-y
Copy link
Member Author

I will bump K8s dependencies to address this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants