Add node events for taint operations (TaintAdded, TaintRemoved, TaintAdopted) (#158)

Author: Priyankasaggu11929

config/rbac/role.yaml

@@ -4,6 +4,13 @@ kind: ClusterRole
 metadata:
   name: manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
 - apiGroups:
   - ""
   resources:

internal/controller/node_controller.go

@@ -242,14 +242,21 @@ func (r *RuleReadinessController) hasTaintBySpec(node *corev1.Node, taintSpec co
 }
 
 // addTaintBySpec adds a taint to a node.
-func (r *RuleReadinessController) addTaintBySpec(ctx context.Context, node *corev1.Node, taintSpec corev1.Taint) error {
+func (r *RuleReadinessController) addTaintBySpec(ctx context.Context, node *corev1.Node, taintSpec corev1.Taint, ruleName string) error {
 	patch := client.StrategicMergeFrom(node.DeepCopy())
 	node.Spec.Taints = append(node.Spec.Taints, taintSpec)
-	return r.Patch(ctx, node, patch)
+	if err := r.Patch(ctx, node, patch); err != nil {
+		return err
+	}
+
+	message := fmt.Sprintf("Taint '%s:%s' added by rule '%s'", taintSpec.Key, taintSpec.Effect, ruleName)
+	r.EventRecorder.Event(node, corev1.EventTypeNormal, "TaintAdded", message)
+
+	return nil
 }
 
 // removeTaintBySpec removes a taint from a node.
-func (r *RuleReadinessController) removeTaintBySpec(ctx context.Context, node *corev1.Node, taintSpec corev1.Taint) error {
+func (r *RuleReadinessController) removeTaintBySpec(ctx context.Context, node *corev1.Node, taintSpec corev1.Taint, ruleName string) error {
 	patch := client.StrategicMergeFrom(node.DeepCopy())
 	var newTaints []corev1.Taint
 	for _, taint := range node.Spec.Taints {
@@ -258,7 +265,14 @@ func (r *RuleReadinessController) removeTaintBySpec(ctx context.Context, node *c
 		}
 	}
 	node.Spec.Taints = newTaints
-	return r.Patch(ctx, node, patch)
+	if err := r.Patch(ctx, node, patch); err != nil {
+		return err
+	}
+
+	message := fmt.Sprintf("Taint '%s:%s' removed by rule '%s'", taintSpec.Key, taintSpec.Effect, ruleName)
+	r.EventRecorder.Event(node, corev1.EventTypeNormal, "TaintRemoved", message)
+
+	return nil
 }
 
 // Bootstrap completion tracking.

internal/controller/node_controller_test.go

@@ -27,6 +27,7 @@ import (
 	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"
@@ -115,10 +116,11 @@ var _ = Describe("Node Controller", func() {
 
 			fakeClientset = fake.NewSimpleClientset()
 			readinessController = &RuleReadinessController{
-				Client:    k8sClient,
-				Scheme:    k8sClient.Scheme(),
-				clientset: fakeClientset,
-				ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+				Client:        k8sClient,
+				Scheme:        k8sClient.Scheme(),
+				clientset:     fakeClientset,
+				ruleCache:     make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+				EventRecorder: record.NewFakeRecorder(10),
 			}
 
 			nodeReconciler = &NodeReconciler{
@@ -377,10 +379,11 @@ var _ = Describe("Node Controller", func() {
 
 			fakeClientset = fake.NewSimpleClientset()
 			readinessController = &RuleReadinessController{
-				Client:    k8sClient,
-				Scheme:    k8sClient.Scheme(),
-				clientset: fakeClientset,
-				ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+				Client:        k8sClient,
+				Scheme:        k8sClient.Scheme(),
+				clientset:     fakeClientset,
+				ruleCache:     make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+				EventRecorder: record.NewFakeRecorder(10),
 			}
 
 			nodeReconciler = &NodeReconciler{
@@ -526,10 +529,11 @@ var _ = Describe("Node Controller", func() {
 
 			fakeClientset = fake.NewSimpleClientset()
 			readinessController = &RuleReadinessController{
-				Client:    k8sClient,
-				Scheme:    k8sClient.Scheme(),
-				clientset: fakeClientset,
-				ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+				Client:        k8sClient,
+				Scheme:        k8sClient.Scheme(),
+				clientset:     fakeClientset,
+				ruleCache:     make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+				EventRecorder: record.NewFakeRecorder(10),
 			}
 
 			nodeReconciler = &NodeReconciler{

internal/controller/nodereadinessrule_controller.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -50,8 +51,9 @@ const (
 // RuleReadinessController manages node taints based on readiness rules.
 type RuleReadinessController struct {
 	client.Client
-	Scheme    *runtime.Scheme
-	clientset kubernetes.Interface
+	Scheme        *runtime.Scheme
+	clientset     kubernetes.Interface
+	EventRecorder record.EventRecorder
 
 	// Cache for efficient rule lookup
 	ruleCacheMutex sync.RWMutex
@@ -68,10 +70,11 @@ type RuleReconciler struct {
 // NewRuleReadinessController creates a new controller.
 func NewRuleReadinessController(mgr ctrl.Manager, clientset kubernetes.Interface) *RuleReadinessController {
 	return &RuleReadinessController{
-		Client:    mgr.GetClient(),
-		Scheme:    mgr.GetScheme(),
-		clientset: clientset,
-		ruleCache: make(map[string]*readinessv1alpha1.NodeReadinessRule),
+		Client:        mgr.GetClient(),
+		Scheme:        mgr.GetScheme(),
+		clientset:     clientset,
+		EventRecorder: mgr.GetEventRecorderFor("node-readiness-controller"),
+		ruleCache:     make(map[string]*readinessv1alpha1.NodeReadinessRule),
 	}
 }
 
@@ -87,6 +90,7 @@ func (r *RuleReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager)
 // +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessrules,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessrules/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=readiness.node.x-k8s.io,resources=nodereadinessrules/finalizers,verbs=update
+// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
 
 func (r *RuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
@@ -325,13 +329,15 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule
 	log.Info("Evaluation result", "node", node.Name, "rule", rule.Name,
 		"allConditionsSatisfied", allConditionsSatisfied, "hasTaint", currentlyHasTaint)
 
+	isFirstEvaluation := r.getPreviousNodeEvaluation(rule, node.Name) == nil
+
 	var err error
 
 	switch {
 	case shouldRemoveTaint && currentlyHasTaint:
 		log.Info("Removing taint", "node", node.Name, "rule", rule.Name, "taint", rule.Spec.Taint.Key)
 
-		if err = r.removeTaintBySpec(ctx, node, rule.Spec.Taint); err != nil {
+		if err = r.removeTaintBySpec(ctx, node, rule.Spec.Taint, rule.Name); err != nil {
 			metrics.Failures.WithLabelValues(rule.Name, "RemoveTaintError").Inc()
 			return fmt.Errorf("failed to remove taint: %w", err)
 		}
@@ -345,12 +351,20 @@ func (r *RuleReadinessController) evaluateRuleForNode(ctx context.Context, rule
 	case !shouldRemoveTaint && !currentlyHasTaint:
 		log.Info("Adding taint", "node", node.Name, "rule", rule.Name, "taint", rule.Spec.Taint.Key)
 
-		if err = r.addTaintBySpec(ctx, node, rule.Spec.Taint); err != nil {
+		if err = r.addTaintBySpec(ctx, node, rule.Spec.Taint, rule.Name); err != nil {
 			metrics.Failures.WithLabelValues(rule.Name, "AddTaintError").Inc()
 			return fmt.Errorf("failed to add taint: %w", err)
 		}
 		metrics.TaintOperations.WithLabelValues(rule.Name, "add").Inc()
 
+	case !shouldRemoveTaint && currentlyHasTaint:
+		if isFirstEvaluation {
+			log.Info("Adopting pre-existing taint", "node", node.Name, "rule", rule.Name, "taint", rule.Spec.Taint.Key)
+
+			message := fmt.Sprintf("Taint '%s:%s' is now managed by rule '%s'", rule.Spec.Taint.Key, rule.Spec.Taint.Effect, rule.Name)
+			r.EventRecorder.Event(node, corev1.EventTypeNormal, "TaintAdopted", message)
+		}
+
 	default:
 		log.Info("No taint action needed", "node", node.Name, "rule", rule.Name,
 			"shouldRemove", shouldRemoveTaint, "hasTaint", currentlyHasTaint)
@@ -598,7 +612,7 @@ func (r *RuleReadinessController) cleanupTaintsForRule(ctx context.Context, rule
 				"rule", rule.Name,
 				"taint", rule.Spec.Taint.Key)
 
-			if err := r.removeTaintBySpec(ctx, &node, rule.Spec.Taint); err != nil {
+			if err := r.removeTaintBySpec(ctx, &node, rule.Spec.Taint, rule.Name); err != nil {
 				errors = append(errors, fmt.Sprintf("node %s: %v", node.Name, err))
 			}
 		}
@@ -650,7 +664,7 @@ func (r *RuleReadinessController) cleanupNodesAfterSelectorChange(ctx context.Co
 					"rule", newRule.Name,
 					"taint", newRule.Spec.Taint.Key)
 
-				if err := r.removeTaintBySpec(ctx, &node, newRule.Spec.Taint); err != nil {
+				if err := r.removeTaintBySpec(ctx, &node, newRule.Spec.Taint, newRule.Name); err != nil {
 					errors = append(errors, fmt.Sprintf("node %s: %v", node.Name, err))
 				}
 			}
@@ -681,3 +695,14 @@ func (r *RuleReconciler) ensureFinalizer(ctx context.Context, rule *readinessv1a
 	}
 	return true, nil
 }
+
+// getPreviousNodeEvaluation retrieves the previous evaluation result for a specific node from the rule status.
+// It returns nil (if the node is evaluated for the first time) otherwsie, return the previously evaluated node data.
+func (r *RuleReadinessController) getPreviousNodeEvaluation(rule *readinessv1alpha1.NodeReadinessRule, nodeName string) *readinessv1alpha1.NodeEvaluation {
+	for i := range rule.Status.NodeEvaluations {
+		if rule.Status.NodeEvaluations[i].NodeName == nodeName {
+			return &rule.Status.NodeEvaluations[i]
+		}
+	}
+	return nil
+}

internal/controller/nodereadinessrule_controller_test.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -56,10 +57,11 @@ var _ = Describe("NodeReadinessRule Controller", func() {
 
 		fakeClientset = fake.NewSimpleClientset()
 		readinessController = &RuleReadinessController{
-			Client:    k8sClient,
-			Scheme:    scheme,
-			clientset: fakeClientset,
-			ruleCache: make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+			Client:        k8sClient,
+			Scheme:        scheme,
+			clientset:     fakeClientset,
+			ruleCache:     make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+			EventRecorder: record.NewFakeRecorder(10),
 		}
 
 		ruleReconciler = &RuleReconciler{

test/e2e/e2e_test.go

@@ -525,6 +525,125 @@ status:
 			exec.Command("kubectl", "delete", "node", nodeName).Run()
 			exec.Command("kubectl", "delete", "nodereadinessrule", "dryrun-test-rule").Run()
 		})
+
+		It("should emit events for taint operations", func() {
+			nodeName := "event-test-node"
+
+			By("creating a test node with pre-existing taint")
+			cmd := exec.Command("kubectl", "apply", "-f", "-")
+			cmd.Stdin = strings.NewReader(fmt.Sprintf(`
+apiVersion: v1
+kind: Node
+metadata:
+  name: %s
+  labels:
+    e2e-test: "events"
+spec:
+  taints:
+    - key: readiness.k8s.io/EventTest
+      effect: NoSchedule
+      value: pending
+status:
+  conditions:
+    - type: EventTest
+      status: "False"
+      lastHeartbeatTime: %s
+      lastTransitionTime: %s
+`, nodeName, time.Now().Format(time.RFC3339), time.Now().Format(time.RFC3339)))
+			_, err := utils.Run(cmd)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("applying the rule to adopt pre-existing taint")
+			cmd = exec.Command("kubectl", "apply", "-f", "-")
+			cmd.Stdin = strings.NewReader(`
+apiVersion: readiness.node.x-k8s.io/v1alpha1
+kind: NodeReadinessRule
+metadata:
+  name: event-test-rule
+spec:
+  conditions:
+    - type: EventTest
+      requiredStatus: "True"
+  taint:
+    key: readiness.k8s.io/EventTest
+    effect: NoSchedule
+    value: pending
+  enforcementMode: "continuous"
+  nodeSelector:
+    matchLabels:
+      e2e-test: "events"
+`)
+			_, err = utils.Run(cmd)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("verifying TaintAdopted event is emitted")
+			Eventually(func() bool {
+				cmd := exec.Command("kubectl", "get", "events", "--field-selector",
+					fmt.Sprintf("involvedObject.name=%s", nodeName), "-o", "json")
+				output, err := utils.Run(cmd)
+				if err != nil {
+					return false
+				}
+				return strings.Contains(output, "TaintAdopted") &&
+					strings.Contains(output, "event-test-rule")
+			}, 30*time.Second, 2*time.Second).Should(BeTrue())
+
+			By("updating node condition to True to trigger taint removal")
+			err = patchNodeCondition(nodeName, "EventTest", "True")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("verifying taint is removed")
+			Eventually(func() bool {
+				cmd := exec.Command("kubectl", "get", "node", nodeName, "-o", "jsonpath={.spec.taints}")
+				output, err := utils.Run(cmd)
+				if err != nil {
+					return false
+				}
+				return !strings.Contains(output, "readiness.k8s.io/EventTest")
+			}, 30*time.Second, 2*time.Second).Should(BeTrue())
+
+			By("verifying TaintRemoved event is emitted")
+			Eventually(func() bool {
+				cmd := exec.Command("kubectl", "get", "events", "--field-selector",
+					fmt.Sprintf("involvedObject.name=%s", nodeName), "-o", "json")
+				output, err := utils.Run(cmd)
+				if err != nil {
+					return false
+				}
+				return strings.Contains(output, "TaintRemoved") &&
+					strings.Contains(output, "event-test-rule")
+			}, 30*time.Second, 2*time.Second).Should(BeTrue())
+
+			By("updating node condition back to False to trigger taint re-addition")
+			err = patchNodeCondition(nodeName, "EventTest", "False")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("verifying taint is re-added")
+			Eventually(func() bool {
+				cmd := exec.Command("kubectl", "get", "node", nodeName, "-o", "jsonpath={.spec.taints}")
+				output, err := utils.Run(cmd)
+				if err != nil {
+					return false
+				}
+				return strings.Contains(output, "readiness.k8s.io/EventTest")
+			}, 30*time.Second, 2*time.Second).Should(BeTrue())
+
+			By("verifying TaintAdded event is emitted")
+			Eventually(func() bool {
+				cmd := exec.Command("kubectl", "get", "events", "--field-selector",
+					fmt.Sprintf("involvedObject.name=%s", nodeName), "-o", "json")
+				output, err := utils.Run(cmd)
+				if err != nil {
+					return false
+				}
+				return strings.Contains(output, "TaintAdded") &&
+					strings.Contains(output, "event-test-rule")
+			}, 30*time.Second, 2*time.Second).Should(BeTrue())
+
+			By("cleaning up test resources")
+			exec.Command("kubectl", "delete", "node", nodeName).Run()
+			exec.Command("kubectl", "delete", "nodereadinessrule", "event-test-rule").Run()
+		})
 	})
 })