From 7b7da83a8a6f6017620fb8da3745f4b1150f6ac4 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Thu, 10 Nov 2022 14:07:02 +0800 Subject: [PATCH] Fix: resourceToWeightMap will never be nil Use len() instead of telling whether it's nil Add tests to guarantee that when resourceToWeightMap is nil, scheduler will not crash Signed-off-by: kerthcet --- .../plugins/noderesources/least_allocated_test.go | 8 +++++--- .../plugins/noderesources/most_allocated_test.go | 8 +++++--- .../plugins/noderesources/resource_allocation.go | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/scheduler/framework/plugins/noderesources/least_allocated_test.go b/pkg/scheduler/framework/plugins/noderesources/least_allocated_test.go index 011c351eea4..4c6d662e48d 100644 --- a/pkg/scheduler/framework/plugins/noderesources/least_allocated_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/least_allocated_test.go @@ -52,6 +52,7 @@ func TestLeastAllocatedScoringStrategy(t *testing.T) { expectedScores framework.NodeScoreList resources []config.ResourceSpec wantErrs field.ErrorList + wantStatusCode framework.Code }{ { // Node1 scores (remaining resources) on 0-MaxNodeScore scale @@ -95,7 +96,7 @@ func TestLeastAllocatedScoringStrategy(t *testing.T) { resources: defaultResources, }, { - name: "Resources not set, nothing scheduled, resources requested, differently sized nodes", + name: "Resources not set, pods scheduled with error", requestedPod: st.MakePod(). Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}). Req(map[v1.ResourceName]string{"cpu": "2000", "memory": "3000"}). @@ -107,6 +108,7 @@ func TestLeastAllocatedScoringStrategy(t *testing.T) { existingPods: nil, expectedScores: []framework.NodeScore{{Name: "node1", Score: framework.MinNodeScore}, {Name: "node2", Score: framework.MinNodeScore}}, resources: nil, + wantStatusCode: framework.Error, }, { // Node1 scores on 0-MaxNodeScore scale @@ -399,8 +401,8 @@ func TestLeastAllocatedScoringStrategy(t *testing.T) { var gotScores framework.NodeScoreList for _, n := range test.nodes { score, status := p.(framework.ScorePlugin).Score(ctx, state, test.requestedPod, n.Name) - if !status.IsSuccess() { - t.Errorf("unexpected error: %v", status) + if status.Code() != test.wantStatusCode { + t.Errorf("unexpected status code, want: %v, got: %v", test.wantStatusCode, status) } gotScores = append(gotScores, framework.NodeScore{Name: n.Name, Score: score}) } diff --git a/pkg/scheduler/framework/plugins/noderesources/most_allocated_test.go b/pkg/scheduler/framework/plugins/noderesources/most_allocated_test.go index d60a480d517..6d191668d99 100644 --- a/pkg/scheduler/framework/plugins/noderesources/most_allocated_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/most_allocated_test.go @@ -51,6 +51,7 @@ func TestMostAllocatedScoringStrategy(t *testing.T) { expectedScores framework.NodeScoreList resources []config.ResourceSpec wantErrs field.ErrorList + wantStatusCode framework.Code }{ { // Node1 scores (used resources) on 0-MaxNodeScore scale @@ -94,7 +95,7 @@ func TestMostAllocatedScoringStrategy(t *testing.T) { resources: defaultResources, }, { - name: "Resources not set, nothing scheduled, resources requested, differently sized nodes", + name: "Resources not set, pods scheduled with error", requestedPod: st.MakePod(). Req(map[v1.ResourceName]string{"cpu": "1000", "memory": "2000"}). Req(map[v1.ResourceName]string{"cpu": "2000", "memory": "3000"}). @@ -106,6 +107,7 @@ func TestMostAllocatedScoringStrategy(t *testing.T) { existingPods: nil, expectedScores: []framework.NodeScore{{Name: "node1", Score: framework.MinNodeScore}, {Name: "node2", Score: framework.MinNodeScore}}, resources: nil, + wantStatusCode: framework.Error, }, { // Node1 scores on 0-MaxNodeScore scale @@ -355,8 +357,8 @@ func TestMostAllocatedScoringStrategy(t *testing.T) { var gotScores framework.NodeScoreList for _, n := range test.nodes { score, status := p.(framework.ScorePlugin).Score(ctx, state, test.requestedPod, n.Name) - if !status.IsSuccess() { - t.Errorf("unexpected error: %v", status) + if status.Code() != test.wantStatusCode { + t.Errorf("unexpected status code, want: %v, got: %v", test.wantStatusCode, status.Code()) } gotScores = append(gotScores, framework.NodeScore{Name: n.Name, Score: score}) } diff --git a/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go b/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go index 6715ccffab2..10e4fbc7f4a 100644 --- a/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go +++ b/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go @@ -51,7 +51,7 @@ func (r *resourceAllocationScorer) score( if node == nil { return 0, framework.NewStatus(framework.Error, "node not found") } - if r.resourceToWeightMap == nil { + if len(r.resourceToWeightMap) == 0 { return 0, framework.NewStatus(framework.Error, "resources not found") }