Skip to content

Replace CSI snapshot handle polling with watch when available - #10249

Open
Pragati5-DEBUG wants to merge 2 commits into
velero-io:mainfrom
Pragati5-DEBUG:fix/9956-csi-watch
Open

Replace CSI snapshot handle polling with watch when available#10249
Pragati5-DEBUG wants to merge 2 commits into
velero-io:mainfrom
Pragati5-DEBUG:fix/9956-csi-watch

Conversation

@Pragati5-DEBUG

Copy link
Copy Markdown
Contributor

Summary

Replace CSI VolumeSnapshotContent handle readiness polling with Kubernetes watch when a WithWatch client is available, reducing apiserver GET traffic during CSI backups.

WaitUntilVSCHandleIsReady now:

Tries a watch-based wait on VolumeSnapshot, then VolumeSnapshotContent
Falls back to polling if watch is unavailable or fails (except on timeout)
Preserves existing VSS behavior via CSI_SNAPSHOT_EARLY_FREQUENT_POLLING (1s polling for the first 10s, then 5s intervals)
The PVC backup item action is updated to use KubebuilderWatchClient() so the watch path is used in production.

Does your change fix a particular issue?

Fixes #9956

Changes

Refactor WaitUntilVSCHandleIsReady into tryPopulateVSCHandle, watchUntilVSCHandleReady, and poll/watch wrappers
Switch pvcBackupItemAction client from crclient.Client to crclient.WithWatch
Update unit tests to use NewFakeControllerRuntimeWatchClient

Testing

  1. go test ./pkg/util/csi/... ./pkg/backup/actions/csi/...

Checklist

Accepted the DCO. Commits without the DCO will delay acceptance.

Created a changelog file (make new-changelog) or comment /kind changelog-not-required on this PR.

Updated the corresponding documentation in site/content/docs/main.

@Pragati5-DEBUG
Pragati5-DEBUG requested a review from a team as a code owner August 12, 2026 13:35
@netlify

netlify Bot commented Aug 12, 2026

Copy link
Copy Markdown

👷 Deploy request for velero pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 6c717af

Use Kubernetes watch to wait for VolumeSnapshotContent snapshot handle
readiness when a WithWatch client is available, falling back to polling
only for non-timeout watch failures. Preserve VSS early frequent polling
via CSI_SNAPSHOT_EARLY_FREQUENT_POLLING, keep timeout errors from
triggering a second poll wait, and add watch-path unit tests.

Fixes velero-io#9956

Signed-off-by: Pragati <Pragati5-DEBUG@users.noreply.github.com>
Signed-off-by: Pragati <Pragati5-DEBUG@users.noreply.github.com>
@Pragati5-DEBUG
Pragati5-DEBUG force-pushed the fix/9956-csi-watch branch 2 times, most recently from 6a47255 to 7798278 Compare August 12, 2026 13:55
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.07937% with 66 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/util/csi/volume_snapshot.go 64.89% 56 Missing and 10 partials ⚠️

📢 Thoughts on this report? Let us know!

@Pragati5-DEBUG
Pragati5-DEBUG force-pushed the fix/9956-csi-watch branch 2 times, most recently from 0f2a073 to 286ae73 Compare August 12, 2026 19:01

// WaitUntilVSCHandleIsReady returns the VolumeSnapshotContent
// object associated with the volumesnapshot.
func WaitUntilVSCHandleIsReady(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This introduces a critical timeout bug when falling back from Watch to Poll.

Both waitUntilVSCHandleIsReadyWithWatch & waitUntilVSCHandleIsReadyWithPoll create a brand new context.WithTimeout(context.Background(), csiSnapshotTimeout).

If the API closes the watch channel after 5 min, we will fall back to polling, which then starts a brand new 10 min timeout from zero. It will silently breaking the csiSnapshotTimeout contract.

Fix :- WaitUntilVSCHandleIsReady should create a single context.WithTimeout at the top level &
pass the resulting ctx context.Context down into the helpers, instead of passing the duration and recreating the context.

@ywk253100
ywk253100 requested a review from sseago August 14, 2026 07:25
@ywk253100

Copy link
Copy Markdown
Contributor

@sseago Could you take a look at this PR when you are availble?

@ywk253100
ywk253100 requested a review from blackpiglet August 18, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider refactoring CSI early frequent polling to avoid unnecessary apiserver calls

3 participants