[release-1.9] ✨ Sync machine annotations to nodes #11980
Conversation
* Sync machine annotattions to nodes Signed-off-by: Nolan Brubaker <[email protected]> * Plumb manager argument through to controller Signed-off-by: Nolan Brubaker <[email protected]> * Manage annotations from previous reconciles Signed-off-by: Nolan Brubaker <[email protected]> * Add e2e test and refactor filtering Signed-off-by: Nolan Brubaker <[email protected]> * Documents new annotations This change documents the new cluster.x-k8s.io/labels-from-machine and cluster.x-k8s.io/annotations-from-machine annotations. * refactor: drop getManaged{Labels, Annotations} wrappers This refactors reconcileNode to remove the getManagedLabels and getManagedAnnotations wrapper functions, in favour of calling the libraries directly. * Moves Tests GetManagedLabels,GetManagedAnnotations This moves the tests for GetManagedLabels and GetManagedAnnotations to their respective util packages, as that's where they're now implemented. * Don't add known CAPI annotatoins to annotations-from-machine Signed-off-by: Nolan Brubaker <[email protected]> * Reduce length of annotation to fix tests Signed-off-by: Nolan Brubaker <[email protected]> * Add more details about a use case Signed-off-by: Nolan Brubaker <[email protected]> * Address review feedback Signed-off-by: Nolan Brubaker <[email protected]> * Apply suggestions from code review Co-authored-by: Stefan Büringer <[email protected]> Signed-off-by: Nolan Brubaker <[email protected]> * Additional review feedback Signed-off-by: Nolan Brubaker <[email protected]> --------- Signed-off-by: Nolan Brubaker <[email protected]> Co-authored-by: Theo Barber-Bany <[email protected]> Co-authored-by: Stefan Büringer <[email protected]>
k8s-ci-robot
added
area/machine
k8s-ci-robot
added
the
size/XL
|
Note: we can discuss at the community call on March 19, 2025. I understand a patch release is coming on March 18, but this is not urgent for that patch. |
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
Thx /lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 1b7684f4e98acaf18ff37531b5e508992083285b
|
|
Test failure probably due to kind image rate limiting (currently a known issue). We can retest once we want to merge the PR |
|
/retest Situation propably is better now. |
|
/lgtm I'm good with the backport 👍 |
|
Discussed at the 19 March community meeting - we'll put lazy consensus on this. If there are no objections by Friday, 20 March we can unhold this and merge for the next 1.9 patch release. |
|
/approve (keeping the hold for Friday) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
k8s-ci-robot
added
the
approved
What this PR does / why we need it:
Introduces propagation of a
Machine's annotations to the relevantNode(backport of #11813 since it doesn't cherry-pick cleanly).There is new functionality introduced by default with this PR, which may not be desirable for a backport. Specifically, any annotations on a
Machineusing thenode.cluster.x-k8s.iodomain prefix will be synced to the correspondingNode. This did not happen prior to this code change, and could be surprising. We had a brief Slack conversation about this, but I figure a PR offers a more durable/searchable place for the conversation and decision.Support for propagating arbitrary annotations through the
--additional-sync-machine-annotationsargument is possible, but this is not enabled by default, and thus should not pose any surprises to users./area machine