parse pod's node affinity once in preFilter

pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go

@@ -33,18 +33,20 @@ import (
 // TODO: Add test case for RequiredDuringSchedulingRequiredDuringExecution after it's implemented.
 func TestNodeAffinity(t *testing.T) {
 	tests := []struct {
-		pod        *v1.Pod
-		labels     map[string]string
-		nodeName   string
-		name       string
-		wantStatus *framework.Status
-		args       config.NodeAffinityArgs
+		name             string
+		pod              *v1.Pod
+		labels           map[string]string
+		nodeName         string
+		wantStatus       *framework.Status
+		args             config.NodeAffinityArgs
+		disablePreFilter bool
 	}{
 		{
-			pod:  &v1.Pod{},
 			name: "no selector",
+			pod:  &v1.Pod{},
 		},
 		{
+			name: "missing labels",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					NodeSelector: map[string]string{
@@ -52,10 +54,10 @@ func TestNodeAffinity(t *testing.T) {
 					},
 				},
 			},
-			name:       "missing labels",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "same labels",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					NodeSelector: map[string]string{
@@ -66,9 +68,9 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name: "same labels",
 		},
 		{
+			name: "node labels are superset",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					NodeSelector: map[string]string{
@@ -80,9 +82,9 @@ func TestNodeAffinity(t *testing.T) {
 				"foo": "bar",
 				"baz": "blah",
 			},
-			name: "node labels are superset",
 		},
 		{
+			name: "node labels are subset",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					NodeSelector: map[string]string{
@@ -94,10 +96,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name:       "node labels are subset",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with matchExpressions using In operator that matches the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -122,9 +124,9 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name: "Pod with matchExpressions using In operator that matches the existing node",
 		},
 		{
+			name: "Pod with matchExpressions using Gt operator that matches the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -150,9 +152,9 @@ func TestNodeAffinity(t *testing.T) {
 				// We use two digit to denote major version and two digit for minor version.
 				"kernel-version": "0206",
 			},
-			name: "Pod with matchExpressions using Gt operator that matches the existing node",
 		},
 		{
+			name: "Pod with matchExpressions using NotIn operator that matches the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -177,9 +179,9 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"mem-type": "DDR3",
 			},
-			name: "Pod with matchExpressions using NotIn operator that matches the existing node",
 		},
 		{
+			name: "Pod with matchExpressions using Exists operator that matches the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -203,9 +205,9 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"GPU": "NVIDIA-GRID-K1",
 			},
-			name: "Pod with matchExpressions using Exists operator that matches the existing node",
 		},
 		{
+			name: "Pod with affinity that don't match node's labels won't schedule onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -230,10 +232,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name:       "Pod with affinity that don't match node's labels won't schedule onto the node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with a nil []NodeSelectorTerm in affinity, can't match the node's labels and won't schedule onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -248,10 +250,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name:       "Pod with a nil []NodeSelectorTerm in affinity, can't match the node's labels and won't schedule onto the node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with an empty []NodeSelectorTerm in affinity, can't match the node's labels and won't schedule onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -266,10 +268,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name:       "Pod with an empty []NodeSelectorTerm in affinity, can't match the node's labels and won't schedule onto the node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with empty MatchExpressions is not a valid value will match no objects and won't schedule onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -288,17 +290,17 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name:       "Pod with empty MatchExpressions is not a valid value will match no objects and won't schedule onto the node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
-			pod: &v1.Pod{},
+			name: "Pod with no Affinity will schedule onto a node",
+			pod:  &v1.Pod{},
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name: "Pod with no Affinity will schedule onto a node",
 		},
 		{
+			name: "Pod with Affinity but nil NodeSelector will schedule onto a node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -311,9 +313,9 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name: "Pod with Affinity but nil NodeSelector will schedule onto a node",
 		},
 		{
+			name: "Pod with multiple matchExpressions ANDed that matches the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -341,9 +343,9 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"GPU": "NVIDIA-GRID-K1",
 			},
-			name: "Pod with multiple matchExpressions ANDed that matches the existing node",
 		},
 		{
+			name: "Pod with multiple matchExpressions ANDed that doesn't match the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -371,10 +373,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"GPU": "NVIDIA-GRID-K1",
 			},
-			name:       "Pod with multiple matchExpressions ANDed that doesn't match the existing node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with multiple NodeSelectorTerms ORed in affinity, matches the node's labels and will schedule onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -408,9 +410,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name: "Pod with multiple NodeSelectorTerms ORed in affinity, matches the node's labels and will schedule onto the node",
 		},
 		{
+			name: "Pod with an Affinity and a PodSpec.NodeSelector(the old thing that we are deprecating) " +
+				"both are satisfied, will schedule onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					NodeSelector: map[string]string{
@@ -437,10 +440,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name: "Pod with an Affinity and a PodSpec.NodeSelector(the old thing that we are deprecating) " +
-				"both are satisfied, will schedule onto the node",
 		},
 		{
+			name: "Pod with an Affinity matches node's labels but the PodSpec.NodeSelector(the old thing that we are deprecating) " +
+				"is not satisfied, won't schedule onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					NodeSelector: map[string]string{
@@ -467,11 +470,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "barrrrrr",
 			},
