From 86aee8cbc7e965787fce00c7038ae12d18a0f54f Mon Sep 17 00:00:00 2001
From: googs1025 <googs1025@gmail.com>
Date: Wed, 18 Sep 2024 23:18:56 +0800
Subject: [PATCH] feature(scheduler): more fine-grained QHints for
 nodeunschedulable plugin

---
 .../nodeunschedulable/node_unschedulable.go   | 11 +++++----
 .../node_unschedulable_test.go                | 23 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go
index 7cc7988061b..865809985ab 100644
--- a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go
+++ b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go
@@ -61,19 +61,20 @@ func (pl *NodeUnschedulable) EventsToRegister(_ context.Context) ([]framework.Cl
 // an informer. It checks whether that change made a previously unschedulable
 // pod schedulable.
 func (pl *NodeUnschedulable) isSchedulableAfterNodeChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
-	_, modifiedNode, err := util.As[*v1.Node](oldObj, newObj)
+	originalNode, modifiedNode, err := util.As[*v1.Node](oldObj, newObj)
 	if err != nil {
 		return framework.Queue, err
 	}
 
-	if !modifiedNode.Spec.Unschedulable {
+	// We queue this Pod when -
+	// 1. the node is updated from unschedulable to schedulable.
+	// 2. the node is added and is schedulable.
+	if (originalNode != nil && originalNode.Spec.Unschedulable && !modifiedNode.Spec.Unschedulable) ||
+		(originalNode == nil && !modifiedNode.Spec.Unschedulable) {
 		logger.V(5).Info("node was created or updated, pod may be schedulable now", "pod", klog.KObj(pod), "node", klog.KObj(modifiedNode))
 		return framework.Queue, nil
 	}
 
-	// TODO: also check if the original node meets the pod's requestments once preCheck is completely removed.
-	// See: https://github.com/kubernetes/kubernetes/issues/110175
-
 	logger.V(5).Info("node was created or updated, but it doesn't make this pod schedulable", "pod", klog.KObj(pod), "node", klog.KObj(modifiedNode))
 	return framework.QueueSkip, nil
 }
diff --git a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go
index d9659d6c66c..5a13ed2be91 100644
--- a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go
+++ b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go
@@ -135,7 +135,7 @@ func TestIsSchedulableAfterNodeChange(t *testing.T) {
 			expectedHint: framework.Queue,
 		},
 		{
-			name: "skip-unrelated-change",
+			name: "skip-unrelated-change-unschedulable-true",
 			pod:  &v1.Pod{},
 			newObj: &v1.Node{
 				Spec: v1.NodeSpec{
@@ -155,6 +155,27 @@ func TestIsSchedulableAfterNodeChange(t *testing.T) {
 			},
 			expectedHint: framework.QueueSkip,
 		},
+		{
+			name: "skip-unrelated-change-unschedulable-false",
+			pod:  &v1.Pod{},
+			newObj: &v1.Node{
+				Spec: v1.NodeSpec{
+					Unschedulable: false,
+					Taints: []v1.Taint{
+						{
+							Key:    v1.TaintNodeNotReady,
+							Effect: v1.TaintEffectNoExecute,
+						},
+					},
+				},
+			},
+			oldObj: &v1.Node{
+				Spec: v1.NodeSpec{
+					Unschedulable: false,
+				},
+			},
+			expectedHint: framework.QueueSkip,
+		},
 		{
 			name: "queue-on-unschedulable-field-change",
 			pod:  &v1.Pod{},