📖 in-place updates: extension-declared Machine preservation on insufficient coverage - #14035
Conversation
2e21be3 to
f70ba1c
Compare
|
/area provider/core |
f70ba1c to
64ec2a9
Compare
|
/area documentation |
64ec2a9 to
035f877
Compare
elmiko
left a comment
There was a problem hiding this comment.
i think this makes sense to me, should we have an example of the condition that will be added?
035f877 to
3d4b49b
Compare
@elmiko Thanks for the suggestion. I've added an example of the condition here: https://github.com/kubernetes-sigs/cluster-api/compare/035f8772a8b41b6060c48d4663fac1fae88cd4b6..3d4b49bff1b8c201f32220dd6577cb8c0f3e991a |
elmiko
left a comment
There was a problem hiding this comment.
looks great, thanks for the update!
/lgtm
|
LGTM label has been added. DetailsGit tree hash: 94de56228b23df0b6eb046f904643ff4c42e2671 |
|
/assign @enxebre @fabriziopandini @sbueringer Got LGTM from @elmiko. This is a follow-up to the in-place updates proposal (#11029) — would appreciate an approve when you have a moment. |
|
/assign @enxebre @fabriziopandini @sbueringer |
|
This needs careful consideration, please don't ping us every few days (4 pings now in 4 days) |
Sorry for the frequent pings. I'll try to keep my fingers under control 😅. (I thought the assign hadn't gone through — it didn't seem to take effect — so I retried a couple of times.) |
|
Hi @sbueringer, checking in on this, does the current approach look reasonable? Happy to answer any questions. Thanks! |
|
I'm a little bit perplexed by the entire idea of allowing a change that cannot be performed by a system (or by a particular cluster), and then trying to stop it. Shouldn't the change be blocked upfront? Never the less, a few high level comments
Let me add a few line more on 2 and 3. While we usually think in term of workflows, e.g. a rollout, in Kubernetes controllers there is no notion of workflows, just an infinite sequence of micro decisions that starts by assessing the current state of system, and if possible, apply the next change shifting the system towards the desired state (all of this taking into considerations constraints like availability, max surge etc.) On top of that, the system allows to multiple change the desired state, no matter if the previous change is already fully rolled out or not, so, in the context of this proposal, it is not clear how to identify what the is the rollout to be stopped when the user changed spec, scaled up, remediation and autoscaling kicked in, and then the user changed spec again... Figuring out this requires a deep investigation in the code and addressing several edge cases and system caveats not yet included in this proposal
Frankly speaking I think that we should try to slice the problem is smaller, well scoped improvements aimed at providing a better granular control on how a rollout happens vs trying to achieve "all or nothing". See e.g. #14160 that goes in this direction |
3d4b49b to
5580976
Compare
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
91b5bf4 to
7eed282
Compare
|
Thanks @fabriziopandini for the detailed explanation, very helpful. I've reworked this PR based on your feedback: the preservation decision now lives in the Runtime Extension hook response ( PTAL when you have a chance. |
Signed-off-by: Liangquan Li <liangquan.li@smartx.com>
7eed282 to
a84257a
Compare
What this PR does:
Iterates on the In-Place Updates proposal by adding
preserveOnInsufficientCoverage, a new optional field on theCanUpdateMachine/CanUpdateMachineSethook responses. When an extension cannot cover the full desired-state diff, it can set this field to request that CAPI preserve the existing Machine rather than fall back to rolling replacement.why we need it:
For infrastructure that is expensive or destructive to replace (bare metal, local storage, license-bound nodes), the implicit fallback to replacement can destroy irreplaceable Machines. This lets the extension declare, per hook response, that the Machine should be preserved and the rollout reported as blocked, so operators can intervene.
The decision now lives in the Runtime Extension hook response rather than a user-facing KCP/MD API field, so this iteration introduces no core or KCP API changes.
Change scope:
1. Updates docs/proposals/20240807-in-place-updates.md (proposal)
2. Updates docs/proposals/20240807-in-place-updates-implementation-notes.md (MD and KCP implementation notes)
3. Documentation only — no code or API type changes
4. Field is optional; when omitted or false, existing rolling-replacement behavior is unchanged
Which issue(s) this PR fixes :
NONE