From ea74209757f85dba6810f6c0a1c4b62cab21146c Mon Sep 17 00:00:00 2001 From: KARMAN SINGH TALWAR Date: Thu, 14 May 2026 01:01:30 +0530 Subject: [PATCH] cleanup bootstrap annotations on bootstrap-only rule deletion --- internal/controller/node_controller.go | 49 +++++++++++++++ .../nodereadinessrule_controller.go | 8 +++ .../nodereadinessrule_controller_test.go | 61 +++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/internal/controller/node_controller.go b/internal/controller/node_controller.go index a3e8aac..a2b809f 100644 --- a/internal/controller/node_controller.go +++ b/internal/controller/node_controller.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "strings" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -324,6 +325,54 @@ func (r *RuleReadinessController) removeTaintBySpec(ctx context.Context, node *c }) } +// cleanupBootstrapAnnotationsForRule removes bootstrap-completion annotations written by this +// rule from all nodes that currently match the rule's selector. Called during rule deletion so +// that stale annotations cannot bypass enforcement if the rule is later recreated with the same name. +func (r *RuleReadinessController) cleanupBootstrapAnnotationsForRule(ctx context.Context, rule *readinessv1alpha1.NodeReadinessRule, nodeList *corev1.NodeList) error { + log := ctrl.LoggerFrom(ctx) + annotationKey := fmt.Sprintf("readiness.k8s.io/bootstrap-completed-%s", rule.Name) + + var errs []string + for _, node := range nodeList.Items { + if !r.ruleAppliesTo(ctx, rule, &node) { + continue + } + if node.Annotations == nil { + continue + } + if _, exists := node.Annotations[annotationKey]; !exists { + continue + } + + log.Info("Removing bootstrap annotation during rule cleanup", + "node", node.Name, + "rule", rule.Name) + + if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + latest := &corev1.Node{} + if err := r.Get(ctx, client.ObjectKey{Name: node.Name}, latest); err != nil { + return err + } + if latest.Annotations == nil { + return nil + } + if _, exists := latest.Annotations[annotationKey]; !exists { + return nil + } + patch := client.MergeFrom(latest.DeepCopy()) + delete(latest.Annotations, annotationKey) + return r.Patch(ctx, latest, patch) + }); err != nil { + errs = append(errs, fmt.Sprintf("node %s: %v", node.Name, err)) + } + } + + if len(errs) > 0 { + return fmt.Errorf("failed to remove bootstrap annotations from some nodes: %s", strings.Join(errs, "; ")) + } + return nil +} + // Bootstrap completion tracking. func (r *RuleReadinessController) isBootstrapCompleted(ctx context.Context, nodeName, ruleName string) bool { // Check node annotation diff --git a/internal/controller/nodereadinessrule_controller.go b/internal/controller/nodereadinessrule_controller.go index 6feb083..9e6fa7c 100644 --- a/internal/controller/nodereadinessrule_controller.go +++ b/internal/controller/nodereadinessrule_controller.go @@ -192,6 +192,14 @@ func (r *RuleReconciler) reconcileDelete(ctx context.Context, rule *readinessv1a return ctrl.Result{RequeueAfter: time.Minute}, err } + if rule.Spec.EnforcementMode == readinessv1alpha1.EnforcementModeBootstrapOnly { + log.Info("Cleaning up bootstrap annotations for deleted rule", "rule", rule.Name) + if err := r.Controller.cleanupBootstrapAnnotationsForRule(ctx, rule, nodeList); err != nil { + log.Error(err, "Failed to cleanup bootstrap annotations for rule", "rule", rule.Name) + return ctrl.Result{RequeueAfter: time.Minute}, err + } + } + log.V(3).Info("Removing the rule from cache") r.Controller.removeRuleFromCache(ctx, rule.Name) diff --git a/internal/controller/nodereadinessrule_controller_test.go b/internal/controller/nodereadinessrule_controller_test.go index 03ebb90..d48d445 100644 --- a/internal/controller/nodereadinessrule_controller_test.go +++ b/internal/controller/nodereadinessrule_controller_test.go @@ -902,6 +902,67 @@ var _ = Describe("NodeReadinessRule Controller", func() { return err != nil && client.IgnoreNotFound(err) == nil }, time.Second*10).Should(BeTrue(), "Rule should be fully deleted") }) + + It("should remove bootstrap annotations from nodes when a bootstrap-only rule is deleted", func() { + bootstrapNode := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bootstrap-cleanup-node", + Labels: map[string]string{"kubernetes.io/hostname": "bootstrap-cleanup-node"}, + Annotations: map[string]string{ + "readiness.k8s.io/bootstrap-completed-bootstrap-cleanup-rule": "true", + "unrelated-annotation": "must-be-preserved", + }, + }, + } + bootstrapRule := &nodereadinessiov1alpha1.NodeReadinessRule{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bootstrap-cleanup-rule", + Finalizers: []string{finalizerName}, + }, + Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{ + Conditions: []nodereadinessiov1alpha1.ConditionRequirement{{Type: "TestReady", RequiredStatus: corev1.ConditionTrue}}, + NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"kubernetes.io/hostname": "bootstrap-cleanup-node"}}, + Taint: corev1.Taint{Key: "readiness.k8s.io/bootstrap-cleanup-taint", Effect: corev1.TaintEffectNoSchedule}, + EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly, + }, + } + + Expect(k8sClient.Create(ctx, bootstrapNode)).To(Succeed()) + Expect(k8sClient.Create(ctx, bootstrapRule)).To(Succeed()) + + // Initial reconcile to populate cache + _, err := ruleReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: "bootstrap-cleanup-rule"}, + }) + Expect(err).NotTo(HaveOccurred()) + + // Verify annotation is present before deletion + preDeleteNode := &corev1.Node{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "bootstrap-cleanup-node"}, preDeleteNode)).To(Succeed()) + Expect(preDeleteNode.Annotations).To(HaveKey("readiness.k8s.io/bootstrap-completed-bootstrap-cleanup-rule")) + + // Delete the rule + Expect(k8sClient.Delete(ctx, bootstrapRule)).To(Succeed()) + + // Trigger deletion reconcile + _, err = ruleReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: "bootstrap-cleanup-rule"}, + }) + Expect(err).NotTo(HaveOccurred()) + + // Verify bootstrap annotation is removed + Eventually(func(g Gomega) { + updatedNode := &corev1.Node{} + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "bootstrap-cleanup-node"}, updatedNode)).To(Succeed()) + g.Expect(updatedNode.Annotations).NotTo(HaveKey("readiness.k8s.io/bootstrap-completed-bootstrap-cleanup-rule"), + "Bootstrap annotation should be removed after rule deletion") + g.Expect(updatedNode.Annotations).To(HaveKeyWithValue("unrelated-annotation", "must-be-preserved"), + "Unrelated annotations must not be affected") + }, time.Second*10).Should(Succeed()) + + // Cleanup + _ = k8sClient.Delete(ctx, bootstrapNode) + }) }) Context("when a node is deleted", func() {