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/util/annotation and pkg/util/finalizers should use metav1.Object instead of pkgruntime.Object for more ergonomic annotation handling #99

Open
gary-lgy opened this issue Jun 9, 2023 · 5 comments · May be fixed by #290
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@gary-lgy
Copy link
Member

gary-lgy commented Jun 9, 2023

k8s.io/apimachinery/pkg/runtime.Object does not have GetAnnotations and SetAnnotations methods. As a result the annotation util functions in pkg/controllers/util/annotation/annotation.go resort to calling meta.Accessor(obj) to convert the object. But meta.Accessor(obj) returns an error that must be propagated upwards.

In fact, all current usages of the annotation util functions pass in objects that implement metav1.Object interface, which allows calling SetAnnotations and GetAnnotations without returning errors.

@gary-lgy gary-lgy added enhancement New feature or request good first issue Good for newcomers labels Jul 6, 2023
@kevin1689-cloud
Copy link
Contributor

kevin1689-cloud commented Jul 9, 2023

@gary-lgy Hi, I want to try this, can you assign this to me?

@gary-lgy gary-lgy changed the title pkg/controllers/util/annotation/annotation.go should use metav1.Object instead of pkgruntime.Object for more ergonomic annotation handling pkg/util/annotation and pkg/util/finalizers should use metav1.Object instead of pkgruntime.Object for more ergonomic annotation handling Jul 20, 2023
@gary-lgy
Copy link
Member Author

@kevin1689-cloud Hi! The major refactor has been completed a while ago. Would you like to continue working on refactoring the annotation util functions as discussed in #144 ?

@kevin1689-cloud
Copy link
Contributor

kevin1689-cloud commented Nov 26, 2023

@kevin1689-cloud Hi! The major refactor has been completed a while ago. Would you like to continue working on refactoring the annotation util functions as discussed in #144 ?

Hi, thanks for the remind. I have been so busy with my work recently that I'm afraid I have no extra time to do the contribute, you can assign it to other people. By the way, wish kubeadmiral will be better and better!

@kevin1689-cloud kevin1689-cloud removed their assignment Nov 26, 2023
@gary-lgy
Copy link
Member Author

@kevin1689-cloud no worries. @z1cheng Thank you for your work on pkg/util/finalizers. Would you like to work on pkg/util/annotations as well? After that, we can close this issue.

@z1cheng
Copy link
Contributor

z1cheng commented Nov 28, 2023

@gary-lgy Sure, will finish it this week :)

@z1cheng z1cheng linked a pull request Dec 1, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants