✨ Add in-place update hooks to API

[alexander-demicev]

What this PR does / why we need it:
This PR introduces the runtime hooks for in-place updates see In-place updates proposal and design doc for more details

umbrella issue #12291

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 #

/area runtime-sdk

[fabriziopandini]

Thanks for this PR

TBH, It is a little bit complex to think about all the implications without a deeper understanding of when/how those hooks are being called. e.g.

• is []string the best way to represent changes (e.g. how this will work when there changes to items in an array)
• do we need more than []string? e.g. reference to KCP/MD, corresponding templates, may be list or reference to controlled machines impacted by the change
• do we need something to link a set of proposed/accepted changes to in-place updates of the corresponding machines? (e.g. what will happen if the spec changes again in the meantime)

Considering I don't have a good answer to those questions yet, my gut feeling is that we should first look into where/how hooks are going to be called and then use learning to finalize & implement hooks, also because without the first part, hooks are not usable.

But no strong opinions, I'm also ok in merging a first release and then iterate, but this will probably require us to make exceptions e.g. if breaking changes will be required (probably not a blocker in this case, but I just want to bring this up).

[alexander-demicev]

@fabriziopandini Thank you for the great feedback, all your points are valid. We need to start somewhere with the implementation, and my intention is to split it into smaller PRs to make the review process much easier. The in-place update feature will be hidden behind a feature gate and won’t be announced as alpha until we all agree on it.

I’ll start implementing a reference updater as soon as possible, and at the same time, we’re going to begin building an experimental updater in Rancher to battle-test the idea. Hopefully, this will help clarify some of the open questions as we go.

Regarding the book, I completely agree that we need to create a dedicated page. I can open an issue to track it, but I’d prefer to wait a bit until we have some functional code to document.

[anmazzotti]

* is []string the best way to represent changes (e.g. how this will work when there changes to items in an array)

* do we need more than []string? e.g. reference to KCP/MD, corresponding templates, may be list or reference to controlled machines impacted by the change

Good call. It probably makes sense to stop at arrays and let the updater figure it out.
The Machine object is also updated/patched before the updater starts taking actions, so all data can be fetched if needed. I assume most implementations will be idempotent, simply fetch the to-update Machine or InfraMachine and reconcile that.

* do we need something to link a set of proposed/accepted changes to in-place updates of the corresponding machines? (e.g. what will happen if the spec changes again in the meantime)

I think the updater implementation has to keep its specific "updating" state. If multiple updates are triggered, the updater should know that a previous update is already running and the new request has to wait. I think the alternative is to forbid updates on resources annotated with runtime.cluster.x-k8s.io/pending-hooks: ExternalUpdate, but this may be too strict. For example you may want to fix a mistake while the in-place upgrade is running, for example if you set the new memoryMB value too low and you see nodes struggling, you may patch that with an updated value to fix your Cluster before it's too late.

All good points anyway, I also have some doubts over corner cases at the moment and I'd like to see some different updaters implemented, so I'm all up to merge and iterate as frequent as possible.

[sbueringer]

Took a look with Fabrizio, first round of feedback

[alexander-demicev]

@sbueringer @fabriziopandini All comments should be resolved

[sbueringer]

Thank you very much!

/lgtm

/assign @fabriziopandini

[k8s-ci-robot]

LGTM label has been added.

Details

Git tree hash: aff3bf25ef66bdcca63524b4811be38884c50653

[fabriziopandini]

/lgtm
/approve

Let's keep on with the momentum on this effort 🥳

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [fabriziopandini]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment