Skip to content

Commit 36ee3c1

Browse files
committed
cache: recover from PVC-informer race in addPod
A Pod ADD event can reach the scheduler before its referenced PVC ADD reaches the PVC informer — for example when a Pod is created together with an OnDemand PVC in quick succession (Spark's k8s scheduler-backend does exactly this for executor pods with `claimName=OnDemand`). Since v1.15.0, the resulting `looking up PVC ... not found` error from NewTaskInfo permanently strands the pod: addPod → NewTaskInfo → addPodCSIVolumesToTask → pvcInformer.Lister().Get(pvcName) returns not-found → klog.Errorf(...) → sc.resyncTask(pi) // enqueues taskKey into errTasks → return err // task never enters sc.Jobs processResyncTask → errTasks.Get() → parseErrTaskKey → getTaskByUID → "failed to find task <uid>" → errTasks.Forget(taskKey) // permanent → return // never reaches retryResyncTask There is no PVC-informer AddFunc that re-queues stuck pods, and the pod's resourceVersion never advances before scheduling, so `UpdatePod → deletePod + addPod` never fires. Executor pods stay Pending; the PVC stays Pending with `WaitForPodScheduled`. Before volcano-sh#5058, `addPod`'s error branch fell through to `sc.addTask(pi)` even after `NewTaskInfo` returned an error, so the incomplete task lived in `sc.Jobs` and processResyncTask could find it by UID on the next tick and call syncTask, which refetches the pod from the apiserver — by which point the PVC informer has synced. volcano-sh#5058 added a DRA-specific happy path using the correct `addTask + resyncTask + return nil` pattern, but also added a generic `return err` that removed the implicit recovery for PVC-not-found. Extend the DRA branch to PVC-not-found errors, using the same pattern. Change getPodCSIVolumes to wrap the lister error with %w so callers can classify it with errors.Is / apierrors.IsNotFound instead of matching on the message string. Adds `TestAddPodWithUnresolvedPVCCachesTaskForResync` mirroring the existing DRA counterpart. Version scope: only v1.15.0 is affected. Verified with `git merge-base --is-ancestor 5ba79f3 <tag>` — every v1.14.x tag through v1.14.3 (released after v1.15.0) is clean; v1.15.0-alpha.0 is also clean. Signed-off-by: Gustavo Parcianello <gustavo.parcianello@sap.com>
1 parent e4f12cb commit 36ee3c1

3 files changed

Lines changed: 144 additions & 1 deletion

File tree

pkg/scheduler/cache/cache_mock.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,13 @@ func (sc *SchedulerCache) initMockInformers() {
164164
// Create node informer
165165
sc.nodeInformer = informerFactory.Core().V1().Nodes()
166166

167+
// Create PVC informer (required by addPodCSIVolumesToTask for pods
168+
// referencing a PersistentVolumeClaim). Materialise via Informer() so
169+
// SharedInformerFactory.Start actually starts it — matches the real
170+
// newSchedulerCache wiring.
171+
sc.pvcInformer = informerFactory.Core().V1().PersistentVolumeClaims()
172+
sc.pvcInformer.Informer()
173+
167174
// Initialize DRA manager if feature is enabled
168175
if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.DynamicResourceAllocation) {
169176
ctx := context.TODO()

pkg/scheduler/cache/event_handlers.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,12 @@ func (sc *SchedulerCache) getPodCSIVolumes(pod *v1.Pod) (map[v1.ResourceName]int
129129
// The PVC is required to proceed with
130130
// scheduling of a new pod because it cannot
131131
// run without it. Bail out immediately.
132-
return volumes, fmt.Errorf("looking up PVC %s/%s: %v", pod.Namespace, pvcName, err)
132+
//
133+
// Wrap with %w so callers can distinguish
134+
// "PVC not yet in informer" (not-found) from
135+
// other lookup failures via errors.Is /
136+
// apierrors.IsNotFound and recover accordingly.
137+
return volumes, fmt.Errorf("looking up PVC %s/%s: %w", pod.Namespace, pvcName, err)
133138
}
134139
// The PVC for an ephemeral volume must be owned by the pod.
135140
if isEphemeral {
@@ -271,6 +276,16 @@ func (sc *SchedulerCache) addPod(pod *v1.Pod) error {
271276
sc.resyncTask(pi)
272277
return nil
273278
}
279+
// Recover from a Pod ADD that races ahead of its PVC ADD on
280+
// the informers. Same pattern as the DRA branch above.
281+
if errors.IsNotFound(err) {
282+
klog.V(4).Infof("PVC for pod <%s/%s> not yet in informer cache, add task and retry: %v", pod.Namespace, pod.Name, err)
283+
if addErr := sc.addTask(pi); addErr != nil {
284+
return addErr
285+
}
286+
sc.resyncTask(pi)
287+
return nil
288+
}
274289
klog.Errorf("generate taskInfo for pod(%s) failed: %v", pod.Name, err)
275290
sc.resyncTask(pi)
276291
return err
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
Copyright 2026 The Volcano Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package cache
18+
19+
import (
20+
"context"
21+
"testing"
22+
"time"
23+
24+
"github.com/stretchr/testify/assert"
25+
v1 "k8s.io/api/core/v1"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/types"
28+
"k8s.io/apimachinery/pkg/util/wait"
29+
30+
schedulingv1beta1 "volcano.sh/apis/pkg/apis/scheduling/v1beta1"
31+
schedulingapi "volcano.sh/volcano/pkg/scheduler/api"
32+
)
33+
34+
// TestAddPodWithUnresolvedPVCCachesTaskForResync verifies the full
35+
// recovery path when a Pod ADD races ahead of its PVC ADD:
36+
//
37+
// 1. addPod caches the task and enqueues it for resync instead of
38+
// permanently dropping the pod.
39+
// 2. Once the PVC arrives on the informer, one processResyncTask
40+
// tick refreshes the task via syncTask and removes it from
41+
// errTasks — the pod is now visible to the scheduler.
42+
//
43+
// Mirrors the DRA counterpart TestAddPodWithUnresolvedResourceClaimTemplateCachesTaskForResync,
44+
// extended with the end-to-end recovery assertion.
45+
func TestAddPodWithUnresolvedPVCCachesTaskForResync(t *testing.T) {
46+
sc := newMockSchedulerCache("volcano")
47+
48+
ctx, cancel := context.WithCancel(context.Background())
49+
defer cancel()
50+
51+
pod := &v1.Pod{
52+
ObjectMeta: metav1.ObjectMeta{
53+
Name: "pod-with-pvc",
54+
Namespace: "default",
55+
UID: types.UID("pod-with-pvc-uid"),
56+
Annotations: map[string]string{
57+
schedulingv1beta1.KubeGroupNameAnnotationKey: "pg-with-pvc",
58+
},
59+
},
60+
Spec: v1.PodSpec{
61+
SchedulerName: "volcano",
62+
Volumes: []v1.Volume{
63+
{
64+
Name: "scratch",
65+
VolumeSource: v1.VolumeSource{
66+
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
67+
ClaimName: "eventual-pvc",
68+
},
69+
},
70+
},
71+
},
72+
},
73+
}
74+
75+
// Seed the pod in the fake kube-client so syncTask's later
76+
// Pods(ns).Get(name) call finds it on the retry tick.
77+
_, err := sc.kubeClient.CoreV1().Pods(pod.Namespace).Create(ctx, pod, metav1.CreateOptions{})
78+
assert.NoError(t, err)
79+
80+
sc.informerFactory.Start(ctx.Done())
81+
for informer, synced := range sc.informerFactory.WaitForCacheSync(ctx.Done()) {
82+
assert.Truef(t, synced, "informer %v failed to sync", informer)
83+
}
84+
85+
// Phase 1: PVC ADD has not arrived yet. addPod must defer to resync.
86+
err = sc.addPod(pod)
87+
assert.NoError(t, err, "addPod must defer to the retry loop when the PVC is missing from the informer cache")
88+
89+
job, found := sc.Jobs[schedulingapi.JobID("default/pg-with-pvc")]
90+
assert.True(t, found, "job must be added to sc.Jobs so processResyncTask can find its task by UID")
91+
if assert.NotNil(t, job) {
92+
_, found = job.Tasks[schedulingapi.TaskID("pod-with-pvc-uid")]
93+
assert.True(t, found, "task must be added to job.Tasks so getTaskByUID succeeds on the retry tick")
94+
}
95+
assert.Equal(t, 1, sc.errTasks.Len(), "one entry must be enqueued on errTasks so processResyncTask visits this task")
96+
97+
// Phase 2: PVC arrives on the informer. processResyncTask should
98+
// refresh the task via syncTask and remove it from errTasks.
99+
pvc := &v1.PersistentVolumeClaim{
100+
ObjectMeta: metav1.ObjectMeta{
101+
Name: "eventual-pvc",
102+
Namespace: "default",
103+
},
104+
}
105+
_, err = sc.kubeClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Create(ctx, pvc, metav1.CreateOptions{})
106+
assert.NoError(t, err)
107+
108+
// Wait for the PVC informer to observe the new PVC.
109+
err = wait.PollUntilContextTimeout(ctx, 20*time.Millisecond, 2*time.Second, true, func(context.Context) (bool, error) {
110+
_, err := sc.pvcInformer.Lister().PersistentVolumeClaims("default").Get("eventual-pvc")
111+
return err == nil, nil
112+
})
113+
assert.NoError(t, err, "PVC never appeared on the informer lister")
114+
115+
// Drive one iteration of the resync worker. This pops the taskKey,
116+
// calls syncTask (fresh kubeClient Get → NewTaskInfo → PVC lookup
117+
// now succeeds), and Forgets the taskKey on success.
118+
sc.processResyncTask()
119+
120+
assert.Equal(t, 0, sc.errTasks.Len(), "task must be removed from errTasks once the PVC has synced and syncTask succeeds")
121+
}

0 commit comments

Comments
 (0)