srl-labs / srl-controller

k8s controller for SR Linux nodes scheduled by KNE
BSD 3-Clause "New" or "Revised" License
16 stars 3 forks source link

Refactor using kubebuilder v3.8.0 #30

Closed hellt closed 1 year ago

hellt commented 1 year ago

Using kubebuilder v3.8.0 scaffolding and client-go 0.25.6 and controller-runtime v0.13.1

Additionally, copied clientset impl from #29 to use dynamic clients

hellt commented 1 year ago

Hi @marcushines I re-created the scaffolding using the kubebuilder v3.8.0 which works with client-go 0.25 and controller-runtime 0.13. Now the infra errors are gone, and only clientset tests are failing...

Since I am not familiar with dynamic-based clientsets, maybe you know a way out from these errors?

hellt commented 1 year ago

a quick note on the status of this PR - I am looking at removing the clientset entirely and use controller-runtime client instead as this seems to be the new recommended way of managing custom resrouces

marcushines commented 1 year ago

I don't see how that client removes the use case of a clientset?

the "reconcile" logic that is enacted by a controller is a piece of the puzzle which you are correct using a clientset isn't "required" as you are starting with runtime objects and jsut type asserting them to what you need

But the way kne uses these clients sets is really more like a public from an external client vs. the operator itself

I don't disagree that we should maybe evaluate the CRD structure we have in kne today and maybe adapt it to work alittle more "k8s crd like"

I also see there is a new kl8s aggregation layer api that also looks interesting

hellt commented 1 year ago

@marcushines i will create a draft PR against kne project to have a discussion around this approach. It looks promising and removes the need for clientset using the dynamic client while still working with structured types.

If all goes well I will open a PR today