Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions internal/controller/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,48 @@ limitations under the License.
package controller

import (
"encoding/json"

corev1 "k8s.io/api/core/v1"
)

const (
// bootstrapStateAnnotation is the node annotation key that stores a JSON map
// of rule names to their bootstrap completion status.
// Using a single annotation with a structured payload avoids Kubernetes'
// 63-character name-segment limit on annotation keys and keeps all rule
// state visible in one place — the same pattern kubectl uses for
// kubectl.kubernetes.io/last-applied-configuration.
bootstrapStateAnnotation = "readiness.k8s.io/bootstrap-state"
)

// getBootstrapState deserializes the bootstrap state map from the node annotation.
// It returns an empty map if the annotation is absent or unparseable.
func getBootstrapState(node *corev1.Node) map[string]bool {
state := make(map[string]bool)
if node.Annotations == nil {
return state
}
raw, ok := node.Annotations[bootstrapStateAnnotation]
if !ok || raw == "" {
return state
}
// Best-effort parse; ignore malformed values.
_ = json.Unmarshal([]byte(raw), &state)
return state
}

// serializeBootstrapState serializes the bootstrap state map to a JSON string
// for storage in the node annotation. Errors are silently ignored and return "{}".
func serializeBootstrapState(state map[string]bool) string {
b, err := json.Marshal(state)
if err != nil {
return "{}"
}
return string(b)
}


// conditionsEqual checks if two condition slices are equal.
func conditionsEqual(a, b []corev1.NodeCondition) bool {
if len(a) != len(b) {
Expand Down
113 changes: 113 additions & 0 deletions internal/controller/helper_unit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
Copyright The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package controller

import (
"testing"

. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestGetBootstrapState(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
annotations map[string]string
expected map[string]bool
}{
{
name: "nil annotations returns empty map",
annotations: nil,
expected: map[string]bool{},
},
{
name: "missing annotation key returns empty map",
annotations: map[string]string{"other-key": "other-value"},
expected: map[string]bool{},
},
{
name: "empty annotation value returns empty map",
annotations: map[string]string{bootstrapStateAnnotation: ""},
expected: map[string]bool{},
},
{
name: "malformed JSON returns empty map",
annotations: map[string]string{bootstrapStateAnnotation: "not-json"},
expected: map[string]bool{},
},
{
name: "valid JSON with one rule",
annotations: map[string]string{bootstrapStateAnnotation: `{"my-rule":true}`},
expected: map[string]bool{"my-rule": true},
},
{
name: "valid JSON with multiple rules",
annotations: map[string]string{bootstrapStateAnnotation: `{"rule-a":true,"rule-b":true}`},
expected: map[string]bool{"rule-a": true, "rule-b": true},
},
{
name: "very long rule name stored as map key — no length constraint",
annotations: map[string]string{
bootstrapStateAnnotation: `{"my-very-long-rule-name-that-is-definitely-going-to-exceed-the-63-character-annotation-key-limit":true}`,
},
expected: map[string]bool{
"my-very-long-rule-name-that-is-definitely-going-to-exceed-the-63-character-annotation-key-limit": true,
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
node := &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Annotations: tt.annotations,
},
}
result := getBootstrapState(node)
g.Expect(result).To(Equal(tt.expected))
})
}
}

func TestSerializeBootstrapState(t *testing.T) {
g := NewWithT(t)

t.Run("empty map serializes to {}", func(t *testing.T) {
result := serializeBootstrapState(map[string]bool{})
g.Expect(result).To(Equal("{}"))
})

t.Run("round-trip: serialize then deserialize preserves entries", func(t *testing.T) {
original := map[string]bool{
"rule-a": true,
"my-very-long-rule-name-that-exceeds-any-annotation-key-limit-by-far": true,
}
serialized := serializeBootstrapState(original)

node := &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{bootstrapStateAnnotation: serialized},
},
}
recovered := getBootstrapState(node)
g.Expect(recovered).To(Equal(original))
})
}

25 changes: 10 additions & 15 deletions internal/controller/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,23 +324,16 @@ func (r *RuleReadinessController) removeTaintBySpec(ctx context.Context, node *c
})
}

// Bootstrap completion tracking.
func (r *RuleReadinessController) isBootstrapCompleted(ctx context.Context, nodeName, ruleName string) bool {
// Check node annotation
node := &corev1.Node{}
if err := r.Get(ctx, client.ObjectKey{Name: nodeName}, node); err != nil {
return false
}

annotationKey := fmt.Sprintf("readiness.k8s.io/bootstrap-completed-%s", ruleName)
_, exists := node.Annotations[annotationKey]
return exists
return getBootstrapState(node)[ruleName]
}