-			name: "Pod with an Affinity matches node's labels but the PodSpec.NodeSelector(the old thing that we are deprecating) " +
-				"is not satisfied, won't schedule onto the node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with an invalid value in Affinity term won't be scheduled onto the node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -496,10 +498,10 @@ func TestNodeAffinity(t *testing.T) {
 			labels: map[string]string{
 				"foo": "bar",
 			},
-			name:       "Pod with an invalid value in Affinity term won't be scheduled onto the node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with matchFields using In operator that matches the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -522,9 +524,9 @@ func TestNodeAffinity(t *testing.T) {
 				},
 			},
 			nodeName: "node_1",
-			name:     "Pod with matchFields using In operator that matches the existing node",
 		},
 		{
+			name: "Pod with matchFields using In operator that does not match the existing node",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -547,10 +549,10 @@ func TestNodeAffinity(t *testing.T) {
 				},
 			},
 			nodeName:   "node_2",
-			name:       "Pod with matchFields using In operator that does not match the existing node",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with two terms: matchFields does not match, but matchExpressions matches",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -583,9 +585,9 @@ func TestNodeAffinity(t *testing.T) {
 			},
 			nodeName: "node_2",
 			labels:   map[string]string{"foo": "bar"},
-			name:     "Pod with two terms: matchFields does not match, but matchExpressions matches",
 		},
 		{
+			name: "Pod with one term: matchFields does not match, but matchExpressions matches",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -616,10 +618,10 @@ func TestNodeAffinity(t *testing.T) {
 			},
 			nodeName:   "node_2",
 			labels:     map[string]string{"foo": "bar"},
-			name:       "Pod with one term: matchFields does not match, but matchExpressions matches",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Pod with one term: both matchFields and matchExpressions match",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -650,9 +652,9 @@ func TestNodeAffinity(t *testing.T) {
 			},
 			nodeName: "node_1",
 			labels:   map[string]string{"foo": "bar"},
-			name:     "Pod with one term: both matchFields and matchExpressions match",
 		},
 		{
+			name: "Pod with two terms: both matchFields and matchExpressions do not match",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -685,10 +687,10 @@ func TestNodeAffinity(t *testing.T) {
 			},
 			nodeName:   "node_2",
 			labels:     map[string]string{"foo": "bar"},
-			name:       "Pod with two terms: both matchFields and matchExpressions do not match",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name: "Matches added affinity and Pod's node affinity",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -725,9 +727,9 @@ func TestNodeAffinity(t *testing.T) {
 					},
 				},
 			},
-			name: "Matches added affinity and Pod's node affinity",
 		},
 		{
+			name: "Matches added affinity but not Pod's node affinity",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: &v1.Affinity{
@@ -764,10 +766,10 @@ func TestNodeAffinity(t *testing.T) {
 					},
 				},
 			},
-			name:       "Matches added affinity but not Pod's node affinity",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod),
 		},
 		{
+			name:     "Doesn't match added affinity",
 			pod:      &v1.Pod{},
 			nodeName: "node_2",
 			labels:   map[string]string{"zone": "foo"},
@@ -786,9 +788,54 @@ func TestNodeAffinity(t *testing.T) {
 					},
 				},
 			},
-			name:       "Doesn't match added affinity",
 			wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, errReasonEnforced),
 		},
