Skip to content

Commit 022046f

Browse files
committed
PR fix
Signed-off-by: gshaibi <gshaibi@nvidia.com>
1 parent 9ae0efc commit 022046f

6 files changed

Lines changed: 165 additions & 104 deletions

File tree

pkg/admission/webhook/v1alpha2/podhooks/pod_resize_validator.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
v2 "github.com/kai-scheduler/KAI-scheduler/pkg/apis/scheduling/v2"
1717
v2alpha2 "github.com/kai-scheduler/KAI-scheduler/pkg/apis/scheduling/v2alpha2"
1818
commonconstants "github.com/kai-scheduler/KAI-scheduler/pkg/common/constants"
19+
commonpod "github.com/kai-scheduler/KAI-scheduler/pkg/common/pod"
1920
commonpodgroup "github.com/kai-scheduler/KAI-scheduler/pkg/common/podgroup"
2021
)
2122

@@ -67,6 +68,10 @@ func (v *PodResizeValidator) validateResize(ctx context.Context, oldPod, newPod
6768
return nil
6869
}
6970

71+
if !commonpod.IsAllocated(oldPod) {
72+
return nil
73+
}
74+
7075
pgName, ok := oldPod.Annotations[commonconstants.PodGroupAnnotationForPod]
7176
if !ok || pgName == "" {
7277
return nil

pkg/admission/webhook/v1alpha2/podhooks/pod_resize_validator_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func podWithRequests(namespace, name, pgName, schedulerName, cpu, memory string)
7070
commonconstants.PodGroupAnnotationForPod: pgName,
7171
},
7272
},
73+
Status: corev1.PodStatus{Phase: corev1.PodRunning},
7374
Spec: corev1.PodSpec{
7475
SchedulerName: schedulerName,
7576
Containers: []corev1.Container{
@@ -407,6 +408,7 @@ func TestPodResizeValidator_SidecarUpsize_DeniedWhenLimitExceeded(t *testing.T)
407408
commonconstants.PodGroupAnnotationForPod: "pg",
408409
},
409410
},
411+
Status: corev1.PodStatus{Phase: corev1.PodRunning},
410412
Spec: corev1.PodSpec{
411413
SchedulerName: testSchedulerName,
412414
InitContainers: []corev1.Container{
@@ -476,6 +478,7 @@ func TestPodResizeValidator_Redistribution_AllowedAtLimit(t *testing.T) {
476478
commonconstants.PodGroupAnnotationForPod: "pg",
477479
},
478480
},
481+
Status: corev1.PodStatus{Phase: corev1.PodRunning},
479482
Spec: corev1.PodSpec{
480483
SchedulerName: testSchedulerName,
481484
Containers: []corev1.Container{
@@ -529,3 +532,37 @@ func TestPodResizeValidator_InitPeakDominates_NoDelta(t *testing.T) {
529532
_, err := v.ValidateUpdate(context.Background(), oldPod, newPod)
530533
assert.NoError(t, err, "queue charge is unchanged, so the resize must be admitted at the limit")
531534
}
535+
536+
func TestPodResizeValidator_PendingUnscheduled_NotValidated(t *testing.T) {
537+
scheme := buildScheme()
538+
// Queue already at its limit — a checked upsize would be denied.
539+
queue := newQueue("q", 2000, -1, 0, "2", "0")
540+
pg := newPodGroup("pg", "ns", "q")
541+
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(queue, pg).Build()
542+
v := NewPodResizeValidator(c, testSchedulerName, true, false)
543+
544+
oldPod := podWithRequests("ns", "p", "pg", testSchedulerName, "1", "0")
545+
oldPod.Status.Phase = corev1.PodPending
546+
newPod := podWithRequests("ns", "p", "pg", testSchedulerName, "5", "0")
547+
548+
_, err := v.ValidateUpdate(context.Background(), oldPod, newPod)
549+
assert.NoError(t, err, "unscheduled pending pod resize must not be quota-checked")
550+
}
551+
552+
func TestPodResizeValidator_PendingScheduled_StillValidated(t *testing.T) {
553+
scheme := buildScheme()
554+
queue := newQueue("q", 2000, -1, 0, "2", "0")
555+
pg := newPodGroup("pg", "ns", "q")
556+
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(queue, pg).Build()
557+
v := NewPodResizeValidator(c, testSchedulerName, true, false)
558+
559+
oldPod := podWithRequests("ns", "p", "pg", testSchedulerName, "1", "0")
560+
oldPod.Status.Phase = corev1.PodPending
561+
oldPod.Status.Conditions = []corev1.PodCondition{
562+
{Type: corev1.PodScheduled, Status: corev1.ConditionTrue},
563+
}
564+
newPod := podWithRequests("ns", "p", "pg", testSchedulerName, "5", "0")
565+
566+
_, err := v.ValidateUpdate(context.Background(), oldPod, newPod)
567+
assert.Error(t, err, "scheduled pending pod holds capacity and must be checked")
568+
}

pkg/common/pod/status.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2026 NVIDIA CORPORATION
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package pod
5+
6+
import (
7+
v1 "k8s.io/api/core/v1"
8+
)
9+
10+
func IsAllocated(pod *v1.Pod) bool {
11+
if pod.Status.Phase == v1.PodPending {
12+
return isScheduled(pod)
13+
}
14+
return pod.Status.Phase == v1.PodRunning
15+
}
16+
17+
func isScheduled(pod *v1.Pod) bool {
18+
for _, condition := range pod.Status.Conditions {
19+
if condition.Type == v1.PodScheduled {
20+
return condition.Status == v1.ConditionTrue
21+
}
22+
}
23+
return false
24+
}

pkg/common/pod/status_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright 2026 NVIDIA CORPORATION
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package pod
5+
6+
import (
7+
"testing"
8+
9+
v1 "k8s.io/api/core/v1"
10+
)
11+
12+
func TestIsAllocated(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
pod *v1.Pod
16+
expectedResult bool
17+
}{
18+
{
19+
"pending pod",
20+
&v1.Pod{
21+
Status: v1.PodStatus{
22+
Phase: v1.PodPending,
23+
},
24+
},
25+
false,
26+
},
27+
{
28+
"pending scheduled pod",
29+
&v1.Pod{
30+
Status: v1.PodStatus{
31+
Phase: v1.PodPending,
32+
Conditions: []v1.PodCondition{
33+
{
34+
Type: v1.PodScheduled,
35+
Status: v1.ConditionTrue,
36+
},
37+
},
38+
},
39+
},
40+
true,
41+
},
42+
{
43+
"running pod",
44+
&v1.Pod{
45+
Status: v1.PodStatus{
46+
Phase: v1.PodRunning,
47+
Conditions: []v1.PodCondition{
48+
{
49+
Type: v1.PodScheduled,
50+
Status: v1.ConditionTrue,
51+
},
52+
},
53+
},
54+
},
55+
true,
56+
},
57+
{
58+
"succeeded pod",
59+
&v1.Pod{
60+
Status: v1.PodStatus{
61+
Phase: v1.PodSucceeded,
62+
Conditions: []v1.PodCondition{
63+
{
64+
Type: v1.PodScheduled,
65+
Status: v1.ConditionTrue,
66+
},
67+
},
68+
},
69+
},
70+
false,
71+
},
72+
{
73+
"failed pod",
74+
&v1.Pod{
75+
Status: v1.PodStatus{
76+
Phase: v1.PodFailed,
77+
Conditions: []v1.PodCondition{
78+
{
79+
Type: v1.PodScheduled,
80+
Status: v1.ConditionTrue,
81+
},
82+
},
83+
},
84+
},
85+
false,
86+
},
87+
}
88+
for _, tt := range tests {
89+
t.Run(tt.name, func(t *testing.T) {
90+
result := IsAllocated(tt.pod)
91+
if tt.expectedResult != result {
92+
t.Errorf("IsAllocated() failed. test name: %s, expected: %v, actual: %v",
93+
tt.name, tt.expectedResult, result)
94+
}
95+
})
96+
}
97+
}

pkg/podgroupcontroller/controllers/metadata/pod.go

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"sigs.k8s.io/controller-runtime/pkg/client"
1414
"sigs.k8s.io/controller-runtime/pkg/log"
1515

16+
commonpod "github.com/kai-scheduler/KAI-scheduler/pkg/common/pod"
1617
commonresources "github.com/kai-scheduler/KAI-scheduler/pkg/common/resources"
1718
"github.com/kai-scheduler/KAI-scheduler/pkg/podgroupcontroller/controllers/resources"
1819
)
@@ -50,7 +51,7 @@ func GetPodMetadata(
5051
}
5152

5253
allocatedResources := v1.ResourceList{}
53-
if isAllocatedPod(pod) {
54+
if commonpod.IsAllocated(pod) {
5455
allocatedResources, err = calculatedAllocatedResources(ctx, pod, kubeClient, draClaims)
5556
if err != nil {
5657
return nil, err
@@ -71,22 +72,6 @@ func isTerminalPod(pod *v1.Pod) bool {
7172
return pod.Status.Phase == v1.PodSucceeded || pod.Status.Phase == v1.PodFailed
7273
}
7374

74-
func isAllocatedPod(pod *v1.Pod) bool {
75-
if pod.Status.Phase == v1.PodPending {
76-
return isPodScheduled(pod)
77-
}
78-
return pod.Status.Phase == v1.PodRunning
79-
}
80-
81-
func isPodScheduled(pod *v1.Pod) bool {
82-
for _, condition := range pod.Status.Conditions {
83-
if condition.Type == v1.PodScheduled {
84-
return condition.Status == v1.ConditionTrue
85-
}
86-
}
87-
return false
88-
}
89-
9075
func calculatedAllocatedResources(
9176
ctx context.Context, pod *v1.Pod, kubeClient client.Client, draClaims []*resourceapi.ResourceClaim,
9277
) (v1.ResourceList, error) {

pkg/podgroupcontroller/controllers/metadata/pod_test.go

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -18,93 +18,6 @@ import (
1818
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1919
)
2020

21-
func TestIsPodAllocated(t *testing.T) {
22-
tests := []struct {
23-
name string
24-
pod *v1.Pod
25-
expectedResult bool
26-
}{
27-
{
28-
"pending pod",
29-
&v1.Pod{
30-
Status: v1.PodStatus{
31-
Phase: v1.PodPending,
32-
},
33-
},
34-
false,
35-
},
36-
{
37-
"pending scheduled pod",
38-
&v1.Pod{
39-
Status: v1.PodStatus{
40-
Phase: v1.PodPending,
41-
Conditions: []v1.PodCondition{
42-
{
43-
Type: v1.PodScheduled,
44-
Status: v1.ConditionTrue,
45-
},
46-
},
47-
},
48-
},
49-
true,
50-
},
51-
{
52-
"running pod",
53-
&v1.Pod{
54-
Status: v1.PodStatus{
55-
Phase: v1.PodRunning,
56-
Conditions: []v1.PodCondition{
57-
{
58-
Type: v1.PodScheduled,
59-
Status: v1.ConditionTrue,
60-
},
61-
},
62-
},
63-
},
64-
true,
65-
},
66-
{
67-
"succeeded pod",
68-
&v1.Pod{
69-
Status: v1.PodStatus{
70-
Phase: v1.PodSucceeded,
71-
Conditions: []v1.PodCondition{
72-
{
73-
Type: v1.PodScheduled,
74-
Status: v1.ConditionTrue,
75-
},
76-
},
77-
},
78-
},
79-
false,
80-
},
81-
{
82-
"failed pod",
83-
&v1.Pod{
84-
Status: v1.PodStatus{
85-
Phase: v1.PodFailed,
86-
Conditions: []v1.PodCondition{
87-
{
88-
Type: v1.PodScheduled,
89-
Status: v1.ConditionTrue,
90-
},
91-
},
92-
},
93-
},
94-
false,
95-
},
96-
}
97-
for _, tt := range tests {
98-
t.Run(tt.name, func(t *testing.T) {
99-
result := isAllocatedPod(tt.pod)
100-
if tt.expectedResult != result {
101-
t.Errorf("isAllocatedPod() failed. test name: %s, expected: %v, actual: %v",
102-
tt.name, tt.expectedResult, result)
103-
}
104-
})
105-
}
106-
}
107-
10821
func TestIsTerminalPod(t *testing.T) {
10922
tests := []struct {
11023
name string

0 commit comments

Comments
 (0)