Skip to content

Commit 52bf6ba

Browse files
Huang-WeiHuang-Wei
committed
Preemption plugin to fetch pod from informer cache
1 parent 3cf8009 commit 52bf6ba

File tree

3 files changed

+68
-69
lines changed

3 files changed

+68
-69
lines changed

‎pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,10 @@ func (pl *DefaultPreemption) preempt(ctx context.Context, state *framework.Cycle
106106
nodeLister := pl.fh.SnapshotSharedLister().NodeInfos()
107107

108108
// 0) Fetch the latest version of <pod>.
109-
// TODO(Huang-Wei): get pod from informer cache instead of API server.
110-
pod, err := util.GetUpdatedPod(cs, pod)
109+
// It's safe to directly fetch pod here. Because the informer cache has already been
110+
// initialized when creating the Scheduler obj, i.e., factory.go#MakeDefaultErrorFunc().
111+
// However, tests may need to manually initialize the shared pod informer.
112+
pod, err := pl.fh.SharedInformerFactory().Core().V1().Pods().Lister().Pods(pod.Namespace).Get(pod.Name)
111113
if err != nil {
112114
klog.Errorf("Error getting the updated preemptor pod object: %v", err)
113115
return "", err

‎pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go‎

Lines changed: 64 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,6 @@ var (
8585
epochTime6 = metav1.NewTime(time.Unix(0, 6))
8686
)
8787

88-
func mergeObjs(pod *v1.Pod, pods []*v1.Pod) []runtime.Object {
89-
var objs []runtime.Object
90-
if pod != nil {
91-
objs = append(objs, pod)
92-
}
93-
for i := range pods {
94-
objs = append(objs, pods[i])
95-
}
96-
return objs
97-
}
98-
9988
func TestPostFilter(t *testing.T) {
10089
onePodRes := map[v1.ResourceName]string{v1.ResourcePods: "1"}
10190
tests := []struct {
@@ -110,9 +99,9 @@ func TestPostFilter(t *testing.T) {
11099
}{
111100
{
112101
name: "pod with higher priority can be made schedulable",
113-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Obj(),
102+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Obj(),
114103
pods: []*v1.Pod{
115-
st.MakePod().Name("p1").UID("p1").Node("node1").Obj(),
104+
st.MakePod().Name("p1").UID("p1").Namespace(v1.NamespaceDefault).Node("node1").Obj(),
116105
},
117106
nodes: []*v1.Node{
118107
st.MakeNode().Name("node1").Capacity(onePodRes).Obj(),
@@ -125,9 +114,9 @@ func TestPostFilter(t *testing.T) {
125114
},
126115
{
127116
name: "pod with tied priority is still unschedulable",
128-
pod: st.MakePod().Name("p").UID("p").Obj(),
117+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Obj(),
129118
pods: []*v1.Pod{
130-
st.MakePod().Name("p1").UID("p1").Node("node1").Obj(),
119+
st.MakePod().Name("p1").UID("p1").Namespace(v1.NamespaceDefault).Node("node1").Obj(),
131120
},
132121
nodes: []*v1.Node{
133122
st.MakeNode().Name("node1").Capacity(onePodRes).Obj(),
@@ -140,9 +129,9 @@ func TestPostFilter(t *testing.T) {
140129
},
141130
{
142131
name: "preemption should respect filteredNodesStatuses",
143-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Obj(),
132+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Obj(),
144133
pods: []*v1.Pod{
145-
st.MakePod().Name("p1").UID("p1").Node("node1").Obj(),
134+
st.MakePod().Name("p1").UID("p1").Namespace(v1.NamespaceDefault).Node("node1").Obj(),
146135
},
147136
nodes: []*v1.Node{
148137
st.MakeNode().Name("node1").Capacity(onePodRes).Obj(),
@@ -155,10 +144,10 @@ func TestPostFilter(t *testing.T) {
155144
},
156145
{
157146
name: "pod can be made schedulable on one node",
158-
pod: st.MakePod().Name("p").UID("p").Priority(midPriority).Obj(),
147+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(midPriority).Obj(),
159148
pods: []*v1.Pod{
160-
st.MakePod().Name("p1").UID("p1").Priority(highPriority).Node("node1").Obj(),
161-
st.MakePod().Name("p2").UID("p2").Priority(lowPriority).Node("node2").Obj(),
149+
st.MakePod().Name("p1").UID("p1").Namespace(v1.NamespaceDefault).Priority(highPriority).Node("node1").Obj(),
150+
st.MakePod().Name("p2").UID("p2").Namespace(v1.NamespaceDefault).Priority(lowPriority).Node("node2").Obj(),
162151
},
163152
nodes: []*v1.Node{
164153
st.MakeNode().Name("node1").Capacity(onePodRes).Obj(),
@@ -173,10 +162,10 @@ func TestPostFilter(t *testing.T) {
173162
},
174163
{
175164
name: "preemption result filtered out by extenders",
176-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Obj(),
165+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Obj(),
177166
pods: []*v1.Pod{
178-
st.MakePod().Name("p1").UID("p1").Node("node1").Obj(),
179-
st.MakePod().Name("p2").UID("p2").Node("node2").Obj(),
167+
st.MakePod().Name("p1").UID("p1").Namespace(v1.NamespaceDefault).Node("node1").Obj(),
168+
st.MakePod().Name("p2").UID("p2").Namespace(v1.NamespaceDefault).Node("node2").Obj(),
180169
},
181170
nodes: []*v1.Node{
182171
st.MakeNode().Name("node1").Capacity(onePodRes).Obj(),
@@ -196,9 +185,18 @@ func TestPostFilter(t *testing.T) {
196185

197186
for _, tt := range tests {
198187
t.Run(tt.name, func(t *testing.T) {
199-
apiObjs := mergeObjs(tt.pod, tt.pods /*, tt.nodes */)
200-
cs := clientsetfake.NewSimpleClientset(apiObjs...)
188+
cs := clientsetfake.NewSimpleClientset()
201189
informerFactory := informers.NewSharedInformerFactory(cs, 0)
190+
podInformer := informerFactory.Core().V1().Pods().Informer()
191+
podInformer.GetStore().Add(tt.pod)
192+
for i := range tt.pods {
193+
podInformer.GetStore().Add(tt.pods[i])
194+
}
195+
// As we use a bare clientset above, it's needed to add a reactor here
196+
// to not fail Victims deletion logic.
197+
cs.PrependReactor("delete", "pods", func(action clienttesting.Action) (bool, runtime.Object, error) {
198+
return true, nil, nil
199+
})
202200
// Register NodeResourceFit as the Filter & PreFilter plugin.
203201
registeredPlugins := []st.RegisterPluginFunc{
204202
st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New),
@@ -1105,7 +1103,7 @@ func TestPreempt(t *testing.T) {
11051103
}{
11061104
{
11071105
name: "basic preemption logic",
1108-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
1106+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
11091107
pods: []*v1.Pod{
11101108
st.MakePod().Name("p1.1").UID("p1.1").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
11111109
st.MakePod().Name("p1.2").UID("p1.2").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
@@ -1119,16 +1117,16 @@ func TestPreempt(t *testing.T) {
11191117
},
11201118
{
11211119
name: "preemption for topology spread constraints",
1122-
pod: st.MakePod().Name("p").UID("p").Label("foo", "").Priority(highPriority).
1120+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Label("foo", "").Priority(highPriority).
11231121
SpreadConstraint(1, "zone", v1.DoNotSchedule, st.MakeLabelSelector().Exists("foo").Obj()).
11241122
SpreadConstraint(1, "hostname", v1.DoNotSchedule, st.MakeLabelSelector().Exists("foo").Obj()).
11251123
Obj(),
11261124
pods: []*v1.Pod{
1127-
st.MakePod().Name("p-a1").UID("p-a1").Node("node-a").Label("foo", "").Priority(highPriority).Obj(),
1128-
st.MakePod().Name("p-a2").UID("p-a2").Node("node-a").Label("foo", "").Priority(highPriority).Obj(),
1129-
st.MakePod().Name("p-b1").UID("p-b1").Node("node-b").Label("foo", "").Priority(lowPriority).Obj(),
1130-
st.MakePod().Name("p-x1").UID("p-x1").Node("node-x").Label("foo", "").Priority(highPriority).Obj(),
1131-
st.MakePod().Name("p-x2").UID("p-x2").Node("node-x").Label("foo", "").Priority(highPriority).Obj(),
1125+
st.MakePod().Name("p-a1").UID("p-a1").Namespace(v1.NamespaceDefault).Node("node-a").Label("foo", "").Priority(highPriority).Obj(),
1126+
st.MakePod().Name("p-a2").UID("p-a2").Namespace(v1.NamespaceDefault).Node("node-a").Label("foo", "").Priority(highPriority).Obj(),
1127+
st.MakePod().Name("p-b1").UID("p-b1").Namespace(v1.NamespaceDefault).Node("node-b").Label("foo", "").Priority(lowPriority).Obj(),
1128+
st.MakePod().Name("p-x1").UID("p-x1").Namespace(v1.NamespaceDefault).Node("node-x").Label("foo", "").Priority(highPriority).Obj(),
1129+
st.MakePod().Name("p-x2").UID("p-x2").Namespace(v1.NamespaceDefault).Node("node-x").Label("foo", "").Priority(highPriority).Obj(),
11321130
},
11331131
nodeNames: []string{"node-a/zone1", "node-b/zone1", "node-x/zone2"},
11341132
registerPlugin: st.RegisterPluginAsExtensions(podtopologyspread.Name, podtopologyspread.New, "PreFilter", "Filter"),
@@ -1137,11 +1135,11 @@ func TestPreempt(t *testing.T) {
11371135
},
11381136
{
11391137
name: "Scheduler extenders allow only node1, otherwise node3 would have been chosen",
1140-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
1138+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
11411139
pods: []*v1.Pod{
1142-
st.MakePod().Name("p1.1").UID("p1.1").Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1143-
st.MakePod().Name("p1.2").UID("p1.2").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1144-
st.MakePod().Name("p2.1").UID("p2.1").Node("node3").Priority(midPriority).Req(largeRes).Obj(),
1140+
st.MakePod().Name("p1.1").UID("p1.1").Namespace(v1.NamespaceDefault).Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1141+
st.MakePod().Name("p1.2").UID("p1.2").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1142+
st.MakePod().Name("p2.1").UID("p2.1").Namespace(v1.NamespaceDefault).Node("node3").Priority(midPriority).Req(largeRes).Obj(),
11451143
},
11461144
nodeNames: []string{"node1", "node2", "node3"},
11471145
extenders: []*st.FakeExtender{
@@ -1154,11 +1152,11 @@ func TestPreempt(t *testing.T) {
11541152
},
11551153
{
11561154
name: "Scheduler extenders do not allow any preemption",
1157-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
1155+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
11581156
pods: []*v1.Pod{
1159-
st.MakePod().Name("p1.1").UID("p1.1").Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1160-
st.MakePod().Name("p1.2").UID("p1.2").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1161-
st.MakePod().Name("p2.1").UID("p2.1").Node("node2").Priority(midPriority).Req(largeRes).Obj(),
1157+
st.MakePod().Name("p1.1").UID("p1.1").Namespace(v1.NamespaceDefault).Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1158+
st.MakePod().Name("p1.2").UID("p1.2").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1159+
st.MakePod().Name("p2.1").UID("p2.1").Namespace(v1.NamespaceDefault).Node("node2").Priority(midPriority).Req(largeRes).Obj(),
11621160
},
11631161
nodeNames: []string{"node1", "node2", "node3"},
11641162
extenders: []*st.FakeExtender{
@@ -1170,11 +1168,11 @@ func TestPreempt(t *testing.T) {
11701168
},
11711169
{
11721170
name: "One scheduler extender allows only node1, the other returns error but ignorable. Only node1 would be chosen",
1173-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
1171+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
11741172
pods: []*v1.Pod{
1175-
st.MakePod().Name("p1.1").UID("p1.1").Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1176-
st.MakePod().Name("p1.2").UID("p1.2").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1177-
st.MakePod().Name("p2.1").UID("p2.1").Node("node2").Priority(midPriority).Req(largeRes).Obj(),
1173+
st.MakePod().Name("p1.1").UID("p1.1").Namespace(v1.NamespaceDefault).Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1174+
st.MakePod().Name("p1.2").UID("p1.2").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1175+
st.MakePod().Name("p2.1").UID("p2.1").Namespace(v1.NamespaceDefault).Node("node2").Priority(midPriority).Req(largeRes).Obj(),
11781176
},
11791177
nodeNames: []string{"node1", "node2", "node3"},
11801178
extenders: []*st.FakeExtender{
@@ -1187,11 +1185,11 @@ func TestPreempt(t *testing.T) {
11871185
},
11881186
{
11891187
name: "One scheduler extender allows only node1, but it is not interested in given pod, otherwise node1 would have been chosen",
1190-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
1188+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptLowerPriority).Obj(),
11911189
pods: []*v1.Pod{
1192-
st.MakePod().Name("p1.1").UID("p1.1").Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1193-
st.MakePod().Name("p1.2").UID("p1.2").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1194-
st.MakePod().Name("p2.1").UID("p2.1").Node("node2").Priority(midPriority).Req(largeRes).Obj(),
1190+
st.MakePod().Name("p1.1").UID("p1.1").Namespace(v1.NamespaceDefault).Node("node1").Priority(midPriority).Req(smallRes).Obj(),
1191+
st.MakePod().Name("p1.2").UID("p1.2").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1192+
st.MakePod().Name("p2.1").UID("p2.1").Namespace(v1.NamespaceDefault).Node("node2").Priority(midPriority).Req(largeRes).Obj(),
11951193
},
11961194
nodeNames: []string{"node1", "node2"},
11971195
extenders: []*st.FakeExtender{
@@ -1205,12 +1203,12 @@ func TestPreempt(t *testing.T) {
12051203
},
12061204
{
12071205
name: "no preempting in pod",
1208-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptNever).Obj(),
1206+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(veryLargeRes).PreemptionPolicy(v1.PreemptNever).Obj(),
12091207
pods: []*v1.Pod{
1210-
st.MakePod().Name("p1.1").UID("p1.1").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1211-
st.MakePod().Name("p1.2").UID("p1.2").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1212-
st.MakePod().Name("p2.1").UID("p2.1").Node("node2").Priority(highPriority).Req(largeRes).Obj(),
1213-
st.MakePod().Name("p3.1").UID("p3.1").Node("node3").Priority(midPriority).Req(mediumRes).Obj(),
1208+
st.MakePod().Name("p1.1").UID("p1.1").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1209+
st.MakePod().Name("p1.2").UID("p1.2").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1210+
st.MakePod().Name("p2.1").UID("p2.1").Namespace(v1.NamespaceDefault).Node("node2").Priority(highPriority).Req(largeRes).Obj(),
1211+
st.MakePod().Name("p3.1").UID("p3.1").Namespace(v1.NamespaceDefault).Node("node3").Priority(midPriority).Req(mediumRes).Obj(),
12141212
},
12151213
nodeNames: []string{"node1", "node2", "node3"},
12161214
registerPlugin: st.RegisterPluginAsExtensions(noderesources.FitName, noderesources.NewFit, "Filter", "PreFilter"),
@@ -1219,12 +1217,12 @@ func TestPreempt(t *testing.T) {
12191217
},
12201218
{
12211219
name: "PreemptionPolicy is nil",
1222-
pod: st.MakePod().Name("p").UID("p").Priority(highPriority).Req(veryLargeRes).Obj(),
1220+
pod: st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(veryLargeRes).Obj(),
12231221
pods: []*v1.Pod{
1224-
st.MakePod().Name("p1.1").UID("p1.1").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1225-
st.MakePod().Name("p1.2").UID("p1.2").Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1226-
st.MakePod().Name("p2.1").UID("p2.1").Node("node2").Priority(highPriority).Req(largeRes).Obj(),
1227-
st.MakePod().Name("p3.1").UID("p3.1").Node("node3").Priority(midPriority).Req(mediumRes).Obj(),
1222+
st.MakePod().Name("p1.1").UID("p1.1").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1223+
st.MakePod().Name("p1.2").UID("p1.2").Namespace(v1.NamespaceDefault).Node("node1").Priority(lowPriority).Req(smallRes).Obj(),
1224+
st.MakePod().Name("p2.1").UID("p2.1").Namespace(v1.NamespaceDefault).Node("node2").Priority(highPriority).Req(largeRes).Obj(),
1225+
st.MakePod().Name("p3.1").UID("p3.1").Namespace(v1.NamespaceDefault).Node("node3").Priority(midPriority).Req(mediumRes).Obj(),
12281226
},
12291227
nodeNames: []string{"node1", "node2", "node3"},
12301228
registerPlugin: st.RegisterPluginAsExtensions(noderesources.FitName, noderesources.NewFit, "Filter", "PreFilter"),
@@ -1236,8 +1234,14 @@ func TestPreempt(t *testing.T) {
12361234
labelKeys := []string{"hostname", "zone", "region"}
12371235
for _, test := range tests {
12381236
t.Run(test.name, func(t *testing.T) {
1239-
apiObjs := mergeObjs(test.pod, test.pods)
1240-
client := clientsetfake.NewSimpleClientset(apiObjs...)
1237+
client := clientsetfake.NewSimpleClientset()
1238+
informerFactory := informers.NewSharedInformerFactory(client, 0)
1239+
podInformer := informerFactory.Core().V1().Pods().Informer()
1240+
podInformer.GetStore().Add(test.pod)
1241+
for i := range test.pods {
1242+
podInformer.GetStore().Add(test.pods[i])
1243+
}
1244+
12411245
deletedPodNames := make(sets.String)
12421246
client.PrependReactor("delete", "pods", func(action clienttesting.Action) (bool, runtime.Object, error) {
12431247
deletedPodNames.Insert(action.(clienttesting.DeleteAction).GetName())
@@ -1277,7 +1281,6 @@ func TestPreempt(t *testing.T) {
12771281
extenders = append(extenders, extender)
12781282
}
12791283

1280-
informerFactory := informers.NewSharedInformerFactory(client, 0)
12811284
fwk, err := st.NewFramework(
12821285
[]st.RegisterPluginFunc{
12831286
test.registerPlugin,
@@ -1332,7 +1335,6 @@ func TestPreempt(t *testing.T) {
13321335
}
13331336
}
13341337
test.pod.Status.NominatedNodeName = node
1335-
client.CoreV1().Pods(test.pod.Namespace).Update(context.TODO(), test.pod, metav1.UpdateOptions{})
13361338

13371339
// Manually set the deleted Pods' deletionTimestamp to non-nil.
13381340
for _, pod := range test.pods {

‎pkg/scheduler/util/utils.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,6 @@ func PatchPod(cs kubernetes.Interface, old *v1.Pod, new *v1.Pod) error {
142142
return err
143143
}
144144

145-
// GetUpdatedPod returns the latest version of <pod> from API server.
146-
func GetUpdatedPod(cs kubernetes.Interface, pod *v1.Pod) (*v1.Pod, error) {
147-
return cs.CoreV1().Pods(pod.Namespace).Get(context.TODO(), pod.Name, metav1.GetOptions{})
148-
}
149-
150145
// DeletePod deletes the given <pod> from API server
151146
func DeletePod(cs kubernetes.Interface, pod *v1.Pod) error {
152147
return cs.CoreV1().Pods(pod.Namespace).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{})

0 commit comments

Comments
 (0)