Merge pull request #122288 from sanposhiho/revert-qhint-nodeunschedulable

Revert "scheduler/NodeUnschedulable: reduce pod scheduling latency"

pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go

@@ -22,10 +22,8 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	v1helper "k8s.io/component-helpers/scheduling/corev1"
-	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/names"
-	"k8s.io/kubernetes/pkg/scheduler/util"
 )
 
 // NodeUnschedulable plugin filters nodes that set node.Spec.Unschedulable=true unless
@@ -50,34 +48,10 @@ const (
 // failed by this plugin schedulable.
 func (pl *NodeUnschedulable) EventsToRegister() []framework.ClusterEventWithHint {
 	return []framework.ClusterEventWithHint{
-		{Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.UpdateNodeTaint}, QueueingHintFn: pl.isSchedulableAfterNodeChange},
+		{Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.UpdateNodeTaint}},
 	}
 }
 
-// isSchedulableAfterNodeChange is invoked for all node events reported by
-// 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) {
-	originalNode, modifiedNode, err := util.As[*v1.Node](oldObj, newObj)
-	if err != nil {
-		logger.Error(err, "unexpected objects in isSchedulableAfterNodeChange", "oldObj", oldObj, "newObj", newObj)
-		return framework.Queue, err
-	}
-
-	originalNodeSchedulable, modifiedNodeSchedulable := false, !modifiedNode.Spec.Unschedulable
-	if originalNode != nil {
-		originalNodeSchedulable = !originalNode.Spec.Unschedulable
-	}
-
-	if !originalNodeSchedulable && modifiedNodeSchedulable {
-		logger.V(4).Info("node was created or updated, pod may be schedulable now", "pod", klog.KObj(pod), "node", klog.KObj(modifiedNode))
-		return framework.Queue, nil
-	}
-
-	logger.V(4).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
-}
-
 // Name returns name of the plugin. It is used in logs, etc.
 func (pl *NodeUnschedulable) Name() string {
 	return Name

pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go

@@ -85,103 +85,3 @@ func TestNodeUnschedulable(t *testing.T) {
 		}
 	}
 }
-
-func TestIsSchedulableAfterNodeChange(t *testing.T) {
-	testCases := []struct {
-		name           string
-		pod            *v1.Pod
-		oldObj, newObj interface{}
-		expectedHint   framework.QueueingHint
-		expectedErr    bool
-	}{
-		{
-			name:         "backoff-wrong-new-object",
-			pod:          &v1.Pod{},
-			newObj:       "not-a-node",
-			expectedHint: framework.Queue,
-			expectedErr:  true,
-		},
-		{
-			name: "backoff-wrong-old-object",
-			pod:  &v1.Pod{},
-			newObj: &v1.Node{
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-			},
-			oldObj:       "not-a-node",
-			expectedHint: framework.Queue,
-			expectedErr:  true,
-		},
-		{
-			name: "skip-queue-on-unschedulable-node-added",
-			pod:  &v1.Pod{},
-			newObj: &v1.Node{
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-			},
-			expectedHint: framework.QueueSkip,
-		},
-		{
-			name: "queue-on-schedulable-node-added",
-			pod:  &v1.Pod{},
-			newObj: &v1.Node{
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-			},
-			expectedHint: framework.Queue,
-		},
-		{
-			name: "skip-unrelated-change",
-			pod:  &v1.Pod{},
-			newObj: &v1.Node{
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-					Taints: []v1.Taint{
-						{
-							Key:    v1.TaintNodeNotReady,
-							Effect: v1.TaintEffectNoExecute,
-						},
-					},
-				},
-			},
-			oldObj: &v1.Node{
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-			},
-			expectedHint: framework.QueueSkip,
-		},
-		{
-			name: "queue-on-unschedulable-field-change",
-			pod:  &v1.Pod{},
-			newObj: &v1.Node{
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-			},
-			oldObj: &v1.Node{
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-			},
-			expectedHint: framework.Queue,
-		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.name, func(t *testing.T) {
-			logger, _ := ktesting.NewTestContext(t)
-			pl := &NodeUnschedulable{}
-			got, err := pl.isSchedulableAfterNodeChange(logger, testCase.pod, testCase.oldObj, testCase.newObj)
-			if err != nil && !testCase.expectedErr {
-				t.Errorf("unexpected error: %v", err)
-			}
-			if got != testCase.expectedHint {
-				t.Errorf("isSchedulableAfterNodeChange() = %v, want %v", got, testCase.expectedHint)
-			}
-		})
-	}
-}