Skip to content

Commit 57ec7ca

Browse files
Thezone-1Thezone-1
authored
fix(scheduler): keep succeeded pods visible so completing gangs are not evicted (#2012)
Signed-off-by: Thezone-1 <somoprovobhattacharjee@gmail.com>
1 parent 3ce5dcf commit 57ec7ca

6 files changed

Lines changed: 392 additions & 6 deletions

File tree

‎.changes/unreleased/Fixed-20260728-181225.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: Fixed
2+
body: stalegangeviction no longer evicts remaining pods of a gang whose pods complete successfully
3+
time: 2026-07-28T18:12:25.0211683+05:30
4+
custom:
5+
Author: Thezone-1
6+
Issue: "1968"

‎pkg/scheduler/actions/stalegangeviction/stalegangeviction_test.go‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,201 @@ func TestStaleGangEviction(t *testing.T) {
709709
},
710710
},
711711
},
712+
{
713+
name: "Gang with a succeeded pod - no evict",
714+
topology: test_utils.TestTopologyBasic{
715+
Jobs: []*jobs_fake.TestJobBasic{
716+
{
717+
Name: "job-1",
718+
QueueName: "q-1",
719+
RootSubGroupSet: jobs_fake.DefaultSubGroup(3),
720+
Tasks: []*tasks_fake.TestTaskBasic{
721+
{
722+
Name: "job-1-0",
723+
State: pod_status.Succeeded,
724+
},
725+
{
726+
Name: "job-1-1",
727+
State: pod_status.Running,
728+
NodeName: "node-1",
729+
},
730+
{
731+
Name: "job-1-2",
732+
State: pod_status.Running,
733+
NodeName: "node-1",
734+
},
735+
},
736+
StaleDuration: pointer.Duration(61 * time.Second),
737+
},
738+
},
739+
Nodes: map[string]nodes_fake.TestNodeBasic{
740+
"node-1": {},
741+
},
742+
Queues: []test_utils.TestQueueBasic{
743+
{
744+
Name: "q-1",
745+
ParentQueue: "d-1",
746+
},
747+
},
748+
Departments: []test_utils.TestDepartmentBasic{
749+
{
750+
Name: "d-1",
751+
},
752+
},
753+
TaskExpectedResults: map[string]test_utils.TestExpectedResultBasic{
754+
"job-1-0": {
755+
Status: pod_status.Succeeded,
756+
},
757+
"job-1-1": {
758+
NodeName: "node-1",
759+
Status: pod_status.Running,
760+
},
761+
"job-1-2": {
762+
NodeName: "node-1",
763+
Status: pod_status.Running,
764+
},
765+
},
766+
Mocks: &test_utils.TestMock{
767+
CacheRequirements: &test_utils.CacheMocking{
768+
NumberOfCacheBinds: 0,
769+
NumberOfCacheEvictions: 0,
770+
NumberOfPipelineActions: 0,
771+
},
772+
},
773+
},
774+
},
775+
{
776+
name: "Gang with only one pod left running - no evict",
777+
topology: test_utils.TestTopologyBasic{
778+
Jobs: []*jobs_fake.TestJobBasic{
779+
{
780+
Name: "job-1",
781+
QueueName: "q-1",
782+
RootSubGroupSet: jobs_fake.DefaultSubGroup(3),
783+
Tasks: []*tasks_fake.TestTaskBasic{
784+
{
785+
Name: "job-1-0",
786+
State: pod_status.Succeeded,
787+
},
788+
{
789+
Name: "job-1-1",
790+
State: pod_status.Succeeded,
791+
},
792+
{
793+
Name: "job-1-2",
794+
State: pod_status.Running,
795+
NodeName: "node-1",
796+
},
797+
},
798+
StaleDuration: pointer.Duration(61 * time.Second),
799+
},
800+
},
801+
Nodes: map[string]nodes_fake.TestNodeBasic{
802+
"node-1": {},
803+
},
804+
Queues: []test_utils.TestQueueBasic{
805+
{
806+
Name: "q-1",
807+
ParentQueue: "d-1",
808+
},
809+
},
810+
Departments: []test_utils.TestDepartmentBasic{
811+
{
812+
Name: "d-1",
813+
},
814+
},
815+
TaskExpectedResults: map[string]test_utils.TestExpectedResultBasic{
816+
"job-1-0": {
817+
Status: pod_status.Succeeded,
818+
},
819+
"job-1-1": {
820+
Status: pod_status.Succeeded,
821+
},
822+
"job-1-2": {
823+
NodeName: "node-1",
824+
Status: pod_status.Running,
825+
},
826+
},
827+
Mocks: &test_utils.TestMock{
828+
CacheRequirements: &test_utils.CacheMocking{
829+
NumberOfCacheBinds: 0,
830+
NumberOfCacheEvictions: 0,
831+
NumberOfPipelineActions: 0,
832+
},
833+
},
834+
},
835+
},
836+
{
837+
name: "Gang with a succeeded pod in one sub group - no evict",
838+
topology: test_utils.TestTopologyBasic{
839+
Jobs: []*jobs_fake.TestJobBasic{
840+
{
841+
Name: "job-1",
842+
QueueName: "q-1",
843+
RootSubGroupSet: func() *subgroup_info.SubGroupSet {
844+
root := subgroup_info.NewSubGroupSet(subgroup_info.RootSubGroupSetName, nil)
845+
root.AddPodSet(subgroup_info.NewPodSet("sub-group-0", 2, nil))
846+
root.AddPodSet(subgroup_info.NewPodSet("sub-group-1", 1, nil))
847+
return root
848+
}(),
849+
Tasks: []*tasks_fake.TestTaskBasic{
850+
{
851+
Name: "job-1-0",
852+
SubGroupName: "sub-group-0",
853+
State: pod_status.Succeeded,
854+
},
855+
{
856+
Name: "job-1-1",
857+
SubGroupName: "sub-group-0",
858+
State: pod_status.Running,
859+
NodeName: "node-1",
860+
},
861+
{
862+
Name: "job-1-2",
863+
SubGroupName: "sub-group-1",
864+
State: pod_status.Running,
865+
NodeName: "node-1",
866+
},
867+
},
868+
StaleDuration: pointer.Duration(61 * time.Second),
869+
},
870+
},
871+
Nodes: map[string]nodes_fake.TestNodeBasic{
872+
"node-1": {},
873+
},
874+
Queues: []test_utils.TestQueueBasic{
875+
{
876+
Name: "q-1",
877+
ParentQueue: "d-1",
878+
},
879+
},
880+
Departments: []test_utils.TestDepartmentBasic{
881+
{
882+
Name: "d-1",
883+
},
884+
},
885+
TaskExpectedResults: map[string]test_utils.TestExpectedResultBasic{
886+
"job-1-0": {
887+
Status: pod_status.Succeeded,
888+
},
889+
"job-1-1": {
890+
NodeName: "node-1",
891+
Status: pod_status.Running,
892+
},
893+
"job-1-2": {
894+
NodeName: "node-1",
895+
Status: pod_status.Running,
896+
},
897+
},
898+
Mocks: &test_utils.TestMock{
899+
CacheRequirements: &test_utils.CacheMocking{
900+
NumberOfCacheBinds: 0,
901+
NumberOfCacheEvictions: 0,
902+
NumberOfPipelineActions: 0,
903+
},
904+
},
905+
},
906+
},
712907
} {
713908
t.Run(test.name, func(t *testing.T) {
714909
t.Logf("Running test number: %v, test name: %v,", i, test.name)

‎pkg/scheduler/cache/cache.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,13 @@ var terminalPodPhases = []v1.PodPhase{
8383
v1.PodFailed,
8484
}
8585

86-
func filterTerminalPods(options *metav1.ListOptions) {
87-
selectors := make([]string, 0, len(terminalPodPhases))
88-
for _, phase := range terminalPodPhases {
86+
var watchFilteredPodPhases = []v1.PodPhase{
87+
v1.PodFailed,
88+
}
89+
90+
func filterFailedPods(options *metav1.ListOptions) {
91+
selectors := make([]string, 0, len(watchFilteredPodPhases))
92+
for _, phase := range watchFilteredPodPhases {
8993
selectors = append(selectors, fmt.Sprintf("status.phase!=%s", phase))
9094
}
9195
selector := strings.Join(selectors, ",")
@@ -103,7 +107,7 @@ func registerSchedulerPodInformer(informerFactory informers.SharedInformerFactor
103107
metav1.NamespaceAll,
104108
resyncPeriod,
105109
k8scache.Indexers{k8scache.NamespaceIndex: k8scache.MetaNamespaceIndexFunc},
106-
filterTerminalPods,
110+
filterFailedPods,
107111
)
108112
})
109113
}

‎pkg/scheduler/cache/cache_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestCache(t *testing.T) {
5858
var _ = Describe("Cache", func() {
5959
Describe("New", func() {
6060
Context("Pod informer filtering", func() {
61-
It("should filter terminal pods without filtering pods by scheduler name", func() {
61+
It("should filter failed pods while keeping succeeded pods, without filtering by scheduler name", func() {
6262
kubeClient := fake.NewSimpleClientset()
6363
cache := New(&SchedulerCacheParams{
6464
KubeClient: kubeClient,
@@ -95,8 +95,8 @@ var _ = Describe("Cache", func() {
9595

9696
Expect(podSelectors).NotTo(BeEmpty())
9797
for _, selector := range podSelectors {
98-
Expect(selector).To(ContainSubstring("status.phase!=Succeeded"))
9998
Expect(selector).To(ContainSubstring("status.phase!=Failed"))
99+
Expect(selector).NotTo(ContainSubstring("status.phase!=Succeeded"))
100100
Expect(selector).NotTo(ContainSubstring("spec.schedulerName"))
101101
}
102102
Expect(nonPodSelectors).To(BeEmpty())

‎pkg/scheduler/cache/pod_transform.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ package cache
55

66
import (
77
v1 "k8s.io/api/core/v1"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
89
"k8s.io/client-go/tools/cache"
10+
11+
commonconstants "github.com/kai-scheduler/KAI-scheduler/pkg/common/constants"
912
)
1013

1114
func setSchedulerPodTransform(informer cache.SharedIndexInformer) error {
@@ -18,6 +21,10 @@ func compactSchedulerPod(obj any) (any, error) {
1821
return obj, nil
1922
}
2023

24+
if pod.Status.Phase == v1.PodSucceeded {
25+
return compactSucceededPod(pod), nil
26+
}
27+
2128
compact := pod.DeepCopy()
2229
compact.ManagedFields = nil
2330
compact.Spec.Containers = compactContainers(compact.Spec.Containers)
@@ -26,6 +33,28 @@ func compactSchedulerPod(obj any) (any, error) {
2633
return compact, nil
2734
}
2835

36+
func compactSucceededPod(pod *v1.Pod) *v1.Pod {
37+
compact := &v1.Pod{
38+
ObjectMeta: metav1.ObjectMeta{
39+
Name: pod.Name,
40+
Namespace: pod.Namespace,
41+
UID: pod.UID,
42+
},
43+
Status: v1.PodStatus{
44+
Phase: pod.Status.Phase,
45+
},
46+
}
47+
48+
if podGroup, found := pod.Annotations[commonconstants.PodGroupAnnotationForPod]; found {
49+
compact.Annotations = map[string]string{commonconstants.PodGroupAnnotationForPod: podGroup}
50+
}
51+
if subGroup, found := pod.Labels[commonconstants.SubGroupLabelKey]; found {
52+
compact.Labels = map[string]string{commonconstants.SubGroupLabelKey: subGroup}
53+
}
54+
55+
return compact
56+
}
57+
2958
func compactContainers(containers []v1.Container) []v1.Container {
3059
compact := make([]v1.Container, 0, len(containers))
3160
for _, container := range containers {

0 commit comments

Comments
 (0)