-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
🏃 Adds a control plane struct to clean up the reconciler #2728
Conversation
Signed-off-by: Chuck Ha <[email protected]>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chuckha 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/milestone v0.3.2
Name: names.SimpleNameGenerator.GenerateName(c.KCP.Name + "-"), | ||
Namespace: c.KCP.Namespace, | ||
Labels: ControlPlaneLabelsForClusterWithHash(c.Cluster.Name, c.ConfigurationHash()), | ||
OwnerReferences: []metav1.OwnerReference{owner}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to make this a controller owner ref?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the reconcile is not currently making this a controller owner reference, which is where this code originated. Do you think this should be a controller owner ref?
@chuckha These are all minor comments, can be reworked later, but feel free to hold if you wanna tackle them now |
Signed-off-by: Chuck Ha [email protected]
What this PR does / why we need it:
This is part of a larger effort to shrink the reconciler and reduce the use of annotations. This type adds a bunch of functions that do small tasks or access pieces of information in a clean way.
Because we are moving away from annotations on machines we need a way to idempotently find values. This holds those functions.
This type explicitly does not have any connections to any services and is just the logic that was found in the reconciler that is nicer outside of 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):Related to #2702