✨ Introduce controllerutil.Object interface #898

ruromero · 2020-04-14T10:27:37Z

Signed-off-by: ruromero [email protected]

Introduce an interface that combines metav1.Object and runtime.Object as discussed in #594

The main purpose of this interface is to reduce type conversions between the two and provide generic method signatures that can show potential errors at compile time and help developers to write simpler code.

This can make reconsider many places in this library and dependent ones its usage each time a cast is done, usually like the following:

//metav1.Object -> runtime.Object
ro, ok := owner.(runtime.Object)

//runtime.Object -> metav1.Object
o, err := meta.Accessor(obj)

An example where this interface can be used is https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/controller/controllerutil/controllerutil.go#L95

Signed-off-by: ruromero <[email protected]>

k8s-ci-robot · 2020-04-14T10:27:40Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2020-04-14T10:27:45Z

Welcome @ruromero!

It looks like this is your first PR to kubernetes-sigs/controller-runtime 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-runtime has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-04-14T10:27:45Z

Hi @ruromero. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ruromero · 2020-04-14T10:39:53Z

Added CLA

joelanford

As @Adirio mentioned here, it might be useful to figure out where we're currently making type assertions in one direction or the other and decide whether we want to update those function signatures as well.

Not sure if that's a blocker for introducing the type though.

Lastly, I know I mentioned in an SDK issue that we should discuss adding this union type to controller-runtime, but now that I'm looking closer and see that metav1.Object and runtime.Object are both provided by k8s.io/apimachinery I wonder if we should pursue adding this there?

joelanford · 2020-04-17T19:35:17Z

pkg/client/interfaces.go

