From 3b4f0be6e30e63b2c0d496bcc5d0691ae1e7270a Mon Sep 17 00:00:00 2001 From: Bartosz Date: Thu, 20 Nov 2025 09:26:14 +0000 Subject: [PATCH] Check NominatedNodeName to decide if a pod is scheduled --- .../framework/plugins/interpodaffinity/plugin.go | 4 +++- .../plugins/interpodaffinity/plugin_test.go | 12 ++++++++++++ .../framework/plugins/noderesources/fit_test.go | 6 ++++++ .../plugins/podtopologyspread/plugin.go | 9 +++++++-- .../plugins/podtopologyspread/plugin_test.go | 16 ++++++++++++++++ 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go b/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go index 05c06f9f2b1..7b521680af5 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go @@ -181,7 +181,9 @@ func (pl *InterPodAffinity) isSchedulableAfterPodChange(logger klog.Logger, pod "pod", klog.KObj(pod)) return fwk.Queue, nil } - if (modifiedPod != nil && modifiedPod.Spec.NodeName == "") || (originalPod != nil && originalPod.Spec.NodeName == "") { + // We don't need to check cases where NodeName or NominatedNodeName changes on a pod because in those cases Add/Delete event is fired + if (modifiedPod != nil && modifiedPod.Spec.NodeName == "" && modifiedPod.Status.NominatedNodeName == "") || + (originalPod != nil && originalPod.Spec.NodeName == "" && originalPod.Status.NominatedNodeName == "") { logger.V(5).Info("the added/updated/deleted pod is unscheduled, so it doesn't make the target pod schedulable", "pod", klog.KObj(pod), "originalPod", klog.KObj(originalPod), "modifiedPod", klog.KObj(modifiedPod)) return fwk.QueueSkip, nil diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/plugin_test.go b/pkg/scheduler/framework/plugins/interpodaffinity/plugin_test.go index 7fc05bc1466..fb2faf75789 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/plugin_test.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/plugin_test.go @@ -154,6 +154,18 @@ func Test_isSchedulableAfterPodChange(t *testing.T) { oldPod: st.MakePod().Node("fake-node").PodAntiAffinityIn("service", "region", []string{"securityscan", "value2"}, st.PodAntiAffinityWithRequiredReq).Label("service", "foo").Obj(), expectedHint: fwk.QueueSkip, }, + { + name: "delete a nominated pod which doesn't match pod's anti-affinity", + pod: st.MakePod().Name("p").PodAntiAffinityIn("service", "region", []string{"securityscan", "value2"}, st.PodAntiAffinityWithRequiredReq).Obj(), + oldPod: st.MakePod().NominatedNodeName("fake-node").Label("aaa", "a").Obj(), + expectedHint: fwk.QueueSkip, + }, + { + name: "delete a nominated pod which matches pod's anti-affinity", + pod: st.MakePod().Name("p").PodAntiAffinityIn("service", "region", []string{"securityscan", "value2"}, st.PodAntiAffinityWithRequiredReq).Obj(), + oldPod: st.MakePod().NominatedNodeName("fake-node").Label("service", "securityscan").Obj(), + expectedHint: fwk.Queue, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/scheduler/framework/plugins/noderesources/fit_test.go b/pkg/scheduler/framework/plugins/noderesources/fit_test.go index 9c7888093a3..1d5f269592b 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit_test.go @@ -1573,6 +1573,12 @@ func Test_isSchedulableAfterPodChange(t *testing.T) { enableInPlacePodVerticalScaling: true, expectedHint: fwk.QueueSkip, }, + "queue-on-nominated-pod-deleted": { + pod: st.MakePod().Name("pod1").UID("pod1").Req(map[v1.ResourceName]string{v1.ResourceCPU: "1"}).UID("uid0").Obj(), + oldObj: st.MakePod().Name("pod2").UID("pod2").Req(map[v1.ResourceName]string{v1.ResourceCPU: "2"}).NominatedNodeName("fake").UID("uid1").Obj(), + enableInPlacePodVerticalScaling: true, + expectedHint: fwk.Queue, + }, "skip-queue-on-disable-inplace-pod-vertical-scaling": { pod: st.MakePod().Name("pod1").Req(map[v1.ResourceName]string{v1.ResourceCPU: "1"}).Obj(), oldObj: st.MakePod().Name("pod2").Req(map[v1.ResourceName]string{v1.ResourceCPU: "2"}).Node("fake").Obj(), diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go index 18e7be733d5..af93f64c68d 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go @@ -184,8 +184,13 @@ func (pl *PodTopologySpread) EventsToRegister(_ context.Context) ([]fwk.ClusterE // involvedInTopologySpreading returns true if the incomingPod is involved in the topology spreading of podWithSpreading. func involvedInTopologySpreading(incomingPod, podWithSpreading *v1.Pod) bool { - return incomingPod.UID == podWithSpreading.UID || - (incomingPod.Spec.NodeName != "" && incomingPod.Namespace == podWithSpreading.Namespace) + if incomingPod.UID == podWithSpreading.UID { + return true + } + if incomingPod.Spec.NodeName == "" && incomingPod.Status.NominatedNodeName == "" { + return false + } + return incomingPod.Namespace == podWithSpreading.Namespace } // hasConstraintWithNodeTaintsPolicyHonor returns true if any constraint has `NodeTaintsPolicy: Honor`. diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go index 491bb49444e..6d9f5056fbe 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go @@ -236,6 +236,22 @@ func Test_isSchedulableAfterPodChange(t *testing.T) { oldPod: st.MakePod().UID("p2").Node("fake-node").Label("foo", "").Obj(), expectedHint: fwk.Queue, }, + { + name: "delete nominated pod with labels that don't match topologySpreadConstraints selector", + pod: st.MakePod().UID("p").Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", v1.DoNotSchedule, fooSelector, nil, nil, nil, nil). + Obj(), + oldPod: st.MakePod().UID("p2").NominatedNodeName("fake-node").Label("bar", "").Obj(), + expectedHint: fwk.QueueSkip, + }, + { + name: "delete nominated pod with labels that match topologySpreadConstraints selector", + pod: st.MakePod().UID("p").Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", v1.DoNotSchedule, fooSelector, nil, nil, nil, nil). + Obj(), + oldPod: st.MakePod().UID("p2").NominatedNodeName("fake-node").Label("foo", "").Obj(), + expectedHint: fwk.Queue, + }, { name: "delete pod with labels that don't match topologySpreadConstraints selector", pod: st.MakePod().UID("p").Name("p").Label("foo", "").