From 3b20a007ed75580f6bb10d1ac6bcec1a3fb698bd Mon Sep 17 00:00:00 2001 From: Kensei Nakada Date: Sat, 7 Jan 2023 07:35:23 +0000 Subject: [PATCH] feature(InterPodAffinity): return Skip in PreFilter --- .../plugins/interpodaffinity/filtering.go | 6 +- .../interpodaffinity/filtering_test.go | 171 ++++++++---------- 2 files changed, 85 insertions(+), 92 deletions(-) diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/filtering.go b/pkg/scheduler/framework/plugins/interpodaffinity/filtering.go index 35d75d258b4..02a7c792693 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/filtering.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/filtering.go @@ -151,7 +151,7 @@ func podMatchesAllAffinityTerms(terms []framework.AffinityTerm, pod *v1.Pod) boo // calculates the following for each existing pod on each node: // 1. Whether it has PodAntiAffinity -// 2. Whether any AffinityTerm matches the incoming pod +// 2. Whether any AntiAffinityTerm matches the incoming pod func (pl *InterPodAffinity) getExistingAntiAffinityCounts(ctx context.Context, pod *v1.Pod, nsLabels labels.Set, nodes []*framework.NodeInfo) topologyToMatchedTermCount { topoMaps := make([]topologyToMatchedTermCount, len(nodes)) index := int32(-1) @@ -259,6 +259,10 @@ func (pl *InterPodAffinity) PreFilter(ctx context.Context, cycleState *framework s.existingAntiAffinityCounts = pl.getExistingAntiAffinityCounts(ctx, pod, s.namespaceLabels, nodesWithRequiredAntiAffinityPods) s.affinityCounts, s.antiAffinityCounts = pl.getIncomingAffinityAntiAffinityCounts(ctx, s.podInfo, allNodes) + if len(s.existingAntiAffinityCounts) == 0 && len(s.podInfo.RequiredAffinityTerms) == 0 && len(s.podInfo.RequiredAntiAffinityTerms) == 0 { + return nil, framework.NewStatus(framework.Skip) + } + cycleState.Write(preFilterStateKey, s) return nil, nil } diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go b/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go index d2d62ee64b0..5aeac5e9765 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go @@ -20,9 +20,9 @@ import ( "context" "fmt" "reflect" - "strings" "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/runtime" @@ -69,16 +69,18 @@ func TestRequiredAffinitySingleNode(t *testing.T) { node1 := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: labels1}} tests := []struct { - pod *v1.Pod - pods []*v1.Pod - node *v1.Node - name string - wantStatus *framework.Status + pod *v1.Pod + pods []*v1.Pod + node *v1.Node + name string + wantPreFilterStatus *framework.Status + wantFilterStatus *framework.Status }{ { - name: "A pod that has no required pod affinity scheduling rules can schedule onto a node with no existing pods", - pod: new(v1.Pod), - node: &node1, + name: "A pod that has no required pod affinity scheduling rules can schedule onto a node with no existing pods", + pod: new(v1.Pod), + node: &node1, + wantPreFilterStatus: framework.NewStatus(framework.Skip), }, { name: "satisfies with requiredDuringSchedulingIgnoredDuringExecution in PodAffinity using In operator that matches the existing pod", @@ -111,7 +113,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { }, nil), pods: []*v1.Pod{st.MakePod().Namespace("ns").Label("service", "securityscan").Node("node1").Obj()}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.UnschedulableAndUnresolvable, ErrReasonAffinityRulesNotMatch, ), @@ -121,7 +123,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { pod: st.MakePod().Namespace(defaultNamespace).Labels(podLabel).PodAffinityIn("service", "", []string{"antivirusscan", "value2"}, st.PodAffinityWithRequiredReq).Obj(), pods: []*v1.Pod{pod}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.UnschedulableAndUnresolvable, ErrReasonAffinityRulesNotMatch, ), @@ -199,7 +201,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { }, nil), pods: []*v1.Pod{pod}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.UnschedulableAndUnresolvable, ErrReasonAffinityRulesNotMatch, ), @@ -230,7 +232,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAntiAffinityIn("service", "zone", []string{"securityscan", "value2"}, st.PodAntiAffinityWithRequiredReq).Obj(), pods: []*v1.Pod{pod}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, ), @@ -244,7 +246,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { st.MakePod().Namespace(defaultNamespace).Labels(podLabel).Node("node1").PodAntiAffinityIn("service", "zone", []string{"securityscan", "value2"}, st.PodAntiAffinityWithRequiredReq).Obj(), }, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonExistingAntiAffinityRulesNotMatch, ), @@ -255,7 +257,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAffinityNotIn("service", "region", []string{"securityscan", "value2"}, st.PodAffinityWithRequiredReq).Obj(), pods: []*v1.Pod{st.MakePod().Label("service", "securityscan").Node("node2").Obj()}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.UnschedulableAndUnresolvable, ErrReasonAffinityRulesNotMatch, ), @@ -268,7 +270,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAntiAffinityIn("service", "zone", []string{"securityscan", "value2"}, st.PodAntiAffinityWithRequiredReq).Obj(), }, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonExistingAntiAffinityRulesNotMatch, ), @@ -280,7 +282,8 @@ func TestRequiredAffinitySingleNode(t *testing.T) { st.MakePod().Namespace(defaultNamespace).Node("node1").Labels(podLabel). PodAntiAffinityNotIn("service", "zone", []string{"securityscan", "value2"}, st.PodAntiAffinityWithRequiredReq).Obj(), }, - node: &node1, + node: &node1, + wantPreFilterStatus: framework.NewStatus(framework.Skip), }, { name: "satisfies the PodAntiAffinity with existing pod but doesn't satisfy PodAntiAffinity symmetry with incoming pod", @@ -292,7 +295,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAntiAffinityExists("security", "zone", st.PodAntiAffinityWithRequiredReq).Obj(), }, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, ), @@ -307,7 +310,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAntiAffinityExists("security", "zone", st.PodAntiAffinityWithRequiredReq).Obj(), }, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, ), @@ -322,7 +325,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAntiAffinityExists("security", "zone", st.PodAntiAffinityWithRequiredReq).Obj(), }, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonExistingAntiAffinityRulesNotMatch, ), @@ -338,7 +341,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAntiAffinityExists("def", "zone", st.PodAntiAffinityWithRequiredReq).Obj(), }, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, ), @@ -354,7 +357,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAntiAffinityExists("def", "zone", st.PodAntiAffinityWithRequiredReq).Obj(), }, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, ), @@ -365,9 +368,9 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAffinityIn("service", "region", []string{"{{.bad-value.}}"}, st.PodAffinityWithRequiredReq). PodAffinityIn("service", "node", []string{"antivirusscan", "value2"}, st.PodAffinityWithRequiredReq).Obj(), node: &node1, - wantStatus: framework.NewStatus( + wantPreFilterStatus: framework.NewStatus( framework.UnschedulableAndUnresolvable, - `Invalid value: "{{.bad-value.}}"`, + `parsing pod: requiredAffinityTerms: values[0][service]: Invalid value: "{{.bad-value.}}": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')`, ), }, { @@ -376,9 +379,9 @@ func TestRequiredAffinitySingleNode(t *testing.T) { PodAffinityIn("service", "region", []string{"foo"}, st.PodAffinityWithRequiredReq). PodAffinityIn("service", "node", []string{"{{.bad-value.}}"}, st.PodAffinityWithRequiredReq).Obj(), node: &node1, - wantStatus: framework.NewStatus( + wantPreFilterStatus: framework.NewStatus( framework.UnschedulableAndUnresolvable, - `Invalid value: "{{.bad-value.}}"`, + `parsing pod: requiredAffinityTerms: values[0][service]: Invalid value: "{{.bad-value.}}": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')`, ), }, { @@ -438,7 +441,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { }, nil), pods: []*v1.Pod{{Spec: v1.PodSpec{NodeName: "node1"}, ObjectMeta: metav1.ObjectMeta{Namespace: "subteam1.team2", Labels: podLabel}}}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.UnschedulableAndUnresolvable, ErrReasonAffinityRulesNotMatch, ), @@ -471,7 +474,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { }), pods: []*v1.Pod{{Spec: v1.PodSpec{NodeName: "node1"}, ObjectMeta: metav1.ObjectMeta{Namespace: "subteam2.team1", Labels: podLabel}}}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, ), @@ -496,7 +499,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { }), pods: []*v1.Pod{{Spec: v1.PodSpec{NodeName: "node1"}, ObjectMeta: metav1.ObjectMeta{Namespace: "subteam2.team1", Labels: podLabel}}}, node: &node1, - wantStatus: framework.NewStatus( + wantFilterStatus: framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, ), @@ -540,16 +543,17 @@ func TestRequiredAffinitySingleNode(t *testing.T) { p := plugintesting.SetupPluginWithInformers(ctx, t, New, &config.InterPodAffinityArgs{}, snapshot, namespaces) state := framework.NewCycleState() _, preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, state, test.pod) + if diff := cmp.Diff(preFilterStatus, test.wantPreFilterStatus); diff != "" { + t.Errorf("PreFilter: status does not match (-want,+got):\n%s", diff) + } if !preFilterStatus.IsSuccess() { - if !strings.Contains(preFilterStatus.Message(), test.wantStatus.Message()) { - t.Errorf("prefilter failed with status: %v", preFilterStatus) - } - } else { - nodeInfo := mustGetNodeInfo(t, snapshot, test.node.Name) - gotStatus := p.(framework.FilterPlugin).Filter(ctx, state, test.pod, nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) - } + return + } + + nodeInfo := mustGetNodeInfo(t, snapshot, test.node.Name) + gotStatus := p.(framework.FilterPlugin).Filter(ctx, state, test.pod, nodeInfo) + if diff := cmp.Diff(gotStatus, test.wantFilterStatus); diff != "" { + t.Errorf("Filter: status does not match (-want,+got):\n%s", diff) } }) } @@ -571,11 +575,12 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { } tests := []struct { - pod *v1.Pod - pods []*v1.Pod - nodes []*v1.Node - wantStatuses []*framework.Status - name string + pod *v1.Pod + pods []*v1.Pod + nodes []*v1.Node + wantFilterStatuses []*framework.Status + wantPreFilterStatus *framework.Status + name string }{ { pod: st.MakePod().Namespace(defaultNamespace).PodAffinityIn("foo", "region", []string{"bar"}, st.PodAffinityWithRequiredReq).Obj(), @@ -587,7 +592,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "node2", Labels: labelRgChinaAzAz1}}, {ObjectMeta: metav1.ObjectMeta{Name: "node3", Labels: labelRgIndia}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ nil, nil, framework.NewStatus( @@ -608,7 +613,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"zone": "az1", "hostname": "h1"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"zone": "az2", "hostname": "h2"}}}, }, - wantStatuses: []*framework.Status{nil, nil}, + wantFilterStatuses: []*framework.Status{nil, nil}, name: "The affinity rule is to schedule all of the pods of this collection to the same zone. The first pod of the collection " + "should not be blocked from being scheduled onto any node, even there's no existing pod that matches the rule anywhere.", }, @@ -623,7 +628,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"zoneLabel": "az1", "hostname": "h1"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"zoneLabel": "az2", "hostname": "h2"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.UnschedulableAndUnresolvable, ErrReasonAffinityRulesNotMatch, @@ -644,7 +649,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, @@ -666,7 +671,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, @@ -688,7 +693,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: labelRgChinaAzAz1}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeC", Labels: labelRgIndia}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, @@ -712,7 +717,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: labelRgChinaAzAz1}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeC", Labels: labelRgIndia}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, @@ -734,8 +739,9 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{nil, nil}, - name: "Test existing pod's anti-affinity: if an existing pod has a term with invalid topologyKey, labelSelector of the term is firstly checked, and then topologyKey of the term is also checked", + wantPreFilterStatus: framework.NewStatus(framework.Skip), + wantFilterStatuses: []*framework.Status{nil, nil}, + name: "Test existing pod's anti-affinity: if an existing pod has a term with invalid topologyKey, labelSelector of the term is firstly checked, and then topologyKey of the term is also checked", }, { pod: st.MakePod().Node("nodeA").Namespace(defaultNamespace).PodAntiAffinityExists("foo", "invalid-node-label", st.PodAntiAffinityWithRequiredReq).Obj(), @@ -746,8 +752,8 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{nil, nil}, - name: "Test incoming pod's anti-affinity: even if labelSelector matches, we still check if topologyKey matches", + wantFilterStatuses: []*framework.Status{nil, nil}, + name: "Test incoming pod's anti-affinity: even if labelSelector matches, we still check if topologyKey matches", }, { pod: st.MakePod().Label("foo", "").Label("bar", "").Obj(), @@ -759,7 +765,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonExistingAntiAffinityRulesNotMatch, @@ -782,7 +788,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, @@ -804,7 +810,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonExistingAntiAffinityRulesNotMatch, @@ -823,7 +829,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, @@ -842,7 +848,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonExistingAntiAffinityRulesNotMatch, @@ -864,7 +870,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonAntiAffinityRulesNotMatch, @@ -889,7 +895,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeC", Labels: map[string]string{"region": "r1", "zone": "z3", "hostname": "nodeC"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.Unschedulable, ErrReasonExistingAntiAffinityRulesNotMatch, @@ -912,8 +918,8 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{nil, nil}, - name: "Test incoming pod's affinity: firstly check if all affinityTerms match, and then check if all topologyKeys match", + wantFilterStatuses: []*framework.Status{nil, nil}, + name: "Test incoming pod's affinity: firstly check if all affinityTerms match, and then check if all topologyKeys match", }, { pod: st.MakePod().Namespace(defaultNamespace).PodAffinityExists("foo", "region", st.PodAffinityWithRequiredReq). @@ -926,7 +932,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: map[string]string{"region": "r1", "zone": "z1", "hostname": "nodeA"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: map[string]string{"region": "r1", "zone": "z2", "hostname": "nodeB"}}}, }, - wantStatuses: []*framework.Status{ + wantFilterStatuses: []*framework.Status{ framework.NewStatus( framework.UnschedulableAndUnresolvable, ErrReasonAffinityRulesNotMatch, @@ -949,16 +955,19 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { []runtime.Object{ &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "NS1"}}, }) + state := framework.NewCycleState() + _, preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, state, test.pod) + if diff := cmp.Diff(preFilterStatus, test.wantPreFilterStatus); diff != "" { + t.Errorf("PreFilter: status does not match (-want,+got):\n%s", diff) + } + if preFilterStatus.IsSkip() { + return + } for indexNode, node := range test.nodes { - state := framework.NewCycleState() - _, preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, state, test.pod) - if !preFilterStatus.IsSuccess() { - t.Errorf("prefilter failed with status: %v", preFilterStatus) - } nodeInfo := mustGetNodeInfo(t, snapshot, node.Name) gotStatus := p.(framework.FilterPlugin).Filter(ctx, state, test.pod, nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatuses[indexNode]) { - t.Errorf("index: %d status does not match: %v, want: %v", indexTest, gotStatus, test.wantStatuses[indexNode]) + if diff := cmp.Diff(gotStatus, test.wantFilterStatuses[indexNode]); diff != "" { + t.Errorf("index: %d: Filter: status does not match (-want,+got):\n%s", indexTest, diff) } } }) @@ -1077,26 +1086,6 @@ func TestPreFilterStateAddRemovePod(t *testing.T) { expectedAntiAffinity topologyToMatchedTermCount expectedAffinity topologyToMatchedTermCount }{ - { - name: "no affinity exist", - pendingPod: st.MakePod().Name("pending").Labels(selector1).Obj(), - existingPods: []*v1.Pod{ - {ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, - Spec: v1.PodSpec{NodeName: "nodeA"}, - }, - {ObjectMeta: metav1.ObjectMeta{Name: "p2"}, - Spec: v1.PodSpec{NodeName: "nodeC"}, - }, - }, - addedPod: st.MakePod().Name("addedPod").Labels(selector1).Node("nodeB").Obj(), - nodes: []*v1.Node{ - {ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: label1}}, - {ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: label2}}, - {ObjectMeta: metav1.ObjectMeta{Name: "nodeC", Labels: label3}}, - }, - expectedAntiAffinity: topologyToMatchedTermCount{}, - expectedAffinity: topologyToMatchedTermCount{}, - }, { name: "preFilterState anti-affinity terms are updated correctly after adding and removing a pod", pendingPod: &v1.Pod{