Remove stale failedNodes entries for deleted nodes

internal/controller/helper.go

@@ -21,6 +21,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	readinessv1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1"
 )
 
 // nodeSelectorChanged checks if nodeSelector has changed.
@@ -117,6 +118,29 @@ func taintsEqual(a, b []corev1.Taint) bool {
 	return true
 }
 
+// filters nodeEvaluations and failedNodes to keep only existing nodes.
+func filterStatusForExistingNodes(
+	existingNodes map[string]bool,
+	nodeEvaluations []readinessv1alpha1.NodeEvaluation,
+	failedNodes []readinessv1alpha1.NodeFailure,
+) ([]readinessv1alpha1.NodeEvaluation, []readinessv1alpha1.NodeFailure) {
+	filteredEvaluations := make([]readinessv1alpha1.NodeEvaluation, 0, len(nodeEvaluations))
+	for _, evaluation := range nodeEvaluations {
+		if existingNodes[evaluation.NodeName] {
+			filteredEvaluations = append(filteredEvaluations, evaluation)
+		}
+	}
+
+	filteredFailedNodes := make([]readinessv1alpha1.NodeFailure, 0, len(failedNodes))
+	for _, failure := range failedNodes {
+		if existingNodes[failure.NodeName] {
+			filteredFailedNodes = append(filteredFailedNodes, failure)
+		}
+	}
+
+	return filteredEvaluations, filteredFailedNodes
+}
+
 // labelsEqual checks if two label maps are equal.
 func labelsEqual(a, b map[string]string) bool {
 	if len(a) != len(b) {

internal/controller/nodereadinessrule_controller.go

@@ -215,14 +215,14 @@ func (r *RuleReadinessController) cleanupDeletedNodes(ctx context.Context, rule
 	}
 
 	// Filter out deleted nodes
-	var newNodeEvaluations []readinessv1alpha1.NodeEvaluation
-	for _, evaluation := range rule.Status.NodeEvaluations {
-		if existingNodes[evaluation.NodeName] {
-			newNodeEvaluations = append(newNodeEvaluations, evaluation)
-		}
-	}
-
-	if len(newNodeEvaluations) == len(rule.Status.NodeEvaluations) {
+	newNodeEvaluations, newFailedNodes := filterStatusForExistingNodes(
+		existingNodes,
+		rule.Status.NodeEvaluations,
+		rule.Status.FailedNodes,
+	)
+
+	if len(newNodeEvaluations) == len(rule.Status.NodeEvaluations) &&
+		len(newFailedNodes) == len(rule.Status.FailedNodes) {
 		log.V(4).Info("No deleted nodes to clean up", "rule", rule.Name)
 		return nil
 	}
@@ -239,19 +239,20 @@ func (r *RuleReadinessController) cleanupDeletedNodes(ctx context.Context, rule
 			return err
 		}
 
-		var freshNodeEvaluations []readinessv1alpha1.NodeEvaluation
-		for _, evaluation := range fresh.Status.NodeEvaluations {
-			if existingNodes[evaluation.NodeName] {
-				freshNodeEvaluations = append(freshNodeEvaluations, evaluation)
-			}
-		}
+		freshNodeEvaluations, freshFailedNodes := filterStatusForExistingNodes(
+			existingNodes,
+			fresh.Status.NodeEvaluations,
+			fresh.Status.FailedNodes,
+		)
 
-		if len(freshNodeEvaluations) == len(fresh.Status.NodeEvaluations) {
+		if len(freshNodeEvaluations) == len(fresh.Status.NodeEvaluations) &&
+			len(freshFailedNodes) == len(fresh.Status.FailedNodes) {
 			return nil
 		}
 
 		patch := client.MergeFrom(fresh.DeepCopy())
 		fresh.Status.NodeEvaluations = freshNodeEvaluations
+		fresh.Status.FailedNodes = freshFailedNodes
 		return r.Status().Patch(ctx, fresh, patch)
 	})
 }

internal/controller/nodereadinessrule_controller_test.go

@@ -908,7 +908,18 @@ var _ = Describe("NodeReadinessRule Controller", func() {
 		})
 
 		AfterEach(func() {
-			Expect(k8sClient.Delete(ctx, rule)).To(Succeed())
+			updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+			if err := k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, updatedRule); err == nil {
+				updatedRule.Finalizers = nil
+				_ = k8sClient.Update(ctx, updatedRule)
+				_ = k8sClient.Delete(ctx, updatedRule)
+			}
+
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, &nodereadinessiov1alpha1.NodeReadinessRule{})
+				return apierrors.IsNotFound(err)
+			}, time.Second*10).Should(BeTrue())
+
 			// node1 is already deleted in the test
 			_ = k8sClient.Delete(ctx, node2)
 		})
@@ -955,6 +966,65 @@ var _ = Describe("NodeReadinessRule Controller", func() {
 				return false
 			}, time.Second*5).Should(BeTrue())
 		})
+
+		It("removes failedNodes entries for deleted nodes", func() {
+			_, err := ruleReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "delete-node-rule"}})
+			Expect(err).NotTo(HaveOccurred())
+
+			Eventually(func() int {
+				updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, updatedRule)
+				return len(updatedRule.Status.NodeEvaluations)
+			}, time.Second*5).Should(Equal(2))
+
+			seededRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, seededRule)).To(Succeed())
+			statusPatch := client.MergeFrom(seededRule.DeepCopy())
+			seededRule.Status.FailedNodes = append(seededRule.Status.FailedNodes, nodereadinessiov1alpha1.NodeFailure{
+				NodeName:           "node1",
+				Reason:             "EvaluationError",
+				Message:            "test failure",
+				LastEvaluationTime: metav1.Now(),
+			})
+			Expect(k8sClient.Status().Patch(ctx, seededRule, statusPatch)).To(Succeed())
+
+			Eventually(func() bool {
+				updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, updatedRule)
+				for _, f := range updatedRule.Status.FailedNodes {
+					if f.NodeName == "node1" {
+						return true
+					}
+				}
+				return false
+			}, time.Second*5).Should(BeTrue())
+			verifyRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, verifyRule)).To(Succeed())
+			for _, f := range verifyRule.Status.FailedNodes {
+				Expect(f.NodeName).NotTo(Equal("node2"), "setup should not add a failure for node2")
+			}
+
+			Expect(k8sClient.Delete(ctx, node1)).To(Succeed())
+
+			_, err = ruleReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "delete-node-rule"}})
+			Expect(err).NotTo(HaveOccurred())
+
+			Eventually(func() bool {
+				updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, updatedRule)
+				for _, f := range updatedRule.Status.FailedNodes {
+					if f.NodeName == "node1" {
+						return false
+					}
+				}
+				return true
+			}, time.Second*5).Should(BeTrue())
+			patchedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "delete-node-rule"}, patchedRule)).To(Succeed())
+			for _, f := range patchedRule.Status.FailedNodes {
+				Expect(f.NodeName).NotTo(Equal("node2"), "cleanup should not introduce a failure for node2")
+			}
+		})
 	})
 
 	Context("when a rule's nodeSelector is modified", func() {