Skip to content

model: Introduce KubernetesResource interface #2800

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

Closed
wants to merge 1 commit into from
Closed

model: Introduce KubernetesResource interface #2800

wants to merge 1 commit into from

Conversation

ruromero
Copy link

@ruromero ruromero commented Apr 7, 2020

Description of the change: Introduce KubernetesResource interface

Motivation for the change: This interface provides a simpler way to interact with metav1.Object and runtime.Object avoiding unnecessary type castings and more flexibility

func someFunc() {
	resources := []KubernetesResource{&corev1.Pod{}, &routev1.Route{}}
	iterateObjects(resources)
}

func iterateObjects(objects []KubernetesResource) {
	for _, o := range objects {
		doSomething(o)
		doSomethingElse(o)
	}
}

func doSomething(object runtime.Object) {
	//Do something
}

func doSomethingElse(object metav1.Object) {
	//Do something else
}

@estroz
Copy link
Member

estroz commented Apr 7, 2020

@ruromero are you using this interface in a controller? Can you provide a more detailed example of how you'd use this interface?

@ruromero
Copy link
Author

ruromero commented Apr 7, 2020

@ruromero are you using this interface in a controller? Can you provide a more detailed example of how you'd use this interface?

Yes we use it extensively in different operators. e.g. https://github.com/kiegroup/kie-cloud-operator/blob/master/pkg/controller/kieapp/kieapp_controller.go#L199

This is actually part of our operator-utils library and wanted to contribute it to the operator-sdk. If you find it useful we can think of more utilities to contribute.

This interface provides a simpler way to interact with metav1.Object and
runtime.Object avoiding unnecessary type castings and more flexibility

Signed-off-by: ruromero <[email protected]>
type KubernetesResource interface {
metav1.Object
runtime.Object
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it should be added? What is the need/scenario that can be solved with?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be an example where a generic method accepts any type of resource, sets the owner reference (metav1.Object) and creates the object (runtime.Object).

https://github.com/RHsyseng/operator-utils/blob/master/pkg/resource/write/writer.go#L62

@@ -21,6 +21,7 @@ require (
github.com/mattn/go-isatty v0.0.12
github.com/mitchellh/go-homedir v1.1.0
github.com/mitchellh/mapstructure v1.1.2
github.com/openshift/api v0.0.0-20200205133042-34f0ec8dab87
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we should add the OCP dep by default. wdyt @joelanford ?

@joelanford
Copy link
Member

This is a duplicate of kubernetes-sigs/controller-runtime#594. Can we continue the discussion there?

@camilamacedo86
Copy link
Contributor

After look the comment #2800 (comment) I understand that this suggestion would better addressed by via kubernetes-sigs/controller-runtime#594 as the solution applied there. So, I am closing this one.

However, please feel free to re-open if you think that it should still be addressed here or if I misunderstood something.

@ruromero
Copy link
Author

Would you re-consider this PR if the issue in controller-runtime does not progress? I still believe this small interface would help many developers writing better operators/controllers.

@estroz
Copy link
Member

estroz commented Apr 14, 2020

@ruromero I would open this PR in controller-runtime. An implementation makes abstractions easier to discuss.

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

Successfully merging this pull request may close these issues.

4 participants