+
+// KubernetesResource allows functions to work indistinctly with any resource that
+// implements both Object interfaces.
+type KubernetesResource interface {


Given that this is a union of two *.Object types would it be more intuitive for this to also include Object in the name?

Also, I wonder if the client package is the most appropriate place for this type. Nothing is really jumping out to me as an alternate though. Perhaps pkg/controller/controllerutil? 🤷‍♂️

Renamed and moved

Signed-off-by: ruromero <[email protected]>

DirectXMan12 · 2020-04-20T22:41:18Z

decide whether we want to update those function signatures as well.

Yeah, I think we want to do that. I think there's actually situations where some of those aren't a breaking change (I seem to recall adding additional interfaces to a compound interface casts transparently or something, but I'd have to double check).

vincepri · 2020-04-24T14:35:18Z

Given that this lives under controllerutil, could we just call it Object instead of KubernetesObject ?

vincepri · 2020-04-24T14:35:32Z

/ok-to-test

Signed-off-by: ruromero <[email protected]>

ruromero · 2020-05-06T12:57:46Z

renamed @vincepri

vincepri · 2020-05-06T15:36:08Z

/lgtm
/assign @DirectXMan12

vincepri · 2020-05-13T15:00:47Z

/retitle ✨ Introduce controllerutil.Object interface

vincepri

/approve

k8s-ci-robot · 2020-05-20T21:03:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ruromero, vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

vincepri · 2020-05-20T21:12:52Z

Should we backport this? I'd be useful for other projects as well

luxas · 2020-07-08T12:52:54Z

Thanks for this change 💯!
I'd love to have this under core k8s.io/apimachinery.
Would someone be up for "backporting" this to there?

vincepri · 2020-07-08T14:33:27Z

@luxas Happy to help, although need some pointers to where we should place this interface :)

luxas · 2020-07-14T11:01:59Z

Without knowing exactly, maybe https://pkg.go.dev/k8s.io/[email protected]/pkg/api/meta?tab=doc would be a good place?

Prep code for supporting k8s 1.21: * Add context.Context parameter to Reconcile() functions [1] * Add RBAC get/create/update coordination.k8s.io/v1 Leases (replacing a configmap for leader election locks) [2] * Adapt event handler code to reflect simplification [3] * Adapt to apimachinery/pkg/runtime Log deprecation (replaced with pkg/client Log) [4] * client.Object is preferred in favor of runtime.Object (v0.7.0 release) [5] * Use admission/v1 instead of v1beta1 for webhook requests [6] [1] - kubernetes-sigs/controller-runtime#1054 [2] - kubernetes-sigs/controller-runtime#1144 [3] - kubernetes-sigs/controller-runtime#1119 [4] - kubernetes-sigs/controller-runtime#1105 [5] - kubernetes-sigs/controller-runtime#898 kubernetes-sigs/controller-runtime#1118 [6] - kubernetes-sigs/controller-runtime#1284 kubernetes-sigs/controller-runtime@a32b29d Signed-off-by: Angel Misevski <[email protected]>

Introduce KubernetesResource interface

044e189

Signed-off-by: ruromero <[email protected]>

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 14, 2020

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 14, 2020

k8s-ci-robot requested review from DirectXMan12 and shawn-hurley April 14, 2020 10:27

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 14, 2020

ruromero changed the title ~~:sparkles Introduce KubernetesResource interface~~ ✨ Introduce KubernetesResource interface Apr 14, 2020

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Apr 14, 2020

joelanford reviewed Apr 17, 2020

View reviewed changes

Rename and move KubernetesObject

1d05f23

Signed-off-by: ruromero <[email protected]>

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 24, 2020

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 6, 2020

ruromero force-pushed the k8sresource branch from f15bad2 to 1486938 Compare May 6, 2020 12:56

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 6, 2020

Rename to Object

ea2e6b1

Signed-off-by: ruromero <[email protected]>

ruromero force-pushed the k8sresource branch from 1486938 to ea2e6b1 Compare May 6, 2020 12:56

k8s-ci-robot assigned DirectXMan12 May 6, 2020

k8s-ci-robot assigned vincepri May 6, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 6, 2020

joelanford mentioned this pull request May 13, 2020

✨ Add ContainsFinalizer helper to the controllerutil #922

Merged

k8s-ci-robot changed the title ~~✨ Introduce KubernetesResource interface~~ ✨ Introduce controllerutil.Object interface May 13, 2020

vincepri mentioned this pull request May 13, 2020

✨ KCP adopts existing machines kubernetes-sigs/cluster-api#2489

Merged

7 tasks

vincepri added this to the v0.6.0 milestone May 13, 2020

vincepri approved these changes May 20, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 20, 2020

k8s-ci-robot merged commit 934bda8 into kubernetes-sigs:master May 20, 2020

ruromero deleted the k8sresource branch May 22, 2020 06:49

alvaroaleman mentioned this pull request May 22, 2020

Expose a union interface for metav1.Object and runtime.Object #594

Closed

This was referenced May 22, 2020

Migrate controllerutil AddFinalizer and RemoveFinalizer controllerutil.Object and deprecate *WithError functions #959

Closed

✨ Migrate controllerutil AddFinalizer and RemoveFinalizer to controllerutil.Object and deprecate *WithError functions #962

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Introduce controllerutil.Object interface #898

✨ Introduce controllerutil.Object interface #898

ruromero commented Apr 14, 2020

k8s-ci-robot commented Apr 14, 2020

k8s-ci-robot commented Apr 14, 2020

k8s-ci-robot commented Apr 14, 2020

ruromero commented Apr 14, 2020

joelanford left a comment

joelanford Apr 17, 2020

ruromero Apr 20, 2020

DirectXMan12 commented Apr 20, 2020

vincepri commented Apr 24, 2020

vincepri commented Apr 24, 2020

ruromero commented May 6, 2020

vincepri commented May 6, 2020

vincepri commented May 13, 2020

vincepri left a comment

k8s-ci-robot commented May 20, 2020

vincepri commented May 20, 2020

luxas commented Jul 8, 2020

vincepri commented Jul 8, 2020

luxas commented Jul 14, 2020

✨ Introduce controllerutil.Object interface #898

✨ Introduce controllerutil.Object interface #898

Conversation

ruromero commented Apr 14, 2020

k8s-ci-robot commented Apr 14, 2020

k8s-ci-robot commented Apr 14, 2020

k8s-ci-robot commented Apr 14, 2020

ruromero commented Apr 14, 2020

joelanford left a comment

Choose a reason for hiding this comment

joelanford Apr 17, 2020

Choose a reason for hiding this comment

ruromero Apr 20, 2020

Choose a reason for hiding this comment

DirectXMan12 commented Apr 20, 2020

vincepri commented Apr 24, 2020

vincepri commented Apr 24, 2020

ruromero commented May 6, 2020

vincepri commented May 6, 2020

vincepri commented May 13, 2020

vincepri left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 20, 2020

vincepri commented May 20, 2020

luxas commented Jul 8, 2020

vincepri commented Jul 8, 2020

luxas commented Jul 14, 2020