+		{
+			name: "Matches node selector correctly even if PreFilter is not called",
+			pod: &v1.Pod{
+				Spec: v1.PodSpec{
+					NodeSelector: map[string]string{
+						"foo": "bar",
+					},
+				},
+			},
+			labels: map[string]string{
+				"foo": "bar",
+				"baz": "blah",
+			},
+			disablePreFilter: true,
+		},
+		{
+			name: "Matches node affinity correctly even if PreFilter is not called",
+			pod: &v1.Pod{
+				Spec: v1.PodSpec{
+					Affinity: &v1.Affinity{
+						NodeAffinity: &v1.NodeAffinity{
+							RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
+								NodeSelectorTerms: []v1.NodeSelectorTerm{
+									{
+										MatchExpressions: []v1.NodeSelectorRequirement{
+											{
+												Key:      "GPU",
+												Operator: v1.NodeSelectorOpExists,
+											}, {
+												Key:      "GPU",
+												Operator: v1.NodeSelectorOpNotIn,
+												Values:   []string{"AMD", "INTER"},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			labels: map[string]string{
+				"GPU": "NVIDIA-GRID-K1",
+			},
+			disablePreFilter: true,
+		},
 	}
 
 	for _, test := range tests {
@@ -804,7 +851,16 @@ func TestNodeAffinity(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Creating plugin: %v", err)
 			}
-			gotStatus := p.(framework.FilterPlugin).Filter(context.Background(), nil, test.pod, nodeInfo)
+
+			state := framework.NewCycleState()
+			var gotStatus *framework.Status
+			if !test.disablePreFilter {
+				gotStatus = p.(framework.PreFilterPlugin).PreFilter(context.Background(), state, test.pod)
+				if !gotStatus.IsSuccess() {
+					t.Errorf("unexpected error: %v", gotStatus)
+				}
+			}
+			gotStatus = p.(framework.FilterPlugin).Filter(context.Background(), state, test.pod, nodeInfo)
 			if !reflect.DeepEqual(gotStatus, test.wantStatus) {
 				t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus)
 			}
@@ -888,14 +944,15 @@ func TestNodeAffinityPriority(t *testing.T) {
 	}
 
 	tests := []struct {
+		name            string
 		pod             *v1.Pod
 		nodes           []*v1.Node
 		expectedList    framework.NodeScoreList
-		name            string
 		args            config.NodeAffinityArgs
 		disablePreScore bool
 	}{
 		{
+			name: "all machines are same priority as NodeAffinity is nil",
 			pod: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{},
@@ -907,9 +964,9 @@ func TestNodeAffinityPriority(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine3", Labels: label3}},
 			},
 			expectedList: []framework.NodeScore{{Name: "machine1", Score: 0}, {Name: "machine2", Score: 0}, {Name: "machine3", Score: 0}},
-			name:         "all machines are same priority as NodeAffinity is nil",
 		},
 		{
+			name: "no machine macthes preferred scheduling requirements in NodeAffinity of pod so all machines' priority is zero",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: affinity1,
@@ -921,9 +978,9 @@ func TestNodeAffinityPriority(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine3", Labels: label3}},
 			},
 			expectedList: []framework.NodeScore{{Name: "machine1", Score: 0}, {Name: "machine2", Score: 0}, {Name: "machine3", Score: 0}},
-			name:         "no machine macthes preferred scheduling requirements in NodeAffinity of pod so all machines' priority is zero",
 		},
 		{
+			name: "only machine1 matches the preferred scheduling requirements of pod",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: affinity1,
@@ -935,9 +992,9 @@ func TestNodeAffinityPriority(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine3", Labels: label3}},
 			},
 			expectedList: []framework.NodeScore{{Name: "machine1", Score: framework.MaxNodeScore}, {Name: "machine2", Score: 0}, {Name: "machine3", Score: 0}},
-			name:         "only machine1 matches the preferred scheduling requirements of pod",
 		},
 		{
+			name: "all machines matches the preferred scheduling requirements of pod but with different priorities ",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: affinity2,
@@ -949,21 +1006,21 @@ func TestNodeAffinityPriority(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine2", Labels: label2}},
 			},
 			expectedList: []framework.NodeScore{{Name: "machine1", Score: 18}, {Name: "machine5", Score: framework.MaxNodeScore}, {Name: "machine2", Score: 36}},
-			name:         "all machines matches the preferred scheduling requirements of pod but with different priorities ",
 		},
 		{
-			pod: &v1.Pod{},
+			name: "added affinity",
+			pod:  &v1.Pod{},
 			nodes: []*v1.Node{
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine1", Labels: label1}},
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine2", Labels: label2}},
 			},
 			expectedList: []framework.NodeScore{{Name: "machine1", Score: framework.MaxNodeScore}, {Name: "machine2", Score: 0}},
-			name:         "added affinity",
 			args: config.NodeAffinityArgs{
 				AddedAffinity: affinity1.NodeAffinity,
 			},
 		},
 		{
+			name: "added affinity and pod has default affinity",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: affinity1,
@@ -975,7 +1032,6 @@ func TestNodeAffinityPriority(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine3", Labels: label5}},
 			},
 			expectedList: []framework.NodeScore{{Name: "machine1", Score: 40}, {Name: "machine2", Score: 60}, {Name: "machine3", Score: framework.MaxNodeScore}},
-			name:         "added affinity and pod has default affinity",
 			args: config.NodeAffinityArgs{
 				AddedAffinity: &v1.NodeAffinity{
 					PreferredDuringSchedulingIgnoredDuringExecution: []v1.PreferredSchedulingTerm{
@@ -996,6 +1052,7 @@ func TestNodeAffinityPriority(t *testing.T) {
 			},
 		},
 		{
+			name: "calculate the priorities correctly even if PreScore is not called",
 			pod: &v1.Pod{
 				Spec: v1.PodSpec{
 					Affinity: affinity2,
@@ -1007,7 +1064,6 @@ func TestNodeAffinityPriority(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "machine2", Labels: label2}},
 			},
 			expectedList:    []framework.NodeScore{{Name: "machine1", Score: 18}, {Name: "machine5", Score: framework.MaxNodeScore}, {Name: "machine2", Score: 36}},
-			name:            "calculate the priorities correctly even if PreScore is not called",
 			disablePreScore: true,
 		},
 	}