Skip to content

Commit d70985f

Browse files
committed
fix(binder): preserve reservations during bind cache lag
Signed-off-by: davidLif <davidshani12@gmail.com>
1 parent 965c684 commit d70985f

3 files changed

Lines changed: 91 additions & 7 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
kind: Fixed
2+
body: |-
3+
Preserve reservation pods during BindRequest cache lag

pkg/binder/binding/resourcereservation/resource_reservation.go

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -226,26 +226,66 @@ func (rsc *service) syncForPods(ctx context.Context, pods []*v1.Pod, gpuGroupToS
226226
return nil
227227
}
228228

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

238238
for _, br := range bindRequestList.Items {
239-
if br.Status.Phase == schedulingv1alpha2.BindRequestPhaseSucceeded ||
240-
br.Status.Phase == schedulingv1alpha2.BindRequestPhaseFailed {
239+
if !slices.Contains(br.Spec.SelectedGPUGroups, gpuGroup) {
241240
continue
242241
}
243-
if slices.Contains(br.Spec.SelectedGPUGroups, gpuGroup) {
244-
return true, nil
242+
243+
if br.Status.Phase == schedulingv1alpha2.BindRequestPhaseFailed {
244+
continue
245245
}
246+
247+
if br.Status.Phase == schedulingv1alpha2.BindRequestPhaseSucceeded {
248+
hasLivePod, err := rsc.hasLivePodForBindRequest(ctx, &br)
249+
if err != nil {
250+
return false, err
251+
}
252+
if hasLivePod {
253+
return true, nil
254+
}
255+
continue
256+
}
257+
258+
return true, nil
246259
}
247260
return false, nil
248261
}
262+
263+
func (rsc *service) hasLivePodForBindRequest(ctx context.Context, bindRequest *schedulingv1alpha2.BindRequest) (bool, error) {
264+
pod := &v1.Pod{}
265+
err := rsc.kubeClient.Get(ctx, client.ObjectKey{
266+
Namespace: bindRequest.Namespace,
267+
Name: bindRequest.Spec.PodName,
268+
}, pod)
269+
if apierrors.IsNotFound(err) {
270+
return false, nil
271+
}
272+
if err != nil {
273+
return false, fmt.Errorf("failed to get pod for BindRequest <%s/%s>: %w",
274+
bindRequest.Namespace, bindRequest.Name, err)
275+
}
276+
277+
if slices.Contains([]v1.PodPhase{v1.PodSucceeded, v1.PodFailed}, pod.Status.Phase) {
278+
return false, nil
279+
}
280+
281+
for _, gpuGroup := range bindRequest.Spec.SelectedGPUGroups {
282+
if slices.Contains(resources.GetGpuGroups(pod), gpuGroup) {
283+
return true, nil
284+
}
285+
}
286+
287+
return true, nil
288+
}
249289
func (rsc *service) ReserveGpuDevice(ctx context.Context, pod *v1.Pod, nodeName string, gpuGroup string) (string, error) {
250290
logger := log.FromContext(ctx)
251291

pkg/binder/binding/resourcereservation/resource_reservation_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,6 +1437,47 @@ var _ = Describe("Race condition: reservation pod deleted during concurrent bind
14371437
"Reservation pod should be deleted when only terminal BindRequests exist")
14381438
})
14391439

1440+
It("should preserve reservation pod when succeeded BindRequest still has a live pod", func() {
1441+
livePod := &v1.Pod{
1442+
ObjectMeta: metav1.ObjectMeta{
1443+
Name: "fraction-pod-1",
1444+
Namespace: "team-a",
1445+
},
1446+
Status: v1.PodStatus{
1447+
Phase: v1.PodPending,
1448+
},
1449+
}
1450+
succeededBindRequest := &schedulingv1alpha2.BindRequest{
1451+
ObjectMeta: metav1.ObjectMeta{
1452+
Name: "bind-request-done",
1453+
Namespace: "team-a",
1454+
},
1455+
Spec: schedulingv1alpha2.BindRequestSpec{
1456+
PodName: livePod.Name,
1457+
SelectedNode: nodeName,
1458+
SelectedGPUGroups: []string{gpuGroup},
1459+
},
1460+
Status: schedulingv1alpha2.BindRequestStatus{
1461+
Phase: schedulingv1alpha2.BindRequestPhaseSucceeded,
1462+
},
1463+
}
1464+
1465+
clientWithObjs := fake.NewClientBuilder().WithScheme(testScheme).
1466+
WithRuntimeObjects(reservationPod.DeepCopy(), livePod, succeededBindRequest).
1467+
WithIndex(&v1.Pod{}, "spec.nodeName", nodeNameIndexer).Build()
1468+
rsc := initializeTestService(clientWithObjs)
1469+
1470+
err := rsc.SyncForGpuGroup(context.TODO(), gpuGroup)
1471+
Expect(err).To(Succeed())
1472+
1473+
pods := &v1.PodList{}
1474+
err = clientWithObjs.List(context.Background(), pods,
1475+
runtimeClient.InNamespace(resourceReservationNameSpace))
1476+
Expect(err).To(Succeed())
1477+
Expect(len(pods.Items)).To(Equal(1),
1478+
"Reservation pod should be preserved while the bound pod may still be missing the gpu-group label")
1479+
})
1480+
14401481
It("should delete reservation pod when only failed BindRequests exist", func() {
14411482
failedBindRequest := &schedulingv1alpha2.BindRequest{
14421483
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)