🌱 Move KCP controllers to internal #5929
Conversation
Signed-off-by: Stefan Büringer buringerst@vmware.com
k8s-ci-robot
added
cncf-cla: yes
|
@sbueringer: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
|
|
||
| // SetupWithManager sets up the reconciler with the Manager. | ||
| func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { | ||
| return (&kubeadmcontrolplanecontrollers.KubeadmControlPlaneReconciler{ |
There was a problem hiding this comment.
to be consistent with the other packages we moved, do we want to take the chance to rename this to kubeadmcontrolplanecontrollers.Reconciler?
There was a problem hiding this comment.
It looks to me like we only did this for the core-controller where the package name is the resource name.
The controllers moved into internal/controllers still have the full name because of controllers., e.g. KubeadmConfigReconciler, DockerClusterReconciler.
I think the idea is:
- if we only have a few controllers and it's not too complicated, keep the controllers in internal/controllers and still call them
controllers.ResourceReconciler. - if it get's to complicated create separate sub-packages per resource and then rename the struct, e.g.
cluster.Reconciler
There was a problem hiding this comment.
agreed, this is consistent with CABPK, CAPD, experiments
|
/lgtm |
k8s-ci-robot
added
the
lgtm
There was a problem hiding this comment.
/lgtm
/approve
@sbueringer please log an issue for finding a better solution for the files existing in controlplane/kubeadm/internal
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Signed-off-by: Stefan Büringer buringerst@vmware.com
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Ref #5455