🐛 Include machinepools in descendant count #4295
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XS
|
/milestone v0.4.0 /lgtm |
k8s-ci-robot
added
the
lgtm
| @@ -326,7 +326,8 @@ func (c *clusterDescendants) length() int { | |||
| return len(c.machineDeployments.Items) + | |||
| len(c.machineSets.Items) + | |||
| len(c.controlPlaneMachines.Items) + | |||
| len(c.workerMachines.Items) | |||
| len(c.workerMachines.Items) + | |||
| len(c.machinePools.Items) | |||
There was a problem hiding this comment.
is there a unit test we can add to validate this?
There was a problem hiding this comment.
Yeah can do, didn't notice any existing ones but I'm happy to add one.
There was a problem hiding this comment.
Added a simple unit test.
jimmidyson
changed the title
Without this, machinepool descendants are ignored in deletion logic and can be orphaned and unable to delete without manually deleting the finalizer.
jimmidyson
force-pushed
the
machine-pool-descendants
branch
from
83d2b02 to
78ecded
Compare
k8s-ci-robot
added
size/M
|
/test pull-cluster-api-test-main |
|
@fabriziopandini Yes I agree - I'd like to see this backported - this bug is affecting us in v0.3.x so a new v0.3.15 release including this fix would be really appreciated if at all possible. |
|
/lgtm @jimmidyson please open a backport PR against the release-0.3 branch. Thanks for catching and fixing this! |
k8s-ci-robot
added
the
lgtm
|
[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 |
k8s-ci-robot
added
the
approved
|
@CecileRobertMichon Backported in #4299 😄 |
What this PR does / why we need it:
Without this, machinepool descendeants are ignored in descendant count deletion logic and can be orphaned and unable to delete without manually deleting the finalizer.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #4296