fix: remove bootstrap annotations from nodes when a bootstrap-only rule is deleted#248
Conversation
✅ Deploy Preview for node-readiness-controller canceled.
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Karman580 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
|
Welcome @Karman580! |
|
Hi @Karman580. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
f9429f3 to
ea74209
Compare
|
I added a comment here #247 (comment) - annotations are controller's metadata on the node, instead of cleaning up across the fleet for rule update, we could make the annotation reflect right state of the rule to decide whether this need another reconciliation. |
|
Instead of solving piecewise, we could plan for a better design for the annotation to address this. |
Description
reconcileDeletecleaned up managed taints but left bootstrap-completionannotations (
readiness.k8s.io/bootstrap-completed-<rule-name>) on nodeobjects indefinitely. Because the annotation key encodes only the rule
name, recreating a rule with the same name caused previously-bootstrapped
nodes to be skipped by
isBootstrapCompleted, potentially leaving themwithout the taint the new rule requires and allowing workloads to schedule
on nodes that should be gated.
This PR adds
cleanupBootstrapAnnotationsForRule, called fromreconcileDeleteaftercleanupTaintsForRuleforbootstrap-onlyrules.It iterates all nodes matching the rule's selector and removes the
annotation via a
RetryOnConflictMergeFrompatch, consistent with theexisting taint cleanup approach. The finalizer is not removed if any
annotation patch fails, so deletion is safely retried on the next cycle.
Related Issue
Fixes #247
Type of Change
/kind bug
Testing
should remove bootstrap annotations from nodes when a bootstrap-only rule is deletedinnodereadinessrule_controller_test.gomake testpasses — all existing tests continue to passChecklist
make testpassesmake lintpassesDoes this PR introduce a user-facing change?