func (r *RuleReadinessController) markBootstrapCompleted(ctx context.Context, nodeName, ruleName string) {
log := ctrl.LoggerFrom(ctx)

annotationKey := fmt.Sprintf("readiness.k8s.io/bootstrap-completed-%s", ruleName)
marked := false

// retry to handle conflict with concurrent node updates
Expand All @@ -350,21 +343,23 @@ func (r *RuleReadinessController) markBootstrapCompleted(ctx context.Context, no
return err
}

// Check if already marked to avoid unnecessary updates
if node.Annotations != nil {
if _, exists := node.Annotations[annotationKey]; exists {
return nil
}
// Decode the existing bootstrap state map.
state := getBootstrapState(node)

// Check if already marked to avoid unnecessary updates.
if state[ruleName] {
return nil
}

patch := client.MergeFrom(node.DeepCopy())

// Initialize annotations if nil
// Initialize annotations map if nil.
if node.Annotations == nil {
node.Annotations = make(map[string]string)
}

node.Annotations[annotationKey] = "true"
state[ruleName] = true
node.Annotations[bootstrapStateAnnotation] = serializeBootstrapState(state)
if err := r.Patch(ctx, node, patch); err != nil {
return err
}
Expand Down
132 changes: 132 additions & 0 deletions internal/controller/node_controller_reproduction_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
Copyright The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package controller

import (
"context"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

nodereadinessiov1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1"
)

var _ = Describe("Node Controller Reproduction", func() {
Context("when reconciling a node with a very long rule name", func() {
var (
ctx context.Context
readinessController *RuleReadinessController
nodeReconciler *NodeReconciler
fakeClientset *fake.Clientset
node *corev1.Node
rule *nodereadinessiov1alpha1.NodeReadinessRule
longRuleName = "my-very-long-rule-name-that-exceeds-the-annotation-key-limit-strictly"
)

BeforeEach(func() {
ctx = context.Background()

fakeClientset = fake.NewSimpleClientset()
readinessController = &RuleReadinessController{
Client: k8sClient,
Scheme: k8sClient.Scheme(),
clientset: fakeClientset,
ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
EventRecorder: record.NewFakeRecorder(10),
}

nodeReconciler = &NodeReconciler{
Client: k8sClient,
Scheme: k8sClient.Scheme(),
Controller: readinessController,
}

node = &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "repro-node",
Labels: map[string]string{"env": "repro"},
},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{
{Type: "ReproCondition", Status: corev1.ConditionTrue},
},
},
}

rule = &nodereadinessiov1alpha1.NodeReadinessRule{
ObjectMeta: metav1.ObjectMeta{
Name: longRuleName,
},
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
{Type: "ReproCondition", RequiredStatus: corev1.ConditionTrue},
},
Taint: corev1.Taint{
Key: "readiness.k8s.io/repro-taint",
Effect: corev1.TaintEffectNoSchedule,
},
NodeSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"env": "repro"},
},
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
},
}
})

JustBeforeEach(func() {
Expect(k8sClient.Create(ctx, node)).To(Succeed())
Expect(k8sClient.Create(ctx, rule)).To(Succeed())
readinessController.updateRuleCache(ctx, rule)
})

AfterEach(func() {
_ = k8sClient.Delete(ctx, node)
updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
if err := k8sClient.Get(ctx, types.NamespacedName{Name: longRuleName}, updatedRule); err == nil {
updatedRule.Finalizers = nil
_ = k8sClient.Update(ctx, updatedRule)
_ = k8sClient.Delete(ctx, updatedRule)
}
readinessController.removeRuleFromCache(ctx, longRuleName)
})

It("should successfully mark bootstrap completed using the JSON payload annotation for long rule names", func() {
// Trigger reconciliation
_, err := nodeReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "repro-node"}})
Expect(err).NotTo(HaveOccurred())

recheckedNode := &corev1.Node{}
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "repro-node"}, recheckedNode)).To(Succeed())

// The bootstrap state should be stored as a JSON map in a single annotation,
// with the full rule name as the key — no length constraint applies.
Expect(recheckedNode.Annotations).To(HaveKey(bootstrapStateAnnotation),
"bootstrap-state annotation should be present on the node")

state := getBootstrapState(recheckedNode)
Expect(state).To(HaveKeyWithValue(longRuleName, true),
"bootstrap state JSON should contain the full rule name marked as completed")
})
})
})