Skip to content

Commit 595ae83

Browse files
Address comments
1 parent cd6beb7 commit 595ae83

File tree

3 files changed

+17
-13
lines changed

3 files changed

+17
-13
lines changed

docs/book/src/reference/glossary.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ not limited to be expanded in the future.
268268

269269
Any change to a Machine spec, that is performed without deleting the machines and creating a new one.
270270

271-
Note: changing [in-place mutable fields](#in-place-mutable-fields) is not considered and in-place upgrade.
271+
Note: changing [in-place mutable fields](#in-place-mutable-fields) is not considered an in-place upgrade.
272272

273273
### Instance
274274

@@ -469,14 +469,14 @@ See [Topology Mutation](../tasks/experimental-features/runtime-sdk/implement-top
469469
# U
470470
---
471471

472-
### Update Lifecycle Hooks
473-
Is a set of Cluster API [Runtime Hooks](#runtime-hook) called when performing the "can update in-place" decision or
474-
when performing an [in-place update](#in-place-update).
475-
476472
### Update Extension
477473

478474
A [runtime extension provider](#runtime-extension-provider) that implements [Update Lifecycle Hooks](#update-lifecycle-hooks).
479475

476+
### Update Lifecycle Hooks
477+
Is a set of Cluster API [Runtime Hooks](#runtime-hook) called when performing the "can update in-place" decision or
478+
when performing an [in-place update](#in-place-update).
479+
480480
# W
481481
---
482482

docs/proposals/20240807-in-place-updates-implementation-notes.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ into the proposal or into the user-facing documentation for this feature.
77

88
## Notes about in-place update implementation for machine deployments
99

10-
- In place is always considered as potentially disruptive
11-
- in place must respect maxUnavailable
10+
- In-place update is always considered as potentially disruptive
11+
- in-place update must respect maxUnavailable
1212
- if maxUnavailable is zero, a new machine must be created first, then as soon as there is “buffer” for in-place, in-place update can proceed
1313
- when in-place is possible, the system should try to in-place update as many machines as possible.
1414
- maxSurge is not fully used (it is used only for scale up by one if maxUnavailable =0)

docs/proposals/20240807-in-place-updates.md

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ This approach, inspired by the principle of immutable infrastructure (the very s
100100

101101
Over time several improvement were made to Cluster API immutable rollouts:
102102
* Support for delete first strategy, thus making it easier to do immutable rollouts on bare metal / environments with constrained resources.
103-
* Support for [In place propagation of changes affecting Kubernetes objects only](https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20221003-In-place-propagation-of-Kubernetes-objects-only-changes.md), thus avoiding unnecessary rollouts
103+
* Support for [In-place propagation of changes affecting Kubernetes objects only](https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20221003-In-place-propagation-of-Kubernetes-objects-only-changes.md), thus avoiding unnecessary rollouts
104104
* Support for [Taint nodes with PreferNoSchedule during rollouts](https://github.com/kubernetes-sigs/cluster-api/pull/10223), thus reducing Pod churn by optimizing how Pods are rescheduled during rollouts.
105105

106106
Even if the project continues to improve immutable rollouts, most probably there are and there will always be some remaining use cases where it is complex for users to perform immutable rollouts, or where users perceive immutable rollouts to be too disruptive to how they are used to manage machines in their organization:
@@ -321,7 +321,7 @@ end
321321

322322
Please note that:
323323
- When in-place is possible, the system should try to in-place update as many machines as possible.
324-
In practice, this means that maxSurge might be not fully used (it is used only for scale up by one if maxUnavailable =0)
324+
In practice, this means that maxSurge might be not fully used (it is used only for scale up by one if maxUnavailable=0)
325325
- No in-place updates are performed when using rollout strategy on delete.
326326

327327
Please refer to [implementation note](./20240807-in-place-updates-implementation-notes.md) for more details about how the move operation is performed.
@@ -333,8 +333,12 @@ In order to do so:
333333
- The "can update in-place" decision is performed at MachineSet level by calling the `CanUpdateMachine` hook.
334334
- Before starting an in-place update, the Machine/InfraMachine/BootstrapConfig are updated
335335
to the new spec.
336-
- Machines updating in-place are considered not available, because in-place updates are always considered as potentially disruptive.
337-
- If maxSurge is zero, a new machine must be created first, then as soon as there is “buffer” for in-place, in-place update can proceed (to better understand this, you might want to consider that maxSurge in KCP is a way to express if the control plane rollout should scale-in or scale out).
336+
- If maxSurge is one, a new machine must be created first, then as soon as there is “buffer” for in-place, in-place update can proceed.
337+
- If maxSurge is zero, in-place update can proceed immediately.
338+
- Note: to better understand above points, you might want to consider that maxSurge in KCP is a way to express if the
339+
control plane rollout should be performed "scaling-out" or "scaling-in"
340+
- Note: KCP has its own definition of availability, that is preserved during a rollout no matter of it is performed using
341+
in-place updates or regular rollouts.
338342

339343
Notably, while KCP will always try to perform in-place whenever possible, KCP might decide to not perform in-place
340344
changes when it determines that this might impact the control-plane health.
@@ -366,7 +370,7 @@ Please refer to [implementation note](./20240807-in-place-updates-implementation
366370

367371
### Machine updates
368372

369-
Once a Machine is marked as `Updating` in place and the Machine's spec has been updated with the desired changes, the Machine controller takes over. This controller is responsible for calling the updaters and tracking the progress of those updaters and exposing this progress in the Machine conditions.
373+
Once a Machine is marked as `Updating` in-place and the Machine's spec has been updated with the desired changes, the Machine controller takes over. This controller is responsible for calling the updaters and tracking the progress of those updaters and exposing this progress in the Machine conditions.
370374

371375
The Machine controller currently calls only one updater (we are explicitly not trying to design a solution for ordering of execution at this stage. However, determining a specific ordering mechanism or dependency management between update extensions will need to be addressed in future iterations of this proposal).
372376

@@ -500,7 +504,7 @@ Given that there are still changes not covered, KCP continue with the next updat
500504
}
501505
```
502506

503-
The difference between current and desired will still be the change applied by the user;
507+
The difference between current and desired will still be the change applied by the user.
504508
If instead the `vsphere-vm-memory-update` would have declared its ability to perform some changes via a patch,
505509
those changes would be applied to change "current" state the second extension should consider.
506510

0 commit comments

Comments
 (0)