-
Couldn't load subscription status.
- Fork 1.4k
✨ KCP/MS: Refactor BootstrapConfig/InfraMachine managedFields for in-place #12890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ KCP/MS: Refactor BootstrapConfig/InfraMachine managedFields for in-place #12890
Conversation
|
/test pull-cluster-api-e2e-main |
6e0bb4f to
32f7d33
Compare
|
/test pull-cluster-api-e2e-main |
Signed-off-by: Stefan Büringer [email protected]
32f7d33 to
79c8a90
Compare
|
/test pull-cluster-api-e2e-main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, the result of an amazing research before the implementation
A few nits from my side but sgtm
|
/test pull-cluster-api-e2e-main |
|
@fabriziopandini Thx for the quick review, PTAL :) |
eec306b to
21d33af
Compare
|
/test pull-cluster-api-e2e-main |
|
We can eventually iterate in future for inline managedFields in tests. /lgtm |
|
/approve |
|
LGTM label has been added. Git tree hash: b919c37355b934c9b113f896461f528ae1b26a14
|
|
[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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Stefan Büringer [email protected]
What this PR does / why we need it:
Today we only create BootstrapConfigs/InfraMachines but we never update them.
To be able to correctly update BootstrapConfigs/InfraMachines (e.g. during in-place updates)
we have to use SSA, e.g. for proper handling of co-ownership of fields.
Additionally we have to use two fieldManagers because we want to be able to:
This would not be possible with one fieldManager as the label & annotations sync would unset all other fields
This PR tackles this problem by:
Current managedField structure (CAPI <= v1.11)
New managedField structure (CAPI >= v1.12)
Machines will behave exactly the same, so there are no changes to Machine creation and no migration is necessary.
Everything below describes the new behavior with CAPI v1.12.
BoostrapConfig/InfraMachine creation
Directly afterward syncMachines will be called
After this, BootstrapConfigs/InfraMachines have the desired managedField structure and are ready for continuous
syncMachine calls to sync labels+annotations and also for triggering in-place updates.
When we trigger in-place updates the following happens
Migration from managedFields v1.11 => v1.12
So now the only missing piece is how do we migrate objects created with CAPI <= v1.11 to the new managedField structure of CAPI >= v1.12.
So after the first in-place update the managedFields of CAPI <= v1.11 object will be identical to he managedFields of CAPI >= v1.12.0 objects.
Because the fields of CAPI <= v1.11 objects are orphaned before that the first in-place update won't be able to unset fields.
Accordingly for Clusters that have been created with CAPI <= v1.11 it will only be possible to unset fields during in-place updates after:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Part of #12291