🌱 Simplify rollout planner #12899
🌱 Simplify rollout planner #12899
Conversation
k8s-ci-robot
added
area/machinedeployment
k8s-ci-robot
added
the
size/L
| // Prevent deletion of machines not yet acknowledged after a move operation. | ||
| // Note: as soon as an in-place upgrade is started, CAPI Should always try to complete it | ||
| // before taking further actions on the same machine. | ||
| if notAcknowledgeMoveReplicas.Has(m.Name) { | ||
| machinesSetMachines = append(machinesSetMachines, m) | ||
| continue | ||
| } |
There was a problem hiding this comment.
This if has been dropped because unnecessary (probably a left over of a previous iteration)
| // Limit the number of machines to be moved to avoid to exceed the final number of replicas for the target MS. | ||
| // TODO(in-place): consider using the DesiredReplicasAnnotation which is propagated together with the other decisions of the rollout planner | ||
| // another option to consider is to shift this check on the newMS (so we perform move, but we don't start the in-place upgrade). | ||
| machinesToMove := min(machinesToDeleteOrMove, ptr.Deref(scope.machineDeployment.Spec.Replicas, 0)-ptr.Deref(targetMS.Spec.Replicas, 0)) |
There was a problem hiding this comment.
As discussed during review, this would have been racy when implemented in the prod code for the oldMS reconciliation.
So now we are moving all the machines, and deferring to the newMS to get rid of exceeding machines.
This leads to the ideal results - perform the minimal number of in place upgrades/avoid unnecessary in place updates - under the assumption that machines upgrading in place have the highest deletion priority.
Notes about how to change the prod code have been changed accordingly
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 16d6547d160f6cbceb5bec2bee2ee1038fd18342
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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
3 participants
What this PR does / why we need it:
This PR simplifies fake ms controller in the rollout planner, to get rid of unnecessary code and to change an assumption that would have been racy when implementing the same in the prod code for the ms controller.
Which issue(s) this PR fixes:
Part of #12291
/area machinedeployment