nodeaffinity: scheduler queueing hints

kubernetes · Dec 14, 2023 · 5dbb1f5 · 5dbb1f5
1 parent 98748cc
commit 5dbb1f5
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 23 deletions.
diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go
@@ -92,7 +92,7 @@ func (pl *NodeAffinity) EventsToRegister() []framework.ClusterEventWithHint {
 // isSchedulableAfterNodeChange is invoked whenever a node changed. It checks whether
 // that change made a previously unschedulable pod schedulable.
 func (pl *NodeAffinity) isSchedulableAfterNodeChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) {
-	originalNode, modifiedNode, err := util.As[*v1.Node](oldObj, newObj)
+	_, modifiedNode, err := util.As[*v1.Node](oldObj, newObj)
 	if err != nil {
 		return framework.Queue, err
 	}
@@ -107,25 +107,14 @@ func (pl *NodeAffinity) isSchedulableAfterNodeChange(logger klog.Logger, pod *v1
 	if err != nil {
 		return framework.Queue, err
 	}
-	if !isMatched {
-		logger.V(4).Info("node was created or updated, but doesn't matches with the pod's NodeAffinity", "pod", klog.KObj(pod), "node", klog.KObj(modifiedNode))
-		return framework.QueueSkip, nil
-	}
-
-	wasMatched := false
-	if originalNode != nil {
-		wasMatched, err = requiredNodeAffinity.Match(originalNode)
-		if err != nil {
-			return framework.Queue, err
-		}
-	}
-
-	if !wasMatched {
-		// This modification makes this Node match with Pod's NodeAffinity.
+	if isMatched {
 		logger.V(4).Info("node was created or updated, and matches with the pod's NodeAffinity", "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(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
 }

diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go
@@ -1226,13 +1226,15 @@ func Test_isSchedulableAfterNodeChange(t *testing.T) {
 			newObj:       st.MakeNode().Label("k", "v").Obj(),
 			expectedHint: framework.QueueSkip,
 		},
-		"skip-labels-changes-on-suitable-node": {
-			args:         &config.NodeAffinityArgs{},
-			pod:          podWithNodeAffinity.DeepCopy(),
-			oldObj:       st.MakeNode().Label("foo", "bar").Obj(),
-			newObj:       st.MakeNode().Label("foo", "bar").Label("k", "v").Obj(),
-			expectedHint: framework.QueueSkip,
-		},
+		// uncomment this case when the isSchedulableAfterNodeChange also check the
+		// original node's labels.
+		// "skip-labels-changes-on-suitable-node": {
+		// 	args:         &config.NodeAffinityArgs{},
+		// 	pod:          podWithNodeAffinity.DeepCopy(),
+		// 	oldObj:       st.MakeNode().Label("foo", "bar").Obj(),
+		// 	newObj:       st.MakeNode().Label("foo", "bar").Label("k", "v").Obj(),
+		// 	expectedHint: framework.QueueSkip,
+		// },
 		"skip-labels-changes-on-node-from-suitable-to-unsuitable": {
 			args:         &config.NodeAffinityArgs{},
 			pod:          podWithNodeAffinity.DeepCopy(),