Skip to content

Remove Restic path for Pod Volume Restore#9662

Open
testsabirweb wants to merge 2 commits intovelero-io:mainfrom
testsabirweb:issue-9468-remove-restic-pvr
Open

Remove Restic path for Pod Volume Restore#9662
testsabirweb wants to merge 2 commits intovelero-io:mainfrom
testsabirweb:issue-9468-remove-restic-pvr

Conversation

@testsabirweb
Copy link
Copy Markdown
Contributor

@testsabirweb testsabirweb commented Apr 1, 2026

Thank you for contributing to Velero!

Please add a summary of your change

This PR removes Restic support from the Pod Volume Restore (PVR) path as part of the v1.19 Restic deprecation work (epic #9467).

Restorer (pkg/podvolume)

  • Before creating PVRs, if any volume’s backup uses a Restic repository, restore returns ErrResticFileSystemBackupUnsupported and no PVRs are created for that pod. The error documents whole-pod behavior: when any volume is Restic-backed, the entire pod volume restore is skipped (including volumes that would otherwise use Kopia), so users are not told only “Restic volumes” failed.
  • Removed the obsolete ResticIdentifier wiring for PVR creation (Kopia-only path).

Advanced PVR controller

  • If spec.uploaderType is explicitly restic, the PVR is failed early with the same error, before finalizers or data path setup.

Legacy controller

  • IsLegacyPVR always returns false (the legacy async reconcile existed only for Restic). The legacy controller therefore no longer processes PVRs.

Server and node-agent

  • Removed dead IsLegacyPVR branches and the markLegacyPVRsFailed recovery path on node-agent startup (those paths never ran after IsLegacyPVR became always false).

Tests

  • Restorer: Restic unsupported path (including mixed Restic + Kopia PVBs).
  • Legacy: no legacy PVR enqueue.
  • Controller: explicit Restic uploader rejected.

Does your change fix a particular issue?

Fixes #9468

Please indicate you've done the following:

  • 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.
    • Added changelogs/unreleased/9662-testsabirweb.
  • Updated the corresponding documentation in site/content/docs/main.
    • Broader Restic removal / user-facing docs are tracked in #9483. This PR does not change site/content/docs/main; follow up there or in a separate PR if required by reviewers.

@github-actions github-actions Bot requested a review from blackpiglet April 1, 2026 07:52
@github-actions github-actions Bot requested a review from Lyndon-Li April 1, 2026 07:52
testsabirweb added a commit to testsabirweb/velero that referenced this pull request Apr 1, 2026
Signed-off-by: Sabir Ali <sabir.ali@spectrocloud.com>
testsabirweb added a commit to testsabirweb/velero that referenced this pull request Apr 1, 2026
Signed-off-by: Sabir Ali <sabir.ali@spectrocloud.com>
@testsabirweb testsabirweb force-pushed the issue-9468-remove-restic-pvr branch from 86bbf28 to aa5770a Compare April 1, 2026 08:02
@testsabirweb testsabirweb changed the title Remove Restic path for Pod Volume Restore (#9468) Remove Restic path for Pod Volume Restore Apr 1, 2026
- Reject restores from Restic PodVolumeBackups in the restorer; export
  ErrResticFileSystemBackupUnsupported with whole-pod messaging when any
  volume is Restic-backed (including mixed Kopia volumes).
- Fail PodVolumeRestores with explicit Restic uploader in the advanced controller.
- Disable legacy Restic-only async path (IsLegacyPVR always false); remove dead
  IsLegacyPVR handling in server and node-agent (markLegacyPVRsFailed).
- Tests: restic unsupported paths, legacy enqueue, controller restic case.

Signed-off-by: Sabir Ali <sabir.ali@spectrocloud.com>
Signed-off-by: Sabir Ali <sabir.ali@spectrocloud.com>
@blackpiglet blackpiglet force-pushed the issue-9468-remove-restic-pvr branch from aa5770a to cc9916d Compare April 2, 2026 08:41
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.03%. Comparing base (4dbdd2d) to head (cc9916d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9662      +/-   ##
==========================================
+ Coverage   60.99%   61.03%   +0.03%     
==========================================
  Files         387      387              
  Lines       36629    36599      -30     
==========================================
- Hits        22341    22337       -4     
+ Misses      12675    12649      -26     
  Partials     1613     1613              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


func IsLegacyPVR(pvr *velerov1api.PodVolumeRestore) bool {
return pvr.Spec.UploaderType == uploader.ResticType
// IsLegacyPVR reports whether the PodVolumeRestore uses the legacy async reconcile path.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This pod_volume_restore_controller_legacy.go file could be fully removed.

log = log.WithField("restore", fmt.Sprintf("%s/%s", pvr.Namespace, pvr.OwnerReferences[0].Name))
}

if !isPVRInFinalState(pvr) && pvr.Spec.UploaderType == uploader.ResticType {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's do this in SetupWithManager and replace IsLegacyPVR with ValidateUploaderType

Comment thread pkg/podvolume/restorer.go

// Intentional: one Restic volume fails the whole pod restore (we do not partially restore Kopia volumes).
for _, info := range volumesToRestore {
if info.repositoryType == velerov1api.BackupRepositoryTypeRestic {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's call ValidateUploaderType to do this check instead

Comment thread pkg/podvolume/util.go
// created with Restic, which Velero no longer supports for file-system backup.
// If any volume for the pod is Restic-backed, the entire pod volume restore is aborted (including
// volumes that used Kopia in the same backup set), so the message reflects whole-pod behavior.
var ErrResticFileSystemBackupUnsupported = errors.New("Restic file-system backup is no longer supported; when any volume for this pod is Restic-backed, the entire pod volume restore is skipped (including non-Restic volumes). Create a new backup with the Kopia uploader and restore from it")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not required if you call ValidateUploaderType

@Lyndon-Li
Copy link
Copy Markdown
Contributor

@testsabirweb Thanks for the contribution. Please resolve the conflicts and also address the comments.

@blackpiglet
Copy link
Copy Markdown
Contributor

blackpiglet commented Apr 16, 2026

@testsabirweb
Given the lack of recent activity on this PR, I was wondering if you would be open to me creating a new PR to resolve this issue?
#9732

@testsabirweb
Copy link
Copy Markdown
Contributor Author

@testsabirweb Given the lack of recent activity on this PR, I was wondering if you would be open to me creating a new PR to resolve this issue? #9732

Hi @blackpiglet ,
Yes, you can work on it.

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.

Remove Restic path for PVR

3 participants