feature(NodeAffinity): return Skip in PreScore when nothing to do in …

…Score
kubernetes · Dec 18, 2023 · 1d04531 · 1d04531
1 parent b46e436
commit 1d04531
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 8 deletions.
diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go
@@ -222,6 +222,10 @@ func (pl *NodeAffinity) PreScore(ctx context.Context, cycleState *framework.Cycl
 	if err != nil {
 		return framework.AsStatus(err)
 	}
+	if preferredNodeAffinity == nil && pl.addedPrefSchedTerms == nil {
+		// NodeAffinity Score has nothing to do with the Pod.
+		return framework.NewStatus(framework.Skip)
+	}
 	state := &preScoreState{
 		preferredNodeAffinity: preferredNodeAffinity,
 	}

diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go
@@ -18,6 +18,7 @@ package nodeaffinity
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -1006,12 +1007,13 @@ func TestNodeAffinityPriority(t *testing.T) {
 	}
 
 	tests := []struct {
-		name         string
-		pod          *v1.Pod
-		nodes        []*v1.Node
-		expectedList framework.NodeScoreList
-		args         config.NodeAffinityArgs
-		runPreScore  bool
+		name               string
+		pod                *v1.Pod
+		nodes              []*v1.Node
+		expectedList       framework.NodeScoreList
+		args               config.NodeAffinityArgs
+		runPreScore        bool
+		wantPreScoreStatus *framework.Status
 	}{
 		{
 			name: "all nodes are same priority as NodeAffinity is nil",
@@ -1026,7 +1028,53 @@ func TestNodeAffinityPriority(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "node3", Labels: label3}},
 			},
 			expectedList: []framework.NodeScore{{Name: "node1", Score: 0}, {Name: "node2", Score: 0}, {Name: "node3", Score: 0}},
-			runPreScore:  true,
+		},
+		{
+			// PreScore returns Skip.
+			name: "Skip is returned in PreScore when NodeAffinity is nil",
+			pod: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			nodes: []*v1.Node{
+				{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: label1}},
+			},
+			runPreScore:        true,
+			wantPreScoreStatus: framework.NewStatus(framework.Skip),
+		},
+		{
+			name: "PreScore returns error when an incoming Pod has a broken affinity",
+			pod: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+				Spec: v1.PodSpec{
+					Affinity: &v1.Affinity{
+						NodeAffinity: &v1.NodeAffinity{
+							PreferredDuringSchedulingIgnoredDuringExecution: []v1.PreferredSchedulingTerm{
+								{
+									Weight: 2,
+									Preference: v1.NodeSelectorTerm{
+										MatchExpressions: []v1.NodeSelectorRequirement{
+											{
+												Key:      "invalid key",
+												Operator: v1.NodeSelectorOpIn,
+												Values:   []string{"bar"},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			nodes: []*v1.Node{
+				{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: label1}},
+			},
+			runPreScore:        true,
+			wantPreScoreStatus: framework.AsStatus(fmt.Errorf(`[0].matchExpressions[0].key: Invalid value: "invalid key": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`)),
 		},
 		{
 			name: "no node matches preferred scheduling requirements in NodeAffinity of pod so all nodes' priority is zero",
@@ -1151,8 +1199,15 @@ func TestNodeAffinityPriority(t *testing.T) {
 			var status *framework.Status
 			if test.runPreScore {
 				status = p.(framework.PreScorePlugin).PreScore(ctx, state, test.pod, tf.BuildNodeInfos(test.nodes))
+				if status.Code() != test.wantPreScoreStatus.Code() {
+					t.Errorf("unexpected status code from PreScore: want: %v got: %v", test.wantPreScoreStatus.Code().String(), status.Code().String())
+				}
+				if status.Message() != test.wantPreScoreStatus.Message() {
+					t.Errorf("unexpected status message from PreScore: want: %v got: %v", test.wantPreScoreStatus.Message(), status.Message())
+				}
 				if !status.IsSuccess() {
-					t.Errorf("unexpected error: %v", status)
+					// no need to proceed.
+					return
 				}
 			}
 			var gotList framework.NodeScoreList