Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changes/unreleased/fixed-20260810-125620.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Fixed
body: |-
Preserve reservation pods during BindRequest cache lag
54 changes: 47 additions & 7 deletions pkg/binder/binding/resourcereservation/resource_reservation.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,26 +226,66 @@ func (rsc *service) syncForPods(ctx context.Context, pods []*v1.Pod, gpuGroupToS
return nil
}

// hasActiveBindRequestsForGpuGroup checks if any non-terminal BindRequests reference
// the given GPU group. This prevents premature reservation pod deletion when the
// informer cache has not yet propagated GPU group labels on recently-bound fraction pods.
// hasActiveBindRequestsForGpuGroup checks if BindRequests still protect the GPU group.
// A succeeded BindRequest also protects the reservation while its pod is still alive:
// the pod label can lag behind the BindRequest status in the controller cache.
func (rsc *service) hasActiveBindRequestsForGpuGroup(ctx context.Context, gpuGroup string) (bool, error) {
bindRequestList := &schedulingv1alpha2.BindRequestList{}
if err := rsc.kubeClient.List(ctx, bindRequestList); err != nil {
return false, fmt.Errorf("failed to list BindRequests: %w", err)
}

for _, br := range bindRequestList.Items {
if br.Status.Phase == schedulingv1alpha2.BindRequestPhaseSucceeded ||
br.Status.Phase == schedulingv1alpha2.BindRequestPhaseFailed {
if !slices.Contains(br.Spec.SelectedGPUGroups, gpuGroup) {
continue
}
if slices.Contains(br.Spec.SelectedGPUGroups, gpuGroup) {
return true, nil

if br.Status.Phase == schedulingv1alpha2.BindRequestPhaseFailed {
continue
}

if br.Status.Phase == schedulingv1alpha2.BindRequestPhaseSucceeded {
hasLivePod, err := rsc.hasLivePodForBindRequest(ctx, &br)
if err != nil {
return false, err
}
if hasLivePod {
return true, nil
}
continue
}

return true, nil
}
return false, nil
}

func (rsc *service) hasLivePodForBindRequest(ctx context.Context, bindRequest *schedulingv1alpha2.BindRequest) (bool, error) {
pod := &v1.Pod{}
err := rsc.kubeClient.Get(ctx, client.ObjectKey{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider also matching the UID of the pod (against the BindRequests's OwnerReference) since a pod can be recreated with the same name.

Namespace: bindRequest.Namespace,
Name: bindRequest.Spec.PodName,
}, pod)
if apierrors.IsNotFound(err) {
return false, nil
}
if err != nil {
return false, fmt.Errorf("failed to get pod for BindRequest <%s/%s>: %w",
bindRequest.Namespace, bindRequest.Name, err)
}

if slices.Contains([]v1.PodPhase{v1.PodSucceeded, v1.PodFailed}, pod.Status.Phase) {
return false, nil
}

for _, gpuGroup := range bindRequest.Spec.SelectedGPUGroups {
if slices.Contains(resources.GetGpuGroups(pod), gpuGroup) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the effect of this loop? We return true afterwards anyway, right?

return true, nil
}
}

return true, nil
}
func (rsc *service) ReserveGpuDevice(ctx context.Context, pod *v1.Pod, nodeName string, gpuGroup string) (string, error) {
logger := log.FromContext(ctx)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,47 @@ var _ = Describe("Race condition: reservation pod deleted during concurrent bind
"Reservation pod should be deleted when only terminal BindRequests exist")
})

It("should preserve reservation pod when succeeded BindRequest still has a live pod", func() {
livePod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "fraction-pod-1",
Namespace: "team-a",
},
Status: v1.PodStatus{
Phase: v1.PodPending,
},
}
succeededBindRequest := &schedulingv1alpha2.BindRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "bind-request-done",
Namespace: "team-a",
},
Spec: schedulingv1alpha2.BindRequestSpec{
PodName: livePod.Name,
SelectedNode: nodeName,
SelectedGPUGroups: []string{gpuGroup},
},
Status: schedulingv1alpha2.BindRequestStatus{
Phase: schedulingv1alpha2.BindRequestPhaseSucceeded,
},
}

clientWithObjs := fake.NewClientBuilder().WithScheme(testScheme).
WithRuntimeObjects(reservationPod.DeepCopy(), livePod, succeededBindRequest).
WithIndex(&v1.Pod{}, "spec.nodeName", nodeNameIndexer).Build()
rsc := initializeTestService(clientWithObjs)

err := rsc.SyncForGpuGroup(context.TODO(), gpuGroup)
Expect(err).To(Succeed())

pods := &v1.PodList{}
err = clientWithObjs.List(context.Background(), pods,
runtimeClient.InNamespace(resourceReservationNameSpace))
Expect(err).To(Succeed())
Expect(len(pods.Items)).To(Equal(1),
"Reservation pod should be preserved while the bound pod may still be missing the gpu-group label")
})

It("should delete reservation pod when only failed BindRequests exist", func() {
failedBindRequest := &schedulingv1alpha2.BindRequest{
ObjectMeta: metav1.ObjectMeta{
Expand Down
Loading