[BUG] RuleReconciler's updateRuleStatus can silently discard concurrent NodeReconciler status writes #341

package controller

import (
	"context"
	"testing"

	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/apimachinery/pkg/types"
	"k8s.io/client-go/kubernetes/fake"
	"k8s.io/client-go/tools/events"
	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"

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

func TestIssue341LostUpdateRepro(t *testing.T) {
	ctx := context.Background()
	scheme := runtime.NewScheme()
	_ = corev1.AddToScheme(scheme)
	_ = readinessv1alpha1.AddToScheme(scheme)

	nodeA := &corev1.Node{
		ObjectMeta: metav1.ObjectMeta{Name: "node-a", Labels: map[string]string{"role": "worker"}},
		Status:     corev1.NodeStatus{Conditions: []corev1.NodeCondition{{Type: "Ready", Status: corev1.ConditionTrue}}},
	}
	nodeB := &corev1.Node{
		ObjectMeta: metav1.ObjectMeta{Name: "node-b", Labels: map[string]string{"role": "worker"}},
		Status:     corev1.NodeStatus{Conditions: []corev1.NodeCondition{{Type: "Ready", Status: corev1.ConditionTrue}}},
	}
	rule := &readinessv1alpha1.NodeReadinessRule{
		ObjectMeta: metav1.ObjectMeta{Name: "repro-rule"},
		Spec: readinessv1alpha1.NodeReadinessRuleSpec{
			Conditions:      []readinessv1alpha1.ConditionRequirement{{Type: "Ready", RequiredStatus: corev1.ConditionTrue}},
			Taint:           corev1.Taint{Key: "readiness.k8s.io/repro", Effect: corev1.TaintEffectNoSchedule},
			NodeSelector:    metav1.LabelSelector{MatchLabels: map[string]string{"role": "worker"}},
			EnforcementMode: readinessv1alpha1.EnforcementModeContinuous,
		},
	}

	fc := fakeclient.NewClientBuilder().WithScheme(scheme).WithObjects(nodeA, nodeB, rule).WithStatusSubresource(rule).Build()
	c := &RuleReadinessController{
		Client: fc, Scheme: scheme, clientset: fake.NewSimpleClientset(),
		ruleCache: map[string]*readinessv1alpha1.NodeReadinessRule{rule.Name: rule}, EventRecorder: events.NewFakeRecorder(10),
	}

	// RuleReconciler starts a sweep from a nodeList snapshot that only knows about node-a.
	if err := c.processAllNodesForRule(ctx, rule, &corev1.NodeList{Items: []corev1.Node{*nodeA}}); err != nil {
		t.Fatalf("processAllNodesForRule: %v", err)
	}
	// While that sweep is "in flight", NodeReconciler independently reconciles node-b.
	if err := c.processNodeAgainstAllRules(ctx, nodeB); err != nil {
		t.Fatalf("processNodeAgainstAllRules: %v", err)
	}
	// RuleReconciler now finishes its (stale) sweep and patches the rule status.
	if err := c.updateRuleStatus(ctx, rule); err != nil {
		t.Fatalf("updateRuleStatus: %v", err)
	}

	final := &readinessv1alpha1.NodeReadinessRule{}
	_ = fc.Get(ctx, types.NamespacedName{Name: rule.Name}, final)
	for _, e := range final.Status.NodeEvaluations {
		if e.NodeName == "node-b" {
			return // node-b survived, bug not present
		}
	}
	t.Fatalf("node-b's evaluation was silently discarded. Final NodeEvaluations: %+v", final.Status.NodeEvaluations)
}