Fail fast with clear error when data mover pod is unschedulable - #10276
Open
kaovilai wants to merge 3 commits into
Open
Fail fast with clear error when data mover pod is unschedulable#10276kaovilai wants to merge 3 commits into
kaovilai wants to merge 3 commits into
Conversation
A prior fix (velero-io#9697) removed a blanket "Unschedulable" phase check from IsPodUnrecoverable because Unschedulable isn't always permanent (e.g. insufficient CPU/memory can resolve via autoscaling, untolerated taints can be removed). That left node-affinity misconfigurations (most commonly a node-agent loadAffinity referencing labels no node actually has) to run out the full preparing/operation timeout before failing with a generic "timeout on preparing data upload" message. Adds a narrower, targeted check: IsPodUnschedulableDueToNodeAffinity verifies whether the pod's required node affinity can be satisfied by ANY node currently in the cluster. Unlike generic scheduling failures, node affinity is purely label-based - if zero nodes' labels satisfy it right now, no amount of waiting on existing nodes changes that, so it's safe to report immediately instead of waiting out the timeout. Wired into all three exposer PeekExposed implementations and all four pod-watch controller handlers (DataUpload, DataDownload, PodVolumeBackup, PodVolumeRestore) via a combined IsPodUnrecoverableOrUnschedulable helper, since all four VGDP paths share the same loadAffinity mechanism. Fixes velero-io#9735 > [!Note] > Responses generated with Claude Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
✅ Deploy Preview for velero ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
A one-time node-list snapshot could race with a node that's mid-join (e.g. a cluster-autoscaler-provisioned node with matching labels that hasn't registered yet) or an admin mid-relabel, producing a false permanent verdict - the same trap velero-io#9697 hit with a blanket Unschedulable check. Gate on the PodScheduled condition's own LastTransitionTime: only report once it's held False for at least unschedulableNodeAffinityGracePeriod (2m), which still fails much faster than the default ~10 minute preparing timeout while riding out ordinary node-join latency. Also documents (rather than implements) why MatchFields and Gt/Lt node-selector operators are out of scope: the only producer of node affinity for these pods is Velero's own ToSystemAffinity, which builds requirements exclusively from a metav1.LabelSelector (an enum with no Gt/Lt) plus CSI topology requirements (always equality-based) - neither MatchFields nor Gt/Lt can occur in practice for this caller. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #9735: when a data-mover exposer pod (CSI snapshot backup/restore, or pod-volume backup/restore) is unschedulable because its required node affinity (typically driven by node-agent
loadAffinity) can't be satisfied by any node, Velero currently waits out the full preparing/operation timeout (10+ minutes by default) before failing with a generic"timeout on preparing data upload"message and no indication of the actual scheduling problem.A prior fix (#9697) intentionally removed a blanket
Unschedulable-phase check fromIsPodUnrecoverable, becauseUnschedulableisn't always permanent — insufficient CPU/memory can resolve via cluster autoscaling, untolerated taints can be removed, another pod can complete and free resources. This PR does not reintroduce that generic check. Instead it adds a narrower, targeted one:kube.IsPodUnschedulableDueToNodeAffinitychecks whether the pod'sPodScheduledcondition isFalseand whether its required node affinity is satisfiable by any node currently in the cluster. Node affinity is purely label-based (unlike resource requests), so if zero nodes' labels satisfy it right now, waiting cannot change that — it's a permanent scheduling mismatch, most commonly a stale/incorrectloadAffinityconfig.kube.IsPodUnrecoverableOrUnschedulable, wired into all three exposerPeekExposedimplementations (CSI snapshot, generic restore, pod-volume) and all four pod-watch controller handlers (DataUpload,DataDownload,PodVolumeBackup,PodVolumeRestore) — all four VGDP paths share the sameloadAffinity/ToSystemAffinitymechanism, so the bug and the fix are both shared across them.In/NotIn/Exists/DoesNotExist) is hand-rolled rather than pulling ink8s.io/component-helpersas a new dependency; unknown/unimplemented operators (Gt/Lt) default to "might be satisfiable" to avoid false positives.Test plan
pkg/util/kube/pod_test.go:TestIsPodUnschedulableDueToNodeAffinity(condition-true no-op, no-affinity no-op, zero-node-match → true, some-node-matches → false) andTestIsPodUnrecoverableOrUnschedulable(fall-through, precedence/short-circuit, healthy-pod no-op).go build ./...,gofmt/goimportsclean,go test ./pkg/util/kube/... ./pkg/exposer/... ./pkg/controller/...passing (envtest-onlyTestAPIsskipped locally — missing kubebuilder binary in this sandbox, unrelated to this change).Fixes #9735
Note
Responses generated with Claude