From c0611b6bb331854ba8f2a532248a32f5acce5ff1 Mon Sep 17 00:00:00 2001 From: utam0k Date: Fri, 5 May 2023 02:26:36 +0000 Subject: [PATCH] Return Skip in InterPodAffinity#PreScore under specific conditions This commit updates the InterPodAffinity PreScore to return a Skip status when the following conditions are met: 1. There are no nodes to score. 2. The incoming pod has no inter-pod affinities && the `IgnorePreferredTermsOfExistingPods` option is enabled. Signed-off-by: utam0k --- .../plugins/interpodaffinity/scoring.go | 11 +-- .../plugins/interpodaffinity/scoring_test.go | 69 ++++++++++++------- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/scoring.go b/pkg/scheduler/framework/plugins/interpodaffinity/scoring.go index fd4cc24327d..2420d20a58f 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/scoring.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/scoring.go @@ -131,7 +131,7 @@ func (pl *InterPodAffinity) PreScore( ) *framework.Status { if len(nodes) == 0 { // No nodes to score. - return nil + return framework.NewStatus(framework.Skip) } if pl.sharedLister == nil { @@ -145,10 +145,7 @@ func (pl *InterPodAffinity) PreScore( // Optionally ignore calculating preferences of existing pods' affinity rules // if the incoming pod has no inter-pod affinities. if pl.args.IgnorePreferredTermsOfExistingPods && !hasPreferredAffinityConstraints && !hasPreferredAntiAffinityConstraints { - cycleState.Write(preScoreStateKey, &preScoreState{ - topologyScore: make(map[string]map[string]int64), - }) - return nil + return framework.NewStatus(framework.Skip) } // Unless the pod being scheduled has preferred affinity terms, we only @@ -213,6 +210,10 @@ func (pl *InterPodAffinity) PreScore( } pl.parallelizer.Until(pCtx, len(allNodes), processNode, pl.Name()) + if index == -1 { + return framework.NewStatus(framework.Skip) + } + for i := 0; i <= int(index); i++ { state.topologyScore.append(topoScores[i]) } diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go b/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go index ec775d9dbb0..96eb0fc5352 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go @@ -22,6 +22,7 @@ import ( "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" @@ -385,7 +386,7 @@ func TestPreferredAffinity(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "node2", Labels: labelRgIndia}}, {ObjectMeta: metav1.ObjectMeta{Name: "node3", Labels: labelAzAz1}}, }, - expectedList: []framework.NodeScore{{Name: "node1", Score: 0}, {Name: "node2", Score: 0}, {Name: "node3", Score: 0}}, + wantStatus: framework.NewStatus(framework.Skip), }, // the node(node1) that have the label {"region": "China"} (match the topology key) and that have existing pods that match the labelSelector get high score // the node(node3) that don't have the label {"region": "whatever the value is"} (mismatch the topology key) but that have existing pods that match the labelSelector get low score @@ -749,6 +750,7 @@ func TestPreferredAffinity(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "node2", Labels: labelRgIndia}}, }, expectedList: []framework.NodeScore{{Name: "node1", Score: 0}, {Name: "node2", Score: 0}}, + wantStatus: framework.NewStatus(framework.Skip), ignorePreferredTermsOfExistingPods: true, }, { @@ -765,6 +767,13 @@ func TestPreferredAffinity(t *testing.T) { expectedList: []framework.NodeScore{{Name: "node1", Score: 0}, {Name: "node2", Score: framework.MaxNodeScore}}, ignorePreferredTermsOfExistingPods: false, }, + { + name: "No nodes to score", + pod: &v1.Pod{Spec: v1.PodSpec{NodeName: ""}, ObjectMeta: metav1.ObjectMeta{Labels: podLabelSecurityS2}}, + pods: []*v1.Pod{}, + nodes: []*v1.Node{}, + wantStatus: framework.NewStatus(framework.Skip), + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -773,31 +782,36 @@ func TestPreferredAffinity(t *testing.T) { state := framework.NewCycleState() p := plugintesting.SetupPluginWithInformers(ctx, t, New, &config.InterPodAffinityArgs{HardPodAffinityWeight: 1, IgnorePreferredTermsOfExistingPods: test.ignorePreferredTermsOfExistingPods}, cache.NewSnapshot(test.pods, test.nodes), namespaces) status := p.(framework.PreScorePlugin).PreScore(ctx, state, test.pod, test.nodes) + if !status.IsSuccess() { + if status.Code() != test.wantStatus.Code() { + t.Errorf("InterPodAffinity#PreScore() returned unexpected status.Code got: %v, want: %v", status.Code(), test.wantStatus.Code()) + } + if !strings.Contains(status.Message(), test.wantStatus.Message()) { - t.Errorf("unexpected error: %v", status) - } - } else { - var gotList framework.NodeScoreList - for _, n := range test.nodes { - nodeName := n.ObjectMeta.Name - score, status := p.(framework.ScorePlugin).Score(ctx, state, test.pod, nodeName) - if !status.IsSuccess() { - t.Errorf("unexpected error: %v", status) - } - gotList = append(gotList, framework.NodeScore{Name: nodeName, Score: score}) - } - - status = p.(framework.ScorePlugin).ScoreExtensions().NormalizeScore(ctx, state, test.pod, gotList) - if !status.IsSuccess() { - t.Errorf("unexpected error: %v", status) - } - - if !reflect.DeepEqual(test.expectedList, gotList) { - t.Errorf("expected:\n\t%+v,\ngot:\n\t%+v", test.expectedList, gotList) + t.Errorf("InterPodAffinity#PreScore() returned unexpected status.Message got: %v, want: %v", status.Message(), test.wantStatus.Message()) } + return } + var gotList framework.NodeScoreList + for _, n := range test.nodes { + nodeName := n.ObjectMeta.Name + score, status := p.(framework.ScorePlugin).Score(ctx, state, test.pod, nodeName) + if !status.IsSuccess() { + t.Errorf("unexpected error from Score: %v", status) + } + gotList = append(gotList, framework.NodeScore{Name: nodeName, Score: score}) + } + + status = p.(framework.ScorePlugin).ScoreExtensions().NormalizeScore(ctx, state, test.pod, gotList) + if !status.IsSuccess() { + t.Errorf("unexpected error from NormalizeScore: %v", status) + } + + if diff := cmp.Diff(test.expectedList, gotList); diff != "" { + t.Errorf("node score list doesn't match (-want,+got): \n %s", diff) + } }) } } @@ -850,6 +864,7 @@ func TestPreferredAffinityWithHardPodAffinitySymmetricWeight(t *testing.T) { hardPodAffinityWeight int32 expectedList framework.NodeScoreList name string + wantStatus *framework.Status }{ { name: "with default weight", @@ -879,7 +894,7 @@ func TestPreferredAffinityWithHardPodAffinitySymmetricWeight(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "node3", Labels: labelAzAz1}}, }, hardPodAffinityWeight: 0, - expectedList: []framework.NodeScore{{Name: "node1", Score: 0}, {Name: "node2", Score: 0}, {Name: "node3", Score: 0}}, + wantStatus: framework.NewStatus(framework.Skip), }, { name: "with no matching namespace", @@ -894,7 +909,7 @@ func TestPreferredAffinityWithHardPodAffinitySymmetricWeight(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "node3", Labels: labelAzAz1}}, }, hardPodAffinityWeight: v1.DefaultHardPodAffinitySymmetricWeight, - expectedList: []framework.NodeScore{{Name: "node1", Score: 0}, {Name: "node2", Score: 0}, {Name: "node3", Score: 0}}, + wantStatus: framework.NewStatus(framework.Skip), }, { name: "with matching NamespaceSelector", @@ -934,9 +949,13 @@ func TestPreferredAffinityWithHardPodAffinitySymmetricWeight(t *testing.T) { state := framework.NewCycleState() p := plugintesting.SetupPluginWithInformers(ctx, t, New, &config.InterPodAffinityArgs{HardPodAffinityWeight: test.hardPodAffinityWeight}, cache.NewSnapshot(test.pods, test.nodes), namespaces) status := p.(framework.PreScorePlugin).PreScore(ctx, state, test.pod, test.nodes) - if !status.IsSuccess() { - t.Errorf("unexpected error: %v", status) + if !test.wantStatus.Equal(status) { + t.Errorf("InterPodAffinity#PreScore() returned unexpected status.Code got: %v, want: %v", status.Code(), test.wantStatus.Code()) } + if !status.IsSuccess() { + return + } + var gotList framework.NodeScoreList for _, n := range test.nodes { nodeName := n.ObjectMeta.Name