-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
馃尡 add support for topology managed fields #5812
馃尡 add support for topology managed fields #5812
Conversation
/test pull-cluster-api-e2e-full-main |
dfbccf7
to
4cf5123
Compare
1ab6e4c
to
faa9f8d
Compare
lgtm pending the following from my side: |
lgtm pending squash + golangci-lint |
4e252fa
to
0b16f7e
Compare
Object: map[string]interface{}{ | ||
"metadata": map[string]interface{}{ | ||
"annotations": map[string]interface{}{ | ||
clusterv1.ClusterTopologyManagedFieldsAnnotation: "kubeadmConfigSpec.clusterConfiguration.imageRepository, " + |
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.
Should we consider using some sort of encoding for this annotation? The paths can be easily compressed, and we could b64 encode the output
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.
I kind of like the idea of this being human readable, so was considering something like
{ kubeadmConfigSpec { clusterConfiguration { imageRepository, version }, initConfiguration { ... } } }
But this requires a little encoder/parser and I did not include in this first iteration, but I really want to sort it out before the actual release
0b16f7e
to
7d080dd
Compare
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.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vincepri 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 |
What this PR does / why we need it:
This PR proposes a solution for ensuring the topology controller handles nicely the removal of fields from the template.
The implementation borrows the idea of managed fields from server-side apply in Kubernetes; when executing a change on templates we keep track of the fields the template expressed opinions on (provided a value).
Then, during the next reconcile on the next object, we consider all the managed fields as authoritative paths, meaning that if the corresponding value has been removed from the desired state, we enforce removal from the resulting object.
Which issue(s) this PR fixes:
Fixes #5811