Skip to content

Commit b690eab

Browse files
committed
fix reviewed comments
1 parent e530102 commit b690eab

File tree

1 file changed

+40
-53
lines changed

1 file changed

+40
-53
lines changed

pkg/capacityscheduling/capacity_scheduling_test.go

Lines changed: 40 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func TestPostFilter(t *testing.T) {
184184
tests := []struct {
185185
name string
186186
pod *v1.Pod
187-
pods []*v1.Pod
187+
existPods []*v1.Pod
188188
nodes []*v1.Node
189189
filteredNodesStatuses framework.NodeToStatusMap
190190
elasticQuotas map[string]*ElasticQuotaInfo
@@ -194,7 +194,7 @@ func TestPostFilter(t *testing.T) {
194194
{
195195
name: "in-namespace preemption",
196196
pod: makePod("t1-p1", "ns1", 50, 0, 0, highPriority, "t1-p1", ""),
197-
pods: []*v1.Pod{
197+
existPods: []*v1.Pod{
198198
makePod("t1-p2", "ns1", 50, 0, 0, midPriority, "t1-p2", "node-a"),
199199
makePod("t1-p3", "ns2", 50, 0, 0, midPriority, "t1-p3", "node-a"),
200200
makePod("t1-p4", "ns2", 50, 0, 0, midPriority, "t1-p4", "node-a"),
@@ -237,7 +237,7 @@ func TestPostFilter(t *testing.T) {
237237
{
238238
name: "cross-namespace preemption",
239239
pod: makePod("t1-p1", "ns1", 50, 0, 0, highPriority, "t1-p1", ""),
240-
pods: []*v1.Pod{
240+
existPods: []*v1.Pod{
241241
makePod("t1-p2", "ns1", 50, 0, 0, midPriority, "t1-p2", "node-a"),
242242
makePod("t1-p3", "ns2", 50, 0, 0, midPriority, "t1-p3", "node-a"),
243243
makePod("t1-p4", "ns2", 50, 0, 0, midPriority, "t1-p4", "node-a"),
@@ -278,9 +278,9 @@ func TestPostFilter(t *testing.T) {
278278
wantStatus: framework.NewStatus(framework.Success),
279279
},
280280
{
281-
name: "Without elasticQuotas",
281+
name: "without elasticQuotas",
282282
pod: makePod("t1-p1", "ns1", 50, 0, 0, highPriority, "t1-p1", ""),
283-
pods: []*v1.Pod{
283+
existPods: []*v1.Pod{
284284
makePod("t1-p2", "ns1", 50, 0, 0, midPriority, "t1-p2", "node-a"),
285285
makePod("t1-p3", "ns2", 50, 0, 0, midPriority, "t1-p3", "node-a"),
286286
makePod("t1-p4", "ns2", 50, 0, 0, midPriority, "t1-p4", "node-a"),
@@ -299,24 +299,18 @@ func TestPostFilter(t *testing.T) {
299299

300300
for _, tt := range tests {
301301
t.Run(tt.name, func(t *testing.T) {
302-
registeredPlugins := []st.RegisterPluginFunc{
303-
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
304-
st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
305-
st.RegisterPluginAsExtensions(noderesources.Name, func(plArgs apiruntime.Object, fh framework.Handle) (framework.Plugin, error) {
306-
return noderesources.NewFit(plArgs, fh, plfeature.Features{})
307-
}, "Filter", "PreFilter"),
308-
}
302+
registeredPlugins := makeRegisteredPlugin()
309303

310304
podItems := []v1.Pod{}
311-
for _, pod := range tt.pods {
305+
for _, pod := range tt.existPods {
312306
podItems = append(podItems, *pod)
313307
}
314308
cs := clientsetfake.NewSimpleClientset(&v1.PodList{Items: podItems})
315309
informerFactory := informers.NewSharedInformerFactory(cs, 0)
316310
podInformer := informerFactory.Core().V1().Pods().Informer()
317311
podInformer.GetStore().Add(tt.pod)
318-
for i := range tt.pods {
319-
podInformer.GetStore().Add(tt.pods[i])
312+
for i := range tt.existPods {
313+
podInformer.GetStore().Add(tt.existPods[i])
320314
}
321315
ctx, cancel := context.WithCancel(context.Background())
322316
defer cancel()
@@ -329,7 +323,7 @@ func TestPostFilter(t *testing.T) {
329323
frameworkruntime.WithEventRecorder(&events.FakeRecorder{}),
330324
frameworkruntime.WithInformerFactory(informerFactory),
331325
frameworkruntime.WithPodNominator(testutil.NewPodNominator(informerFactory.Core().V1().Pods().Lister())),
332-
frameworkruntime.WithSnapshotSharedLister(testutil.NewFakeSharedLister(tt.pods, tt.nodes)),
326+
frameworkruntime.WithSnapshotSharedLister(testutil.NewFakeSharedLister(tt.existPods, tt.nodes)),
333327
)
334328
if err != nil {
335329
t.Fatal(err)
@@ -360,14 +354,8 @@ func TestPostFilter(t *testing.T) {
360354
pdbLister: getPDBLister(informerFactory),
361355
}
362356
gotResult, gotStatus := c.PostFilter(ctx, state, tt.pod, tt.filteredNodesStatuses)
363-
if gotStatus.Code() == framework.Error {
364-
if diff := gocmp.Diff(tt.wantStatus.Reasons(), gotStatus.Reasons()); diff != "" {
365-
t.Errorf("Unexpected status (-want, +got):\n%s", diff)
366-
}
367-
} else {
368-
if diff := gocmp.Diff(tt.wantStatus, gotStatus); diff != "" {
369-
t.Errorf("Unexpected status (-want, +got):\n%s", diff)
370-
}
357+
if diff := gocmp.Diff(tt.wantStatus, gotStatus); diff != "" {
358+
t.Errorf("Unexpected status (-want, +got):\n%s", diff)
371359
}
372360
if diff := gocmp.Diff(tt.wantResult, gotResult); diff != "" {
373361
t.Errorf("Unexpected postFilterResult (-want, +got):\n%s", diff)
@@ -722,13 +710,7 @@ func TestDryRunPreemption(t *testing.T) {
722710

723711
for _, tt := range tests {
724712
t.Run(tt.name, func(t *testing.T) {
725-
registeredPlugins := []st.RegisterPluginFunc{
726-
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
727-
st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
728-
st.RegisterPluginAsExtensions(noderesources.Name, func(plArgs apiruntime.Object, fh framework.Handle) (framework.Plugin, error) {
729-
return noderesources.NewFit(plArgs, fh, plfeature.Features{})
730-
}, "Filter", "PreFilter"),
731-
}
713+
registeredPlugins := makeRegisteredPlugin()
732714

733715
cs := clientsetfake.NewSimpleClientset()
734716
ctx := context.Background()
@@ -815,16 +797,16 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
815797
tests := []struct {
816798
name string
817799
pod *v1.Pod
818-
pods []*v1.Pod
800+
existPods []*v1.Pod
819801
nodes []*v1.Node
820802
nominatedNodeStatus *framework.Status
821803
elasticQuotas map[string]*ElasticQuotaInfo
822804
expected bool
823805
}{
824806
{
825-
name: "Pod with PreemptNever preemption policy",
826-
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Priority(highPriority).PreemptionPolicy(v1.PreemptNever).Obj(),
827-
pods: []*v1.Pod{makePod("t1-p0", "ns1", 50, 10, 0, midPriority, "t1-p1", "node-a")},
807+
name: "Pod with PreemptNever preemption policy",
808+
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Priority(highPriority).PreemptionPolicy(v1.PreemptNever).Obj(),
809+
existPods: []*v1.Pod{makePod("t1-p0", "ns1", 50, 10, 0, midPriority, "t1-p1", "node-a")},
828810
nodes: []*v1.Node{
829811
st.MakeNode().Name("node-a").Capacity(res).Obj(),
830812
},
@@ -846,9 +828,9 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
846828
expected: false,
847829
},
848830
{
849-
name: "Pod with nominated node",
850-
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Priority(highPriority).NominatedNodeName("node-a").Obj(),
851-
pods: []*v1.Pod{makePod("t1-p0", "ns1", 50, 10, 0, midPriority, "t1-p1", "node-a")},
831+
name: "Pod with unschedulableAndUnresolvable nominated node",
832+
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Priority(highPriority).NominatedNodeName("node-a").Obj(),
833+
existPods: []*v1.Pod{makePod("t1-p0", "ns1", 50, 10, 0, midPriority, "t1-p1", "node-a")},
852834
nodes: []*v1.Node{
853835
st.MakeNode().Name("node-a").Capacity(res).Obj(),
854836
},
@@ -872,7 +854,7 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
872854
{
873855
name: "Pod with terminating pod in the same namespace",
874856
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Namespace("ns1").Priority(highPriority).NominatedNodeName("node-a").Obj(),
875-
pods: []*v1.Pod{
857+
existPods: []*v1.Pod{
876858
st.MakePod().Name("t1-p0").UID("t1-p0").Namespace("ns1").Priority(midPriority).Node("node-a").Terminating().Obj(),
877859
},
878860
nodes: []*v1.Node{
@@ -896,9 +878,9 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
896878
expected: false,
897879
},
898880
{
899-
name: "Pod with terminating pod in the different namespace",
881+
name: "Pod with terminating pod in another namespace",
900882
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Namespace("ns1").Priority(highPriority).NominatedNodeName("node-a").Obj(),
901-
pods: []*v1.Pod{
883+
existPods: []*v1.Pod{
902884
st.MakePod().Name("t1-p0").UID("t1-p0").Namespace("ns2").Priority(midPriority).Node("node-a").Terminating().Obj(),
903885
},
904886
nodes: []*v1.Node{
@@ -934,9 +916,9 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
934916
expected: false,
935917
},
936918
{
937-
name: "Pod without ElasticQuotas and terminating pod exists",
919+
name: "Pod without elasticQuotas and terminating pod exists in the same namespace",
938920
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Namespace("ns1").Priority(highPriority).NominatedNodeName("node-a").Obj(),
939-
pods: []*v1.Pod{
921+
existPods: []*v1.Pod{
940922
st.MakePod().Name("t1-p0").UID("t1-p0").Namespace("ns2").Priority(midPriority).Node("node-a").Terminating().Obj(),
941923
},
942924
nodes: []*v1.Node{
@@ -947,9 +929,9 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
947929
expected: false,
948930
},
949931
{
950-
name: "Pod with other namespace's ElasticQuotas and terminating pod exists",
932+
name: "Pod without ElasticQuotas and terminating pod exists in another namespace",
951933
pod: st.MakePod().Name("t1-p1").UID("t1-p1").Namespace("ns1").Priority(highPriority).NominatedNodeName("node-a").Obj(),
952-
pods: []*v1.Pod{
934+
existPods: []*v1.Pod{
953935
st.MakePod().Name("t1-p0").UID("t1-p0").Namespace("ns2").Priority(midPriority).Node("node-a").Terminating().Obj(),
954936
},
955937
nodes: []*v1.Node{
@@ -976,13 +958,7 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
976958

977959
for _, tt := range tests {
978960
t.Run(tt.name, func(t *testing.T) {
979-
registeredPlugins := []st.RegisterPluginFunc{
980-
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
981-
st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
982-
st.RegisterPluginAsExtensions(noderesources.Name, func(plArgs apiruntime.Object, fh framework.Handle) (framework.Plugin, error) {
983-
return noderesources.NewFit(plArgs, fh, plfeature.Features{})
984-
}, "Filter", "PreFilter"),
985-
}
961+
registeredPlugins := makeRegisteredPlugin()
986962
cs := clientsetfake.NewSimpleClientset()
987963
ctx := context.Background()
988964

@@ -993,7 +969,7 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
993969
frameworkruntime.WithClientSet(cs),
994970
frameworkruntime.WithEventRecorder(&events.FakeRecorder{}),
995971
frameworkruntime.WithPodNominator(testutil.NewPodNominator(nil)),
996-
frameworkruntime.WithSnapshotSharedLister(testutil.NewFakeSharedLister(tt.pods, tt.nodes)),
972+
frameworkruntime.WithSnapshotSharedLister(testutil.NewFakeSharedLister(tt.existPods, tt.nodes)),
997973
frameworkruntime.WithInformerFactory(informers.NewSharedInformerFactory(cs, 0)),
998974
)
999975
if err != nil {
@@ -1632,3 +1608,14 @@ func makeResourceList(cpu, mem int64) v1.ResourceList {
16321608
v1.ResourceMemory: *resource.NewQuantity(mem, resource.BinarySI),
16331609
}
16341610
}
1611+
1612+
func makeRegisteredPlugin() []st.RegisterPluginFunc {
1613+
registeredPlugins := []st.RegisterPluginFunc{
1614+
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
1615+
st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New),
1616+
st.RegisterPluginAsExtensions(noderesources.Name, func(plArgs apiruntime.Object, fh framework.Handle) (framework.Plugin, error) {
1617+
return noderesources.NewFit(plArgs, fh, plfeature.Features{})
1618+
}, "Filter", "PreFilter"),
1619+
}
1620+
return registeredPlugins
1621+
}

0 commit comments

Comments
 (0)