Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/9662-testsabirweb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Issue #9468: Remove Restic path for Pod Volume Restore; Restic file-system restores are no longer supported
44 changes: 0 additions & 44 deletions pkg/cmd/cli/nodeagent/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes"
cacheutil "k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -509,8 +508,6 @@ func (s *nodeAgentServer) run() {
if err := pvrReconciler.AttemptPVRResume(s.ctx, s.logger.WithField("node", s.nodeName), s.namespace); err != nil {
s.logger.WithError(errors.WithStack(err)).Error("Failed to attempt PVR resume")
}

s.markLegacyPVRsFailed(s.mgr.GetClient())
}()

s.logger.Info("Controllers starting...")
Expand Down Expand Up @@ -604,47 +601,6 @@ func (s *nodeAgentServer) validatePodVolumesHostPath(client kubernetes.Interface
return nil
}

func (s *nodeAgentServer) markLegacyPVRsFailed(client ctrlclient.Client) {
pvrs := &velerov1api.PodVolumeRestoreList{}
if err := client.List(s.ctx, pvrs, &ctrlclient.ListOptions{Namespace: s.namespace}); err != nil {
s.logger.WithError(errors.WithStack(err)).Error("failed to list podvolumerestores")
return
}

for i, pvr := range pvrs.Items {
if !controller.IsLegacyPVR(&pvr) {
continue
}

if pvr.Status.Phase != velerov1api.PodVolumeRestorePhaseInProgress {
s.logger.Debugf("the status of podvolumerestore %q is %q, skip", pvr.GetName(), pvr.Status.Phase)
continue
}

pod := &corev1api.Pod{}
if err := client.Get(s.ctx, types.NamespacedName{
Namespace: pvr.Spec.Pod.Namespace,
Name: pvr.Spec.Pod.Name,
}, pod); err != nil {
s.logger.WithError(errors.WithStack(err)).Errorf("failed to get pod \"%s/%s\" of podvolumerestore %q",
pvr.Spec.Pod.Namespace, pvr.Spec.Pod.Name, pvr.GetName())
continue
}
if pod.Spec.NodeName != s.nodeName {
s.logger.Debugf("the node of pod referenced by podvolumerestore %q is %q, not %q, skip", pvr.GetName(), pod.Spec.NodeName, s.nodeName)
continue
}

if err := controller.UpdatePVRStatusToFailed(s.ctx, client, &pvrs.Items[i], errors.New("cannot survive from node-agent restart"),
fmt.Sprintf("get a legacy podvolumerestore with status %q during the server starting, mark it as %q", velerov1api.PodVolumeRestorePhaseInProgress, velerov1api.PodVolumeRestorePhaseFailed),
time.Now(), s.logger); err != nil {
s.logger.WithError(errors.WithStack(err)).Errorf("failed to patch podvolumerestore %q", pvr.GetName())
continue
}
s.logger.WithField("podvolumerestore", pvr.GetName()).Warn(pvr.Status.Message)
}
}

var getConfigsFunc = nodeagent.GetConfigs

func (s *nodeAgentServer) getDataPathConfigs() error {
Expand Down
4 changes: 0 additions & 4 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1164,10 +1164,6 @@ func markPodVolumeRestoresCancel(ctx context.Context, client ctrlclient.Client,

for i := range pvrs.Items {
pvr := pvrs.Items[i]
if controller.IsLegacyPVR(&pvr) {
log.WithField("PVR", pvr.GetName()).Warn("Found a legacy PVR during velero server restart, cannot stop it")
continue
}

if pvr.Status.Phase == velerov1api.PodVolumeRestorePhaseAccepted ||
pvr.Status.Phase == velerov1api.PodVolumeRestorePhasePrepared ||
Expand Down
5 changes: 5 additions & 0 deletions pkg/controller/pod_volume_restore_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/datapath"
"github.com/vmware-tanzu/velero/pkg/exposer"
"github.com/vmware-tanzu/velero/pkg/nodeagent"
"github.com/vmware-tanzu/velero/pkg/podvolume"
repository "github.com/vmware-tanzu/velero/pkg/repository/manager"
"github.com/vmware-tanzu/velero/pkg/restorehelper"
velerotypes "github.com/vmware-tanzu/velero/pkg/types"
Expand Down Expand Up @@ -147,6 +148,10 @@ func (r *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req
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

return r.errorOut(ctx, pvr, podvolume.ErrResticFileSystemBackupUnsupported, podvolume.ErrResticFileSystemBackupUnsupported.Error(), log)
}

// Logic for clear resources when pvr been deleted
if !isPVRInFinalState(pvr) {
if !controllerutil.ContainsFinalizer(pvr, PodVolumeFinalizer) {
Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/pod_volume_restore_controller_legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,8 @@ func (c *PodVolumeRestoreReconcilerLegacy) closeDataPath(ctx context.Context, pv
c.dataPathMgr.RemoveAsyncBR(pvbName)
}

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.

// That path existed only for Restic; Restic file-system restore is no longer supported, so this always returns false.
func IsLegacyPVR(_ *velerov1api.PodVolumeRestore) bool {
return false
}
3 changes: 2 additions & 1 deletion pkg/controller/pod_volume_restore_controller_legacy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,6 @@ func TestFindVolumeRestoresForPodLegacy(t *testing.T) {
},
}).Build()
requests = reconciler.findVolumeRestoresForPod(t.Context(), pod)
assert.Len(t, requests, 1)
// Legacy Restic-only PVR reconciliation is removed; no PVR matches IsLegacyPVR.
assert.Empty(t, requests)
}
7 changes: 7 additions & 0 deletions pkg/controller/pod_volume_restore_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import (
datapathmockes "github.com/vmware-tanzu/velero/pkg/datapath/mocks"
"github.com/vmware-tanzu/velero/pkg/exposer"
exposermockes "github.com/vmware-tanzu/velero/pkg/exposer/mocks"
"github.com/vmware-tanzu/velero/pkg/podvolume"
"github.com/vmware-tanzu/velero/pkg/restorehelper"
"github.com/vmware-tanzu/velero/pkg/test"
velerotest "github.com/vmware-tanzu/velero/pkg/test"
Expand Down Expand Up @@ -700,6 +701,12 @@ func TestPodVolumeRestoreReconcile(t *testing.T) {
pvr: pvrBuilder().Result(),
notCreatePVR: true,
},
{
name: "restic uploader is not supported",
pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, pvrName).UploaderType(uploader.ResticType).Result(),
expected: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, pvrName).UploaderType(uploader.ResticType).Phase(velerov1api.PodVolumeRestorePhaseFailed).Message(podvolume.ErrResticFileSystemBackupUnsupported.Error()).Result(),
expectedErr: podvolume.ErrResticFileSystemBackupUnsupported.Error(),
},
{
name: "pvr not created in velero default namespace",
pvr: builder.ForPodVolumeRestore("test-ns", pvrName).Result(),
Expand Down
14 changes: 8 additions & 6 deletions pkg/podvolume/restorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,13 @@ func (r *restorer) RestorePodVolumes(data RestoreData, tracker *volume.RestoreVo
return nil
}

// 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

return []error{ErrResticFileSystemBackupUnsupported}
}
}

