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

pkg/conditions panics for k8s.io types #343

Open
aruiz14 opened this issue Dec 14, 2023 · 3 comments
Open

pkg/conditions panics for k8s.io types #343

aruiz14 opened this issue Dec 14, 2023 · 3 comments

Comments

@aruiz14
Copy link
Contributor

aruiz14 commented Dec 14, 2023

While working on adding a test for some unrelated pull request, I found that my code was panicking at some part.
Since I needed to use any object type for my test, I thought it was good to use to use a simple one from the Kubernetes core types, so I picked corev1.Service.

I turns out that the Conditions field for corev1.ServiceStatus uses metav1.Condition, whose fields include LastTransitionTime. This name is different from LastUpdatedTime, which is what wrangler's pkg/condition expects to set.
Beware that there is no call to IsValid() for the result of getFieldValue(value, "LastUpdateTime"), hence causing the panic when unconditionally trying to set its value.

On a separate note, this makes me wonder if the https://github.com/rancher/wrangler/tree/master/pkg/generated/controllers package is actually useful or it could be cleaned up.

@moio
Copy link
Contributor

moio commented Dec 14, 2023

@MbolotSuse @KevinJoiner @rmweir thoughts, especially about the last sentence?

@MbolotSuse
Copy link
Contributor

On a separate note, this makes me wonder if the https://github.com/rancher/wrangler/tree/master/pkg/generated/controllers package is actually useful or it could be cleaned up.

Near as I can tell, we make use of this package in Rancher (note that this is only one location, my quick search showed many more), so we can't remove it.

As far as the wider issue goes, it's possible that you found a bug in wrangler, or that the functionality that you noted wasn't intended to be used for core types (and isn't used for core types).

@moio
Copy link
Contributor

moio commented Dec 18, 2023

@MbolotSuse what's the opinion of maintiners?

Should this be fixed or at least documented?

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