-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 refactor failure domains logic out of controlplane internal package #4160
🌱 refactor failure domains logic out of controlplane internal package #4160
Conversation
f217a4b
to
cb305d8
Compare
/milestone v0.4.0 |
42605a7
to
31fb44c
Compare
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.
Thanks for this refactor @CecileRobertMichon !
Two nits not blocking and some question about machine_filters.go:
- does it makes sense to have machine filters a a separate package or should we merge machine filters into collections given that they are used together?
- should we leave KCP specific filters somewhere in KCP and move under util only generic machine filters (instead of moving everything)?
c0696cc
to
401a4ab
Compare
401a4ab
to
1324269
Compare
/lgtm |
/approve will follow up with #2062 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon 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 |
What this PR does / why we need it:
What: In preparation for #3358, this PR moves the failure domains business logic out of
controlplane/kubeadm/internal
intoutil/
. There should be no behavior changes as a result of this PR.Why: Nothing in
failure_domains.go
,machine_filters.go
andmachine_collection.go
is kubeadm or control plane specific. The logic should be available for reuse for other packages outside the KCP implementation.How:
FilterableMachineCollection
toMachines
func PickMost(c *ControlPlane, machines FilterableMachineCollection) *string
tofunc PickMost(failureDomains clusterv1.FailureDomains, groupMachines, machines collections.Machines) *string
) to allow make it generic, just likePickFewest
(nothing in the func logic is control plane specific).Note that none of these should be breaking changes as the modified code was previously part of the
internal
package which by Go convention cannot be imported.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 #3358