From ccfdce30f952ea1d1884f350ad047c3c4d4d2a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?= Date: Thu, 3 Sep 2026 14:56:35 +0800 Subject: [PATCH] Fall back to full restore rather than fail if fail to get the volume ID (#10465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fall back to full restore rather than fail if fail to get the volum e ID Signed-off-by: Wenkai Yin(尹文开) --- pkg/exposer/generic_restore.go | 44 ++-- pkg/exposer/generic_restore_test.go | 333 ++++++++++++++++++++++++++++ pkg/uploader/block/snapshot.go | 3 + pkg/uploader/block/snapshot_test.go | 21 ++ 4 files changed, 382 insertions(+), 19 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 44235ff78..f144a03ae 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -255,27 +255,11 @@ func (e *genericRestoreExposer) Expose(ctx context.Context, ownerObject corev1ap // Get volumeID before creating the restore pod because the existingPV may be deleted when creating the PVC if the volume policy is different var volumeID string if param.CSI != nil && param.CSI.Snapshot != nil { - vs := &snapshotv1api.VolumeSnapshot{} - if err := e.ctrlClient.Get(ctx, client.ObjectKey{ - Namespace: param.CSI.Snapshot.VolumeSnapshotNamespace, - Name: param.CSI.Snapshot.VolumeSnapshot, - }, vs); err != nil { - return errors.Wrapf(err, "error to get volume snapshot %s/%s", param.CSI.Snapshot.VolumeSnapshotNamespace, param.CSI.Snapshot.VolumeSnapshot) - } - - var vsc *snapshotv1api.VolumeSnapshotContent - vsc, err = csi.GetVSCForVS(ctx, vs, e.ctrlClient) + volumeID, err = e.getVolumeID(ctx, param.CSI.Snapshot, param.TargetPVName) if err != nil { - return errors.Wrapf(err, "error to get volume snapshot content for volume snapshot %s/%s", vs.Namespace, vs.Name) + // only log the error. Without the volume ID, exposer will fallback to full restore. + curLog.Errorf("failed to get volume ID from snapshot %s/%s, err: %v", param.CSI.Snapshot.VolumeSnapshotNamespace, param.CSI.Snapshot.VolumeSnapshot, err) } - - var cbtInfo csi.CBTInfo - cbtInfo, err = csi.GetCBTInfo(ctx, e.kubeClient, e.log, vs, vsc, param.TargetPVName) - if err != nil { - return errors.Wrap(err, "error to get CBT info") - } - curLog.Debugf("CBT info: %+v", cbtInfo) - volumeID = cbtInfo.VolumeID } curLog.Info("Creating restore PVC") @@ -1082,3 +1066,25 @@ func (e *genericRestoreExposer) validateSelectedNode(ctx context.Context, node s return true } + +func (e *genericRestoreExposer) getVolumeID(ctx context.Context, snapshot *velerov2alpha1api.CSISnapshotSpec, targetPVName string) (string, error) { + vs := &snapshotv1api.VolumeSnapshot{} + if err := e.ctrlClient.Get(ctx, client.ObjectKey{ + Namespace: snapshot.VolumeSnapshotNamespace, + Name: snapshot.VolumeSnapshot, + }, vs); err != nil { + return "", errors.Wrapf(err, "error to get volume snapshot %s/%s", snapshot.VolumeSnapshotNamespace, snapshot.VolumeSnapshot) + } + + vsc, err := csi.GetVSCForVS(ctx, vs, e.ctrlClient) + if err != nil { + return "", errors.Wrapf(err, "error to get volume snapshot content for volume snapshot %s/%s", vs.Namespace, vs.Name) + } + + var cbtInfo csi.CBTInfo + cbtInfo, err = csi.GetCBTInfo(ctx, e.kubeClient, e.log, vs, vsc, targetPVName) + if err != nil { + return "", errors.Wrap(err, "error to get CBT info") + } + return cbtInfo.VolumeID, nil +} diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index abeb17f40..ff7fb126e 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -21,6 +21,7 @@ import ( "time" "github.com/cockroachdb/errors" + snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" appsv1api "k8s.io/api/apps/v1" @@ -33,8 +34,10 @@ import ( clientTesting "k8s.io/client-go/testing" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + velerov2alpha1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1" velerotest "github.com/vmware-tanzu/velero/pkg/test" velerotypes "github.com/vmware-tanzu/velero/pkg/types" + "github.com/vmware-tanzu/velero/pkg/util" "github.com/vmware-tanzu/velero/pkg/util/datamover" "github.com/vmware-tanzu/velero/pkg/util/kube" ) @@ -716,6 +719,336 @@ func TestRestoreExpose_SecretCopy(t *testing.T) { }) } +func TestGetVolumeID(t *testing.T) { + vscName := "fake-vsc" + snapshotHandle := "fake-snapshot-handle" + + tests := []struct { + name string + snapshot *velerov2alpha1api.CSISnapshotSpec + targetPVName string + ctrlClientObj []runtime.Object + kubeClientObj []runtime.Object + expectedID string + expectedErr string + }{ + { + name: "VS not found in ctrlClient", + snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "non-existent-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + expectedErr: "error to get volume snapshot fake-ns/non-existent-vs", + }, + { + name: "GetVSCForVS error - VS has no bound VSC", + snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "fake-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + ctrlClientObj: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-vs", + }, + Status: nil, + }, + }, + expectedErr: "error to get volume snapshot content for volume snapshot fake-ns/fake-vs: invalid snapshot info in volume snapshot fake-vs", + }, + { + name: "GetVSCForVS error - VSC not found in ctrlClient", + snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "fake-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + ctrlClientObj: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-vs", + }, + Status: &snapshotv1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &vscName, + }, + }, + }, + expectedErr: "error to get volume snapshot content for volume snapshot fake-ns/fake-vs: error getting volume snapshot content from API", + }, + { + name: "GetCBTInfo error - target PV not found", + snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "fake-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + targetPVName: "missing-pv", + ctrlClientObj: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-vs", + }, + Status: &snapshotv1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &vscName, + }, + }, + &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: vscName, + }, + Status: &snapshotv1api.VolumeSnapshotContentStatus{ + SnapshotHandle: &snapshotHandle, + }, + }, + }, + expectedErr: "error to get CBT info: failed to get pv missing-pv", + }, + { + name: "GetCBTInfo error - empty volumeID on PV", + snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "fake-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + targetPVName: "fake-pv", + ctrlClientObj: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-vs", + }, + Status: &snapshotv1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &vscName, + }, + }, + &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: vscName, + }, + Status: &snapshotv1api.VolumeSnapshotContentStatus{ + SnapshotHandle: &snapshotHandle, + }, + }, + }, + kubeClientObj: []runtime.Object{ + &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-pv", + }, + }, + }, + expectedErr: "error to get CBT info: volumeID must not be empty for CBT", + }, + { + name: "success with VKS annotations", + snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "fake-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + ctrlClientObj: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-vs", + Annotations: map[string]string{ + util.VSphereCNSChangeIDAnno: "c-1", + util.VSphereCNSSnapshotAnno: "vol-vks+snap-1", + }, + }, + Status: &snapshotv1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &vscName, + }, + }, + &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: vscName, + }, + }, + }, + expectedID: "vol-vks", + }, + { + name: "success with PV CSI volume handle", + snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "fake-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + targetPVName: "fake-pv", + ctrlClientObj: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-vs", + }, + Status: &snapshotv1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &vscName, + }, + }, + &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: vscName, + }, + Status: &snapshotv1api.VolumeSnapshotContentStatus{ + SnapshotHandle: &snapshotHandle, + }, + }, + }, + kubeClientObj: []runtime.Object{ + &corev1api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-pv", + }, + Spec: corev1api.PersistentVolumeSpec{ + PersistentVolumeSource: corev1api.PersistentVolumeSource{ + CSI: &corev1api.CSIPersistentVolumeSource{ + VolumeHandle: "csi-vol-789", + }, + }, + }, + }, + }, + expectedID: "csi-vol-789", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset(test.kubeClientObj...) + fakeCtrlClient := velerotest.NewFakeControllerRuntimeClient(t, test.ctrlClientObj...) + + exposer := genericRestoreExposer{ + kubeClient: fakeKubeClient, + ctrlClient: fakeCtrlClient, + log: velerotest.NewLogger(), + } + + volID, err := exposer.getVolumeID(t.Context(), test.snapshot, test.targetPVName) + if test.expectedErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), test.expectedErr) + assert.Empty(t, volID) + } else { + require.NoError(t, err) + assert.Equal(t, test.expectedID, volID) + } + }) + } +} + +func TestRestoreExpose_CSISnapshot(t *testing.T) { + scName := "fake-sc" + restore := &velerov1.Restore{ + TypeMeta: metav1.TypeMeta{APIVersion: velerov1.SchemeGroupVersion.String(), Kind: "Restore"}, + ObjectMeta: metav1.ObjectMeta{Namespace: velerov1.DefaultNamespace, Name: "fake-restore", UID: "fake-uid"}, + } + ownerObject := corev1api.ObjectReference{ + Kind: restore.Kind, + Namespace: restore.Namespace, + Name: restore.Name, + UID: restore.UID, + APIVersion: restore.APIVersion, + } + targetPVCObj := &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Namespace: "fake-ns", Name: "fake-target-pvc"}, + Spec: corev1api.PersistentVolumeClaimSpec{StorageClassName: &scName}, + } + storageClass := &storagev1api.StorageClass{ObjectMeta: metav1.ObjectMeta{Name: "fake-sc"}} + daemonSet := &appsv1api.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{Namespace: "velero", Name: "node-agent"}, + TypeMeta: metav1.TypeMeta{Kind: "DaemonSet", APIVersion: appsv1api.SchemeGroupVersion.String()}, + Spec: appsv1api.DaemonSetSpec{ + Template: corev1api.PodTemplateSpec{ + Spec: corev1api.PodSpec{Containers: []corev1api.Container{{Image: "fake-image"}}}, + }, + }, + } + + vscName := "fake-vsc" + + t.Run("getVolumeID fails - falls back to full restore and creates pod without volume ID", func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset(targetPVCObj, storageClass, daemonSet) + fakeCtrlClient := velerotest.NewFakeControllerRuntimeClient(t) + exposer := genericRestoreExposer{ + kubeClient: fakeKubeClient, + ctrlClient: fakeCtrlClient, + log: velerotest.NewLogger(), + } + + err := exposer.Expose(t.Context(), ownerObject, GenericRestoreExposeParam{ + TargetPVCName: "fake-target-pvc", + TargetNamespace: "fake-ns", + HostingPodLabels: map[string]string{}, + Resources: corev1api.ResourceRequirements{}, + ExposeTimeout: time.Millisecond, + CSI: &GenericRestoreExposeCSI{ + Snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "non-existent-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + }, + }) + require.NoError(t, err) + + pod, err := fakeKubeClient.CoreV1().Pods(ownerObject.Namespace).Get(t.Context(), ownerObject.Name, metav1.GetOptions{}) + require.NoError(t, err) + require.Len(t, pod.Spec.Containers, 1) + for _, arg := range pod.Spec.Containers[0].Args { + assert.NotContains(t, arg, "--volume-id=") + assert.NotContains(t, arg, "--vs-namespace=") + } + }) + + t.Run("getVolumeID succeeds - passes volume ID to restore pod", func(t *testing.T) { + fakeKubeClient := fake.NewSimpleClientset(targetPVCObj, storageClass, daemonSet) + fakeCtrlClient := velerotest.NewFakeControllerRuntimeClient(t, + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-vs", + Annotations: map[string]string{ + util.VSphereCNSChangeIDAnno: "c-1", + util.VSphereCNSSnapshotAnno: "vol-123+snap-1", + }, + }, + Status: &snapshotv1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &vscName, + }, + }, + &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: vscName, + }, + }, + ) + exposer := genericRestoreExposer{ + kubeClient: fakeKubeClient, + ctrlClient: fakeCtrlClient, + log: velerotest.NewLogger(), + } + + err := exposer.Expose(t.Context(), ownerObject, GenericRestoreExposeParam{ + TargetPVCName: "fake-target-pvc", + TargetNamespace: "fake-ns", + HostingPodLabels: map[string]string{}, + Resources: corev1api.ResourceRequirements{}, + ExposeTimeout: time.Millisecond, + CSI: &GenericRestoreExposeCSI{ + Snapshot: &velerov2alpha1api.CSISnapshotSpec{ + VolumeSnapshot: "fake-vs", + VolumeSnapshotNamespace: "fake-ns", + }, + }, + }) + require.NoError(t, err) + + pod, err := fakeKubeClient.CoreV1().Pods(ownerObject.Namespace).Get(t.Context(), ownerObject.Name, metav1.GetOptions{}) + require.NoError(t, err) + require.Len(t, pod.Spec.Containers, 1) + assert.Contains(t, pod.Spec.Containers[0].Args, "--volume-id=vol-123") + assert.Contains(t, pod.Spec.Containers[0].Args, "--vs-namespace=fake-ns") + }) +} + func TestRebindVolume(t *testing.T) { restore := &velerov1.Restore{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/uploader/block/snapshot.go b/pkg/uploader/block/snapshot.go index 595a80fd5..566ebad4f 100644 --- a/pkg/uploader/block/snapshot.go +++ b/pkg/uploader/block/snapshot.go @@ -225,6 +225,9 @@ func Restore(ctx context.Context, blkUp Uploader, rep udmrepo.BackupRepo, snapsh } else if snapshot.Tags[uploader.CBTVolumeIDTag] == "" { log.Warnf("No VolumeID tag from snapshot %s, fallback to full restore", snapshotID) incremental = false + } else if cbtSource.VolumeID == "" { + log.Warnf("No VolumeID in cbt source %v, fallback to full restore", cbtSource) + incremental = false } else if snapshot.Tags[uploader.CBTVolumeIDTag] != cbtSource.VolumeID { log.Warnf("VolumeID %s from snapshot %s is not expected as %s, fallback to full restore", snapshot.Tags[uploader.CBTVolumeIDTag], snapshotID, cbtSource.VolumeID) incremental = false diff --git a/pkg/uploader/block/snapshot_test.go b/pkg/uploader/block/snapshot_test.go index 5a18c376e..ae95f12ef 100644 --- a/pkg/uploader/block/snapshot_test.go +++ b/pkg/uploader/block/snapshot_test.go @@ -766,6 +766,27 @@ func TestRestore(t *testing.T) { }, expectedSize: 4096, }, + { + name: "incremental restore fallback - empty cbtSource VolumeID", + incremental: true, + cbtSource: cbtservice.SourceInfo{Snapshot: "snap-cbt", VolumeID: ""}, + setupMocks: func(blkup *mockUploader, repo *udmrepomocks.BackupRepo) { + snapWithTags := udmrepo.Snapshot{ + Tags: map[string]string{ + uploader.CBTChangeIDTag: "cid-1", + uploader.CBTVolumeIDTag: "vol-1", + }, + } + repo.On("GetSnapshot", mock.Anything, udmrepo.ID("snap-001")).Return(snapWithTags, nil) + blkup.On("Restore", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(int64(4096), int64(4096), nil) + }, + setupOpenDev: func(t *testing.T) *os.File { + t.Helper() + return tempFile(t, "") + }, + expectedSize: 4096, + }, { name: "incremental restore fallback - VolumeID mismatch", incremental: true,