enhance unit tests

Signed-off-by: helayoty <heelayot@microsoft.com>
This commit is contained in:
helayoty
2026-03-17 18:42:26 +00:00
parent b728516811
commit 88cccd9d7e
6 changed files with 174 additions and 71 deletions

View File

@@ -412,6 +412,8 @@ func (sched *Scheduler) podGroupPodSchedulingAlgorithm(ctx context.Context, sche
// If the preemption is required for this pod group, all pods are moved back to the scheduling queue
// and require the next pod group scheduling cycle to verify the preemption outcome.
func (sched *Scheduler) submitPodGroupAlgorithmResult(ctx context.Context, schedFwk framework.Framework, podGroupInfo *framework.QueuedPodGroupInfo, podGroupResult podGroupAlgorithmResult, start time.Time) {
logger := klog.FromContext(ctx)
var scheduledPods, unschedulablePods int
for i, pInfo := range podGroupInfo.QueuedPodInfos {
var podResult algorithmResult
@@ -490,15 +492,6 @@ func (sched *Scheduler) submitPodGroupAlgorithmResult(ctx context.Context, sched
unschedulablePods++
}
}
logger := klog.FromContext(ctx)
// If the PodGroup was already successfully scheduled, don't regress the
// condition back to False on a subsequent cycle for extra pods.
alreadyScheduled := false
if pg, err := sched.podGroupLister.PodGroups(podGroupInfo.Namespace).Get(podGroupInfo.Name); err == nil {
existing := apimeta.FindStatusCondition(pg.Status.Conditions, schedulingapi.PodGroupScheduled)
alreadyScheduled = existing != nil && existing.Status == metav1.ConditionTrue
}
var condition *metav1.Condition
switch {
@@ -513,13 +506,11 @@ func (sched *Scheduler) submitPodGroupAlgorithmResult(ctx context.Context, sched
metrics.PodGroupScheduled(schedFwk.ProfileName(), metrics.SinceInSeconds(start))
case podGroupResult.status.IsRejected():
if !alreadyScheduled {
condition = &metav1.Condition{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionFalse,
Reason: schedulingapi.PodGroupReasonUnschedulable,
Message: podGroupResult.status.Message(),
}
condition = &metav1.Condition{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionFalse,
Reason: schedulingapi.PodGroupReasonUnschedulable,
Message: podGroupResult.status.Message(),
}
if podGroupResult.waitingOnPreemption {
logger.V(2).Info("Pod group is waiting for preemption", "podGroup", klog.KObj(podGroupInfo), "unschedulablePods", unschedulablePods)
@@ -530,20 +521,16 @@ func (sched *Scheduler) submitPodGroupAlgorithmResult(ctx context.Context, sched
}
default:
if !alreadyScheduled {
condition = &metav1.Condition{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionFalse,
Reason: schedulingapi.PodGroupReasonSchedulerError,
Message: podGroupResult.status.AsError().Error(),
}
condition = &metav1.Condition{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionFalse,
Reason: schedulingapi.PodGroupReasonSchedulerError,
Message: podGroupResult.status.AsError().Error(),
}
utilruntime.HandleErrorWithContext(ctx, podGroupResult.status.AsError(), "Error scheduling pod group", "podGroup", klog.KObj(podGroupInfo), "errorPods", len(podGroupInfo.QueuedPodInfos))
metrics.PodGroupScheduleError(schedFwk.ProfileName(), metrics.SinceInSeconds(start))
}
if condition != nil {
sched.updatePodGroupCondition(ctx, podGroupInfo, condition)
}
sched.updatePodGroupCondition(ctx, podGroupInfo, condition)
}
// updatePodGroupCondition patches the given condition on a PodGroup.
@@ -551,6 +538,8 @@ func (sched *Scheduler) updatePodGroupCondition(ctx context.Context,
podGroupInfo *framework.QueuedPodGroupInfo, condition *metav1.Condition) {
logger := klog.FromContext(ctx)
// If the PodGroup was already successfully scheduled, don't regress the
// condition back to False on a subsequent cycle for extra pods.
pg, err := sched.podGroupLister.PodGroups(podGroupInfo.Namespace).Get(podGroupInfo.Name)
if err != nil {
utilruntime.HandleErrorWithLogger(logger, err, "Failed to get PodGroup for status update", "podGroup", klog.KObj(podGroupInfo))

View File

@@ -347,6 +347,9 @@ func TestPodGroupCycle_UpdateSnapshotError(t *testing.T) {
}
_, ctx := ktesting.NewTestContext(t)
ctx, cancel := context.WithCancel(ctx)
defer cancel()
registry := frameworkruntime.Registry{
queuesort.Name: queuesort.New,
defaultbinder.Name: defaultbinder.New,
@@ -380,9 +383,9 @@ func TestPodGroupCycle_UpdateSnapshotError(t *testing.T) {
client := clientsetfake.NewClientset(testPodGroup)
informerFactory := informers.NewSharedInformerFactory(client, 0)
if err := informerFactory.Scheduling().V1alpha2().PodGroups().Informer().GetStore().Add(testPodGroup); err != nil {
t.Fatalf("Failed to add PodGroup to informer store: %v", err)
}
podGroupLister := informerFactory.Scheduling().V1alpha2().PodGroups().Lister()
informerFactory.Start(ctx.Done())
informerFactory.WaitForCacheSync(ctx.Done())
var failureHandlerCalled bool
sched := &Scheduler{
@@ -390,7 +393,7 @@ func TestPodGroupCycle_UpdateSnapshotError(t *testing.T) {
SchedulingQueue: internalqueue.NewTestQueue(ctx, nil),
Cache: cache,
client: client,
podGroupLister: informerFactory.Scheduling().V1alpha2().PodGroups().Lister(),
podGroupLister: podGroupLister,
FailureHandler: func(ctx context.Context, fwk framework.Framework, p *framework.QueuedPodInfo, status *fwk.Status, ni *fwk.NominatingInfo, start time.Time) {
failureHandlerCalled = true
if updateSnapshotErr.Error() != status.AsError().Error() {
@@ -1243,14 +1246,14 @@ func TestSubmitPodGroupAlgorithmResult(t *testing.T) {
{
name: "Error for one pod",
algorithmResult: podGroupAlgorithmResult{
status: fwk.NewStatus(fwk.Error),
status: fwk.NewStatus(fwk.Error, "plugin returned error"),
podResults: []algorithmResult{{
scheduleResult: ScheduleResult{SuggestedHost: "node1"},
status: nil,
permitStatus: nil,
}, {
scheduleResult: ScheduleResult{SuggestedHost: "", nominatingInfo: clearNominatedNode},
status: fwk.NewStatus(fwk.Error),
status: fwk.NewStatus(fwk.Error, "plugin returned error"),
}},
},
expectBound: sets.New[string](),
@@ -1259,13 +1262,13 @@ func TestSubmitPodGroupAlgorithmResult(t *testing.T) {
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionFalse,
Reason: schedulingapi.PodGroupReasonSchedulerError,
Message: "",
Message: "plugin returned error",
},
},
{
name: "Error for one pod while waiting on preemption",
algorithmResult: podGroupAlgorithmResult{
status: fwk.NewStatus(fwk.Error),
status: fwk.NewStatus(fwk.Error, "internal failure"),
podResults: []algorithmResult{{
scheduleResult: ScheduleResult{
SuggestedHost: "node1",
@@ -1276,7 +1279,7 @@ func TestSubmitPodGroupAlgorithmResult(t *testing.T) {
permitStatus: nil,
}, {
scheduleResult: ScheduleResult{SuggestedHost: "", nominatingInfo: clearNominatedNode},
status: fwk.NewStatus(fwk.Error),
status: fwk.NewStatus(fwk.Error, "internal failure"),
}},
},
expectBound: sets.New[string](),
@@ -1285,7 +1288,44 @@ func TestSubmitPodGroupAlgorithmResult(t *testing.T) {
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionFalse,
Reason: schedulingapi.PodGroupReasonSchedulerError,
Message: "",
Message: "internal failure",
},
},
{
name: "Already Scheduled, successful cycle keeps condition",
existingPodGroup: &schedulingv1alpha2.PodGroup{
ObjectMeta: metav1.ObjectMeta{Name: "pg", Namespace: "default"},
Status: schedulingv1alpha2.PodGroupStatus{
Conditions: []metav1.Condition{{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionTrue,
Reason: "Scheduled",
Message: "All pods scheduled",
LastTransitionTime: metav1.Now(),
}},
},
},
algorithmResult: podGroupAlgorithmResult{
status: nil,
podResults: []algorithmResult{{
scheduleResult: ScheduleResult{SuggestedHost: "node1"},
status: nil,
permitStatus: nil,
}, {
scheduleResult: ScheduleResult{SuggestedHost: "node1"},
status: nil,
permitStatus: nil,
}, {
scheduleResult: ScheduleResult{SuggestedHost: "node1"},
status: nil,
permitStatus: nil,
}},
},
expectBound: sets.New("p1", "p2", "p3"),
expectCondition: &metav1.Condition{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionTrue,
Reason: "Scheduled",
},
},
{
@@ -1415,13 +1455,13 @@ func TestSubmitPodGroupAlgorithmResult(t *testing.T) {
cache.AddNode(klog.FromContext(ctx), testNode)
informerFactory := informers.NewSharedInformerFactory(client, 0)
if err := informerFactory.Scheduling().V1alpha2().PodGroups().Informer().GetStore().Add(pg); err != nil {
t.Fatalf("Failed to add PodGroup to informer store: %v", err)
}
podGroupLister := informerFactory.Scheduling().V1alpha2().PodGroups().Lister()
informerFactory.Start(ctx.Done())
informerFactory.WaitForCacheSync(ctx.Done())
sched := &Scheduler{
client: client,
podGroupLister: informerFactory.Scheduling().V1alpha2().PodGroups().Lister(),
podGroupLister: podGroupLister,
Cache: cache,
Profiles: profile.Map{"test-scheduler": schedFwk},
SchedulingQueue: internalqueue.NewTestQueue(ctx, nil),
@@ -1467,13 +1507,8 @@ func TestSubmitPodGroupAlgorithmResult(t *testing.T) {
t.Fatalf("Failed to get PodGroup: %v", err)
}
cond := apimeta.FindStatusCondition(updatedPodGroup.Status.Conditions, schedulingapi.PodGroupScheduled)
if tt.expectCondition != nil {
if cond == nil {
t.Fatalf("Expected PodGroupScheduled condition to be set, but it was not found")
}
if diff := cmp.Diff(*tt.expectCondition, *cond, cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")); diff != "" {
t.Errorf("Unexpected PodGroupScheduled condition (-want +got):\n%s", diff)
}
if diff := cmp.Diff(tt.expectCondition, cond, cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")); diff != "" {
t.Errorf("Unexpected PodGroupScheduled condition (-want +got):\n%s", diff)
}
})
}
@@ -1487,6 +1522,9 @@ func TestUpdatePodGroupCondition(t *testing.T) {
podGroupName string
condition *metav1.Condition
expectCondition *metav1.Condition
// expectLastTransitionTimeUnchanged, when true, verifies that LastTransitionTime
// is preserved from the existing condition.
expectLastTransitionTimeUnchanged bool
}{
{
name: "set Scheduled condition to True on empty status",
@@ -1640,6 +1678,7 @@ func TestUpdatePodGroupCondition(t *testing.T) {
Reason: "Scheduled",
Message: "All pods scheduled",
},
expectLastTransitionTimeUnchanged: true,
},
{
name: "do not regress Scheduled to SchedulerError",
@@ -1671,9 +1710,10 @@ func TestUpdatePodGroupCondition(t *testing.T) {
Reason: "Scheduled",
Message: "All pods scheduled",
},
expectLastTransitionTimeUnchanged: true,
},
{
name: "transition from Unschedulable to SchedulerError",
name: "transition from Unschedulable to SchedulerError preserves LastTransitionTime",
existingPodGroup: &schedulingv1alpha2.PodGroup{
ObjectMeta: metav1.ObjectMeta{Name: "pg-unsched-to-se", Namespace: "ns1"},
Status: schedulingv1alpha2.PodGroupStatus{
@@ -1702,9 +1742,10 @@ func TestUpdatePodGroupCondition(t *testing.T) {
Reason: schedulingapi.PodGroupReasonSchedulerError,
Message: "internal error",
},
expectLastTransitionTimeUnchanged: true,
},
{
name: "transition from SchedulerError to Unschedulable",
name: "transition from SchedulerError to Unschedulable preserves LastTransitionTime",
existingPodGroup: &schedulingv1alpha2.PodGroup{
ObjectMeta: metav1.ObjectMeta{Name: "pg-se-to-unsched", Namespace: "ns1"},
Status: schedulingv1alpha2.PodGroupStatus{
@@ -1733,6 +1774,7 @@ func TestUpdatePodGroupCondition(t *testing.T) {
Reason: schedulingapi.PodGroupReasonUnschedulable,
Message: "not enough resources",
},
expectLastTransitionTimeUnchanged: true,
},
{
name: "Scheduled to Scheduled is a no-op",
@@ -1764,6 +1806,7 @@ func TestUpdatePodGroupCondition(t *testing.T) {
Reason: "Scheduled",
Message: "All pods scheduled",
},
expectLastTransitionTimeUnchanged: true,
},
{
name: "PodGroup not found does not panic",
@@ -1782,6 +1825,27 @@ func TestUpdatePodGroupCondition(t *testing.T) {
Message: "test",
},
},
{
name: "ObservedGeneration is set from PodGroup generation",
existingPodGroup: &schedulingv1alpha2.PodGroup{
ObjectMeta: metav1.ObjectMeta{Name: "pg-gen", Namespace: "ns1", Generation: 7},
},
namespace: "ns1",
podGroupName: "pg-gen",
condition: &metav1.Condition{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionTrue,
Reason: "Scheduled",
Message: "All pods scheduled",
},
expectCondition: &metav1.Condition{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionTrue,
Reason: "Scheduled",
Message: "All pods scheduled",
ObservedGeneration: 7,
},
},
}
for _, tt := range tests {
@@ -1794,12 +1858,17 @@ func TestUpdatePodGroupCondition(t *testing.T) {
}
client := clientsetfake.NewClientset(objects...)
informerFactory := informers.NewSharedInformerFactory(client, 0)
podGroupLister := informerFactory.Scheduling().V1alpha2().PodGroups().Lister()
informerFactory.Start(ctx.Done())
informerFactory.WaitForCacheSync(ctx.Done())
sched := &Scheduler{client: client, podGroupLister: podGroupLister}
var existingLTT metav1.Time
if tt.existingPodGroup != nil {
if err := informerFactory.Scheduling().V1alpha2().PodGroups().Informer().GetStore().Add(tt.existingPodGroup); err != nil {
t.Fatalf("Failed to add PodGroup to informer store: %v", err)
if existing := apimeta.FindStatusCondition(tt.existingPodGroup.Status.Conditions, schedulingapi.PodGroupScheduled); existing != nil {
existingLTT = existing.LastTransitionTime
}
}
sched := &Scheduler{client: client, podGroupLister: informerFactory.Scheduling().V1alpha2().PodGroups().Lister()}
podGroupInfo := &framework.QueuedPodGroupInfo{
PodGroupInfo: &framework.PodGroupInfo{
@@ -1821,12 +1890,14 @@ func TestUpdatePodGroupCondition(t *testing.T) {
}
cond := apimeta.FindStatusCondition(updatedPodGroup.Status.Conditions, tt.expectCondition.Type)
if cond == nil {
t.Fatalf("Expected %s condition to be set, but it was not found", tt.expectCondition.Type)
if diff := cmp.Diff(tt.expectCondition, cond, cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")); diff != "" {
t.Errorf("Unexpected PodGroupScheduled condition (-want +got):\n%s", diff)
}
if diff := cmp.Diff(*tt.expectCondition, *cond, cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")); diff != "" {
t.Errorf("Unexpected PodGroupScheduled condition (-want +got):\n%s", diff)
if tt.expectLastTransitionTimeUnchanged {
if !cond.LastTransitionTime.Time.Truncate(time.Second).Equal(existingLTT.Time.Truncate(time.Second)) {
t.Errorf("Expected LastTransitionTime to be preserved as %v, got %v", existingLTT, cond.LastTransitionTime)
}
}
})
}

View File

@@ -1041,16 +1041,15 @@ func TestSchedulerScheduleOne(t *testing.T) {
item.sendPod = withSchedulingGroup(item.sendPod, group)
item.expectErrorPod = withSchedulingGroup(item.expectErrorPod, group)
item.expectPodInBackoffQ = withSchedulingGroup(item.expectPodInBackoffQ, group)
item.expectPodInUnschedulable = withSchedulingGroup(item.expectPodInUnschedulable, group)
if item.expectPodInUnschedulable != nil {
// Pods from a pod group skip unschedulablePods structure and land directly in the backoffQ.
item.expectPodInBackoffQ = withSchedulingGroup(item.expectPodInUnschedulable, group)
item.expectPodInUnschedulable = nil
}
testPG := &schedulingv1alpha2.PodGroup{
ObjectMeta: metav1.ObjectMeta{Name: "pg", Namespace: item.sendPod.Namespace},
}
pgIndexer := clientcache.NewIndexer(clientcache.MetaNamespaceKeyFunc, clientcache.Indexers{clientcache.NamespaceIndex: clientcache.MetaNamespaceIndexFunc})
if err := pgIndexer.Add(testPG); err != nil {
t.Fatalf("Failed to add PodGroup to indexer: %v", err)
}
podGroupLister = schedulinglisters.NewPodGroupLister(pgIndexer)
clientObjs = []runtime.Object{item.sendPod, testPG}
} else {
clientObjs = []runtime.Object{item.sendPod}
@@ -1075,6 +1074,7 @@ func TestSchedulerScheduleOne(t *testing.T) {
internalCache := internalcache.New(ctx, apiDispatcher, scheduleAsPodGroup)
if scheduleAsPodGroup {
podGroupLister = informerFactory.Scheduling().V1alpha2().PodGroups().Lister()
internalCache.AddPodGroupMember(item.sendPod)
}
cache := &fakecache.Cache{

View File

@@ -1645,8 +1645,8 @@ func (wrapper *WorkloadWrapper) Namespace(namespace string) *WorkloadWrapper {
}
// PodGroupTemplate appends the given PodGroupTemplate to the Workload spec.
func (wrapper *WorkloadWrapper) PodGroupTemplate(t *schedulingapi.PodGroupTemplate) *WorkloadWrapper {
wrapper.Workload.Spec.PodGroupTemplates = append(wrapper.Workload.Spec.PodGroupTemplates, *t)
func (wrapper *WorkloadWrapper) PodGroupTemplate(t schedulingapi.PodGroupTemplate) *WorkloadWrapper {
wrapper.Workload.Spec.PodGroupTemplates = append(wrapper.Workload.Spec.PodGroupTemplates, t)
return wrapper
}
@@ -1659,8 +1659,8 @@ func MakePodGroupTemplate() *PodGroupTemplateWrapper {
}
// Obj returns the inner PodGroupTemplate.
func (wrapper *PodGroupTemplateWrapper) Obj() *schedulingapi.PodGroupTemplate {
return &wrapper.PodGroupTemplate
func (wrapper *PodGroupTemplateWrapper) Obj() schedulingapi.PodGroupTemplate {
return wrapper.PodGroupTemplate
}
// Name sets `name` as the name of the inner PodGroupTemplate.

View File

@@ -98,7 +98,7 @@ func MoreImportantPod(pod1, pod2 *v1.Pod) bool {
// Retriable defines the retriable errors during a scheduling cycle.
func Retriable(err error) bool {
return apierrors.IsInternalError(err) || apierrors.IsServiceUnavailable(err) ||
net.IsConnectionRefused(err)
apierrors.IsConflict(err) || net.IsConnectionRefused(err)
}
// PatchPodStatus calculates the delta bytes change from <old.Status> to <newStatus>,

View File

@@ -28,8 +28,10 @@ import (
v1 "k8s.io/api/core/v1"
schedulingv1alpha2 "k8s.io/api/scheduling/v1alpha2"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/net"
clientsetfake "k8s.io/client-go/kubernetes/fake"
clienttesting "k8s.io/client-go/testing"
@@ -302,15 +304,16 @@ func TestPatchPodStatus(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
_, ctx := ktesting.NewTestContext(t)
ctx, cancel := context.WithCancel(ctx)
defer cancel()
client := tc.client
_, err := client.CoreV1().Pods(tc.pod.Namespace).Create(context.TODO(), &tc.pod, metav1.CreateOptions{})
if err != nil {
t.Fatal(err)
}
_, ctx := ktesting.NewTestContext(t)
ctx, cancel := context.WithCancel(ctx)
defer cancel()
err = PatchPodStatus(ctx, client, tc.pod.Name, tc.pod.Namespace, &tc.pod.Status, &tc.statusToUpdate)
if err != nil && tc.validateErr == nil {
// shouldn't be error
@@ -483,19 +486,59 @@ func TestPatchPodGroupStatus(t *testing.T) {
},
},
},
{
name: "retry patch request when a conflict error is returned",
client: func() *clientsetfake.Clientset {
client := clientsetfake.NewClientset()
reqcount := 0
client.PrependReactor("patch", "podgroups", func(action clienttesting.Action) (bool, runtime.Object, error) {
defer func() { reqcount++ }()
if reqcount == 0 {
return true, &schedulingv1alpha2.PodGroup{},
apierrors.NewConflict(schema.GroupResource{
Resource: "podgroups"}, "pg1",
errors.New("the object has been modified"))
}
if reqcount == 1 {
return false, &schedulingv1alpha2.PodGroup{}, nil
}
return true, nil, errors.New("requests comes in more than three times.")
})
return client
}(),
podGroup: schedulingv1alpha2.PodGroup{
ObjectMeta: metav1.ObjectMeta{
Namespace: "ns",
Name: "pg1",
},
},
statusToUpdate: &schedulingv1alpha2.PodGroupStatus{
Conditions: []metav1.Condition{
{
Type: schedulingapi.PodGroupScheduled,
Status: metav1.ConditionFalse,
Reason: schedulingapi.PodGroupReasonUnschedulable,
Message: "not enough capacity for the gang",
LastTransitionTime: now,
},
},
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
_, ctx := ktesting.NewTestContext(t)
ctx, cancel := context.WithCancel(ctx)
defer cancel()
client := tc.client
_, err := client.SchedulingV1alpha2().PodGroups(tc.podGroup.Namespace).Create(ctx, &tc.podGroup, metav1.CreateOptions{})
if err != nil {
t.Fatal(err)
}
defer cancel()
err = PatchPodGroupStatus(ctx, client, tc.podGroup.Name, tc.podGroup.Namespace, &tc.podGroup.Status, tc.statusToUpdate)
if err != nil && tc.validateErr == nil {