if err := nodeagent.IsRunningOnLinux(r.ctx, r.kubeClient, data.Restore.Namespace); err != nil {
return []error{errors.Wrapf(err, "error to check node agent status")}
}
Expand Down Expand Up @@ -166,11 +173,6 @@ func (r *restorer) RestorePodVolumes(data RestoreData, tracker *volume.RestoreVo
podVolumes[podVolume.Name] = podVolume
}

repoIdentifier := ""
if repositoryType == velerov1api.BackupRepositoryTypeRestic {
repoIdentifier = repo.Spec.ResticIdentifier
}

for volume, backupInfo := range volumesToRestore {
volumeObj, ok := podVolumes[volume]
var pvc *corev1api.PersistentVolumeClaim
Expand All @@ -185,7 +187,7 @@ func (r *restorer) RestorePodVolumes(data RestoreData, tracker *volume.RestoreVo
}
}

volumeRestore := newPodVolumeRestore(data.Restore, data.Pod, data.BackupLocation, volume, backupInfo.snapshotID, backupInfo.snapshotSize, repoIdentifier, backupInfo.uploaderType, data.SourceNamespace, pvc)
volumeRestore := newPodVolumeRestore(data.Restore, data.Pod, data.BackupLocation, volume, backupInfo.snapshotID, backupInfo.snapshotSize, "", backupInfo.uploaderType, data.SourceNamespace, pvc)
if err := veleroclient.CreateRetryGenerateName(r.crClient, r.ctx, volumeRestore); err != nil {
errs = append(errs, errors.WithStack(err))
continue
Expand Down
7 changes: 4 additions & 3 deletions pkg/podvolume/restorer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ func TestRestorePodVolumes(t *testing.T) {
},
},
{
name: "get repository type fail",
// Mixed uploader types: early exit now fails the whole pod restore before getVolumesRepositoryType
// (previously this pair hit "multiple repository type in one backup").
name: "restic backup unsupported",
pvbs: []*velerov1api.PodVolumeBackup{
createPVBObj(true, true, 1, "restic"),
createPVBObj(true, true, 2, "kopia"),
Expand All @@ -217,8 +219,7 @@ func TestRestorePodVolumes(t *testing.T) {
sourceNamespace: "fake-ns",
errs: []expectError{
{
err: "multiple repository type in one backup",
prefixOnly: true,
err: ErrResticFileSystemBackupUnsupported.Error(),
},
},
},
Expand Down
7 changes: 7 additions & 0 deletions pkg/podvolume/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package podvolume

import (
"errors"
"fmt"
"strings"

Expand All @@ -36,6 +37,12 @@
podAnnotationPrefix = "snapshot.velero.io/"
)

// ErrResticFileSystemBackupUnsupported is returned when a restore targets PodVolumeBackups
// 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")

Check failure on line 44 in pkg/podvolume/util.go

View workflow job for this annotation

GitHub Actions / Run Linter Check

ST1005: error strings should not be capitalized (staticcheck)
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


// volumeBackupInfo describes the backup info of a volume backed up by PodVolumeBackups
type volumeBackupInfo struct {
snapshotID string
Expand Down
Loading