diff --git a/.changes/unreleased/fixed-20260810-125620.yaml b/.changes/unreleased/fixed-20260810-125620.yaml new file mode 100644 index 000000000..dd0798e37 --- /dev/null +++ b/.changes/unreleased/fixed-20260810-125620.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: |- + Preserve reservation pods during BindRequest cache lag diff --git a/pkg/binder/binding/resourcereservation/resource_reservation.go b/pkg/binder/binding/resourcereservation/resource_reservation.go index 1944827e3..dc1ac17aa 100644 --- a/pkg/binder/binding/resourcereservation/resource_reservation.go +++ b/pkg/binder/binding/resourcereservation/resource_reservation.go @@ -226,9 +226,9 @@ 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 { @@ -236,16 +236,56 @@ func (rsc *service) hasActiveBindRequestsForGpuGroup(ctx context.Context, gpuGro } 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{ + 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) { + 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) diff --git a/pkg/binder/binding/resourcereservation/resource_reservation_test.go b/pkg/binder/binding/resourcereservation/resource_reservation_test.go index 0749cdef9..d3ddf8b38 100644 --- a/pkg/binder/binding/resourcereservation/resource_reservation_test.go +++ b/pkg/binder/binding/resourcereservation/resource_reservation_test.go @@ -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{