Skip to content

Commit 65f9662

Browse files
authored
propagate context and use merge patch in bootstrap completion tracking (#173)
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
1 parent 62e8b4f commit 65f9662

File tree

2 files changed

+63
-7
lines changed

2 files changed

+63
-7
lines changed

internal/controller/node_controller.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func (r *RuleReadinessController) processNodeAgainstAllRules(ctx context.Context
125125
}
126126

127127
// Skip if bootstrap-only and already completed
128-
if r.isBootstrapCompleted(node.Name, rule.Name) && rule.Spec.EnforcementMode == readinessv1alpha1.EnforcementModeBootstrapOnly {
128+
if r.isBootstrapCompleted(ctx, node.Name, rule.Name) && rule.Spec.EnforcementMode == readinessv1alpha1.EnforcementModeBootstrapOnly {
129129
log.Info("Skipping bootstrap-only rule - already completed",
130130
"node", node.Name, "rule", rule.Name)
131131
continue
@@ -275,10 +275,10 @@ func (r *RuleReadinessController) removeTaintBySpec(ctx context.Context, node *c
275275
}
276276

277277
// Bootstrap completion tracking.
278-
func (r *RuleReadinessController) isBootstrapCompleted(nodeName, ruleName string) bool {
278+
func (r *RuleReadinessController) isBootstrapCompleted(ctx context.Context, nodeName, ruleName string) bool {
279279
// Check node annotation
280280
node := &corev1.Node{}
281-
if err := r.Get(context.TODO(), client.ObjectKey{Name: nodeName}, node); err != nil {
281+
if err := r.Get(ctx, client.ObjectKey{Name: nodeName}, node); err != nil {
282282
return false
283283
}
284284

@@ -306,14 +306,15 @@ func (r *RuleReadinessController) markBootstrapCompleted(ctx context.Context, no
306306
}
307307
}
308308

309+
patch := client.MergeFrom(node.DeepCopy())
310+
309311
// Initialize annotations if nil
310312
if node.Annotations == nil {
311313
node.Annotations = make(map[string]string)
312314
}
313315

314316
node.Annotations[annotationKey] = "true"
315-
// TODO: consider replacing this with SSA
316-
return r.Update(ctx, node)
317+
return r.Patch(ctx, node, patch)
317318
})
318319

319320
if err != nil {

internal/controller/nodereadinessrule_controller_test.go

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
499499
ruleName := "bootstrap-test-rule"
500500

501501
// Initially not completed
502-
completed := readinessController.isBootstrapCompleted(nodeName, ruleName)
502+
completed := readinessController.isBootstrapCompleted(ctx, nodeName, ruleName)
503503
Expect(completed).To(BeFalse())
504504

505505
// Create a node for testing
@@ -516,9 +516,64 @@ var _ = Describe("NodeReadinessRule Controller", func() {
516516

517517
// Should now be completed
518518
Eventually(func() bool {
519-
return readinessController.isBootstrapCompleted(nodeName, ruleName)
519+
return readinessController.isBootstrapCompleted(ctx, nodeName, ruleName)
520520
}).Should(BeTrue())
521521
})
522+
523+
It("should return false when context is cancelled", func() {
524+
nodeName := "bootstrap-ctx-test-node"
525+
ruleName := "bootstrap-ctx-test-rule"
526+
527+
// Create a node with the bootstrap annotation already set
528+
node := &corev1.Node{
529+
ObjectMeta: metav1.ObjectMeta{
530+
Name: nodeName,
531+
Annotations: map[string]string{
532+
"readiness.k8s.io/bootstrap-completed-" + ruleName: "true",
533+
},
534+
},
535+
}
536+
Expect(k8sClient.Create(ctx, node)).To(Succeed())
537+
defer func() { _ = k8sClient.Delete(ctx, node) }()
538+
539+
// Verify it returns true with a valid context
540+
Expect(readinessController.isBootstrapCompleted(ctx, nodeName, ruleName)).To(BeTrue())
541+
542+
// A cancelled context should cause the Get to fail, returning false
543+
cancelledCtx, cancel := context.WithCancel(ctx)
544+
cancel()
545+
Expect(readinessController.isBootstrapCompleted(cancelledCtx, nodeName, ruleName)).To(BeFalse())
546+
})
547+
548+
It("should set bootstrap annotation via patch in markBootstrapCompleted", func() {
549+
nodeName := "bootstrap-patch-test-node"
550+
ruleName := "bootstrap-patch-test-rule"
551+
552+
// Create a node with existing annotations that should be preserved
553+
node := &corev1.Node{
554+
ObjectMeta: metav1.ObjectMeta{
555+
Name: nodeName,
556+
Annotations: map[string]string{
557+
"existing-annotation": "should-be-preserved",
558+
},
559+
},
560+
}
561+
Expect(k8sClient.Create(ctx, node)).To(Succeed())
562+
defer func() { _ = k8sClient.Delete(ctx, node) }()
563+
564+
// Mark bootstrap completed
565+
readinessController.markBootstrapCompleted(ctx, nodeName, ruleName)
566+
567+
// Verify annotation was added and existing annotation is preserved
568+
Eventually(func(g Gomega) {
569+
updatedNode := &corev1.Node{}
570+
g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: nodeName}, updatedNode)).To(Succeed())
571+
g.Expect(updatedNode.Annotations).To(HaveKeyWithValue(
572+
"readiness.k8s.io/bootstrap-completed-"+ruleName, "true"))
573+
g.Expect(updatedNode.Annotations).To(HaveKeyWithValue(
574+
"existing-annotation", "should-be-preserved"))
575+
}).Should(Succeed())
576+
})
522577
})
523578

524579
Context("when a new rule is created", func() {

0 commit comments

Comments
 (0)