From da7f085dcb18598bbf7937fc0e051e0d7d844ad9 Mon Sep 17 00:00:00 2001 From: Abdullah Gharaibeh Date: Mon, 14 Mar 2022 15:37:03 -0400 Subject: [PATCH] Added a NodeAffinity PreFilter that looks for node.Name MatchField terms; if exist, the pod is only evaluated against the matching nodes. --- .../plugins/nodeaffinity/node_affinity.go | 48 +++++- .../nodeaffinity/node_affinity_test.go | 145 ++++++++++++++---- 2 files changed, 158 insertions(+), 35 deletions(-) diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go index 62a8daa90c7..ae7b4eab8ea 100644 --- a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go +++ b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go @@ -20,8 +20,10 @@ import ( "context" "fmt" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/component-helpers/scheduling/corev1/nodeaffinity" "k8s.io/kubernetes/pkg/scheduler/apis/config" "k8s.io/kubernetes/pkg/scheduler/apis/config/validation" @@ -58,6 +60,9 @@ const ( // errReasonEnforced is the reason for added node affinity not matching. errReasonEnforced = "node(s) didn't match scheduler-enforced node affinity" + + // errReasonConflict is the reason for pod's conflicting affinity rules. + errReasonConflict = "pod affinity terms conflict" ) // Name returns name of the plugin. It is used in logs, etc. @@ -86,7 +91,48 @@ func (pl *NodeAffinity) EventsToRegister() []framework.ClusterEvent { func (pl *NodeAffinity) PreFilter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod) (*framework.PreFilterResult, *framework.Status) { state := &preFilterState{requiredNodeSelectorAndAffinity: nodeaffinity.GetRequiredNodeAffinity(pod)} cycleState.Write(preFilterStateKey, state) + affinity := pod.Spec.Affinity + if affinity == nil || + affinity.NodeAffinity == nil || + affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil || + len(affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms) == 0 { + return nil, nil + } + + // Check if there is affinity to a specific node and return it. + terms := affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms + var nodeNames sets.String + for _, t := range terms { + var termNodeNames sets.String + for _, r := range t.MatchFields { + if r.Key == metav1.ObjectNameField && r.Operator == v1.NodeSelectorOpIn { + // The requirements represent ANDed constraints, and so we need to + // find the intersection of nodes. + s := sets.NewString(r.Values...) + if termNodeNames == nil { + termNodeNames = s + } else { + termNodeNames = termNodeNames.Intersection(s) + } + } + } + if termNodeNames == nil { + // If this term has no node.Name field affinity, + // then all nodes are eligible because the terms are ORed. + return nil, nil + } + // If the set is empty, it means the terms had affinity to different + // sets of nodes, and since they are ANDed, then the pod will not match any node. + if len(termNodeNames) == 0 { + return nil, framework.NewStatus(framework.UnschedulableAndUnresolvable, errReasonConflict) + } + nodeNames = nodeNames.Union(termNodeNames) + } + if nodeNames != nil { + return &framework.PreFilterResult{NodeNames: nodeNames}, nil + } return nil, nil + } // PreFilterExtensions not necessary for this plugin as state doesn't depend on pod additions or deletions. diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go index 756fcaa548f..ba0da937273 100644 --- a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go +++ b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity_test.go @@ -18,12 +18,12 @@ package nodeaffinity import ( "context" - "reflect" "testing" "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/scheduler/apis/config" "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/runtime" @@ -33,13 +33,15 @@ import ( // TODO: Add test case for RequiredDuringSchedulingRequiredDuringExecution after it's implemented. func TestNodeAffinity(t *testing.T) { tests := []struct { - name string - pod *v1.Pod - labels map[string]string - nodeName string - wantStatus *framework.Status - args config.NodeAffinityArgs - disablePreFilter bool + name string + pod *v1.Pod + labels map[string]string + nodeName string + wantStatus *framework.Status + wantPreFilterStatus *framework.Status + wantPreFilterResult *framework.PreFilterResult + args config.NodeAffinityArgs + disablePreFilter bool }{ { name: "no selector", @@ -513,7 +515,7 @@ func TestNodeAffinity(t *testing.T) { { Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_1"}, + Values: []string{"node1"}, }, }, }, @@ -523,7 +525,8 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_1", + nodeName: "node1", + wantPreFilterResult: &framework.PreFilterResult{NodeNames: sets.NewString("node1")}, }, { name: "Pod with matchFields using In operator that does not match the existing node", @@ -538,7 +541,7 @@ func TestNodeAffinity(t *testing.T) { { Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_1"}, + Values: []string{"node1"}, }, }, }, @@ -548,8 +551,9 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_2", - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod), + nodeName: "node2", + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod), + wantPreFilterResult: &framework.PreFilterResult{NodeNames: sets.NewString("node1")}, }, { name: "Pod with two terms: matchFields does not match, but matchExpressions matches", @@ -564,7 +568,7 @@ func TestNodeAffinity(t *testing.T) { { Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_1"}, + Values: []string{"node1"}, }, }, }, @@ -583,7 +587,7 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_2", + nodeName: "node2", labels: map[string]string{"foo": "bar"}, }, { @@ -599,7 +603,7 @@ func TestNodeAffinity(t *testing.T) { { Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_1"}, + Values: []string{"node1"}, }, }, MatchExpressions: []v1.NodeSelectorRequirement{ @@ -616,9 +620,10 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_2", - labels: map[string]string{"foo": "bar"}, - wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod), + nodeName: "node2", + labels: map[string]string{"foo": "bar"}, + wantPreFilterResult: &framework.PreFilterResult{NodeNames: sets.NewString("node1")}, + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod), }, { name: "Pod with one term: both matchFields and matchExpressions match", @@ -633,7 +638,7 @@ func TestNodeAffinity(t *testing.T) { { Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_1"}, + Values: []string{"node1"}, }, }, MatchExpressions: []v1.NodeSelectorRequirement{ @@ -650,8 +655,9 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_1", - labels: map[string]string{"foo": "bar"}, + nodeName: "node1", + labels: map[string]string{"foo": "bar"}, + wantPreFilterResult: &framework.PreFilterResult{NodeNames: sets.NewString("node1")}, }, { name: "Pod with two terms: both matchFields and matchExpressions do not match", @@ -666,7 +672,7 @@ func TestNodeAffinity(t *testing.T) { { Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_1"}, + Values: []string{"node1"}, }, }, }, @@ -685,10 +691,78 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_2", + nodeName: "node2", labels: map[string]string{"foo": "bar"}, wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod), }, + { + name: "Pod with two terms of node.Name affinity", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: metav1.ObjectNameField, + Operator: v1.NodeSelectorOpIn, + Values: []string{"node1"}, + }, + }, + }, + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: metav1.ObjectNameField, + Operator: v1.NodeSelectorOpIn, + Values: []string{"node2"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + nodeName: "node2", + wantPreFilterResult: &framework.PreFilterResult{NodeNames: sets.NewString("node1", "node2")}, + }, + { + name: "Pod with two conflicting mach field requirements", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Affinity: &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{ + { + Key: metav1.ObjectNameField, + Operator: v1.NodeSelectorOpIn, + Values: []string{"node1"}, + }, + { + Key: metav1.ObjectNameField, + Operator: v1.NodeSelectorOpIn, + Values: []string{"node2"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + nodeName: "node2", + labels: map[string]string{"foo": "bar"}, + wantPreFilterStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, errReasonConflict), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonPod), + }, { name: "Matches added affinity and Pod's node affinity", pod: &v1.Pod{ @@ -712,7 +786,7 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_2", + nodeName: "node2", labels: map[string]string{"zone": "foo"}, args: config.NodeAffinityArgs{ AddedAffinity: &v1.NodeAffinity{ @@ -721,7 +795,7 @@ func TestNodeAffinity(t *testing.T) { MatchFields: []v1.NodeSelectorRequirement{{ Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_2"}, + Values: []string{"node2"}, }}, }}, }, @@ -751,7 +825,7 @@ func TestNodeAffinity(t *testing.T) { }, }, }, - nodeName: "node_2", + nodeName: "node2", labels: map[string]string{"zone": "foo"}, args: config.NodeAffinityArgs{ AddedAffinity: &v1.NodeAffinity{ @@ -760,7 +834,7 @@ func TestNodeAffinity(t *testing.T) { MatchFields: []v1.NodeSelectorRequirement{{ Key: metav1.ObjectNameField, Operator: v1.NodeSelectorOpIn, - Values: []string{"node_2"}, + Values: []string{"node2"}, }}, }}, }, @@ -771,7 +845,7 @@ func TestNodeAffinity(t *testing.T) { { name: "Doesn't match added affinity", pod: &v1.Pod{}, - nodeName: "node_2", + nodeName: "node2", labels: map[string]string{"zone": "foo"}, args: config.NodeAffinityArgs{ AddedAffinity: &v1.NodeAffinity{ @@ -855,14 +929,17 @@ func TestNodeAffinity(t *testing.T) { 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) + gotPreFilterResult, gotStatus := p.(framework.PreFilterPlugin).PreFilter(context.Background(), state, test.pod) + if diff := cmp.Diff(test.wantPreFilterStatus, gotStatus); diff != "" { + t.Errorf("unexpected PreFilter Status (-want,+got):\n%s", diff) + } + if diff := cmp.Diff(test.wantPreFilterResult, gotPreFilterResult); diff != "" { + t.Errorf("unexpected PreFilterResult (-want,+got):\n%s", diff) } } 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) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("unexpected Filter Status (-want,+got):\n%s", diff) } }) }