diff --git a/changelogs/unreleased/9687-shubham-pampattiwar b/changelogs/unreleased/9687-shubham-pampattiwar new file mode 100644 index 000000000..14f518c41 --- /dev/null +++ b/changelogs/unreleased/9687-shubham-pampattiwar @@ -0,0 +1 @@ +Fix VolumeGroupSnapshot restore failure with Ceph RBD CSI driver by creating stub VolumeGroupSnapshotContent during restore and looking up VolumeSnapshotClass by driver for credential support diff --git a/internal/volume/volumes_information.go b/internal/volume/volumes_information.go index 463b81f46..4d5961bdb 100644 --- a/internal/volume/volumes_information.go +++ b/internal/volume/volumes_information.go @@ -146,6 +146,10 @@ type CSISnapshotInfo struct { // The VolumeSnapshot's Status.ReadyToUse value ReadyToUse *bool + + // The VolumeGroupSnapshotHandle from VSC status, used to create stub VGSC during restore + // for CSI drivers that populate this field (e.g., Ceph RBD). + VolumeGroupSnapshotHandle string `json:"volumeGroupSnapshotHandle,omitempty"` } // SnapshotDataMovementInfo is used for displaying the snapshot data mover status. @@ -456,6 +460,10 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() { if volumeSnapshotContent.Status.SnapshotHandle != nil { snapshotHandle = *volumeSnapshotContent.Status.SnapshotHandle } + volumeGroupSnapshotHandle := "" + if volumeSnapshotContent.Status != nil && volumeSnapshotContent.Status.VolumeGroupSnapshotHandle != nil { + volumeGroupSnapshotHandle = *volumeSnapshotContent.Status.VolumeGroupSnapshotHandle + } if pvcPVInfo := v.pvMap.retrieve("", *volumeSnapshot.Spec.Source.PersistentVolumeClaimName, volumeSnapshot.Namespace); pvcPVInfo != nil { volumeInfo := &BackupVolumeInfo{ BackupMethod: CSISnapshot, @@ -466,12 +474,13 @@ func (v *BackupVolumesInformation) generateVolumeInfoForCSIVolumeSnapshot() { SnapshotDataMoved: false, PreserveLocalSnapshot: true, CSISnapshotInfo: &CSISnapshotInfo{ - VSCName: *volumeSnapshot.Status.BoundVolumeSnapshotContentName, - Size: size, - Driver: volumeSnapshotContent.Spec.Driver, - SnapshotHandle: snapshotHandle, - OperationID: operation.Spec.OperationID, - ReadyToUse: volumeSnapshot.Status.ReadyToUse, + VSCName: *volumeSnapshot.Status.BoundVolumeSnapshotContentName, + Size: size, + Driver: volumeSnapshotContent.Spec.Driver, + SnapshotHandle: snapshotHandle, + OperationID: operation.Spec.OperationID, + ReadyToUse: volumeSnapshot.Status.ReadyToUse, + VolumeGroupSnapshotHandle: volumeGroupSnapshotHandle, }, PVInfo: &PVInfo{ ReclaimPolicy: string(pvcPVInfo.PV.Spec.PersistentVolumeReclaimPolicy), diff --git a/pkg/apis/velero/v1/labels_annotations.go b/pkg/apis/velero/v1/labels_annotations.go index 85d8b05aa..921af498e 100644 --- a/pkg/apis/velero/v1/labels_annotations.go +++ b/pkg/apis/velero/v1/labels_annotations.go @@ -141,6 +141,7 @@ const ( VolumeSnapshotRestoreSize = "velero.io/csi-volumesnapshot-restore-size" DriverNameAnnotation = "velero.io/csi-driver-name" VSCDeletionPolicyAnnotation = "velero.io/csi-vsc-deletion-policy" + VolumeGroupSnapshotHandleAnnotation = "velero.io/csi-volumegroupsnapshot-handle" VolumeSnapshotClassSelectorLabel = "velero.io/csi-volumesnapshot-class" VolumeSnapshotClassDriverBackupAnnotationPrefix = "velero.io/csi-volumesnapshot-class" VolumeSnapshotClassDriverPVCAnnotation = "velero.io/csi-volumesnapshot-class" diff --git a/pkg/backup/actions/csi/volumesnapshot_action.go b/pkg/backup/actions/csi/volumesnapshot_action.go index b7283b628..0e0e9a840 100644 --- a/pkg/backup/actions/csi/volumesnapshot_action.go +++ b/pkg/backup/actions/csi/volumesnapshot_action.go @@ -151,6 +151,12 @@ func (p *volumeSnapshotBackupItemAction) Execute( annotations[velerov1api.VolumeSnapshotRestoreSize] = resource.NewQuantity( *vsc.Status.RestoreSize, resource.BinarySI).String() } + + // Capture VolumeGroupSnapshotHandle to create stub VGSC during restore + // for CSI drivers that populate this field (e.g., Ceph RBD). + if vsc.Status.VolumeGroupSnapshotHandle != nil { + annotations[velerov1api.VolumeGroupSnapshotHandleAnnotation] = *vsc.Status.VolumeGroupSnapshotHandle + } } p.log.Infof("Patching VolumeSnapshotContent %s with velero BackupNameLabel", diff --git a/pkg/controller/restore_finalizer_controller.go b/pkg/controller/restore_finalizer_controller.go index 0061bdf45..6ff7f8cb0 100644 --- a/pkg/controller/restore_finalizer_controller.go +++ b/pkg/controller/restore_finalizer_controller.go @@ -22,6 +22,8 @@ import ( "sync" "time" + volumegroupsnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1beta1" + snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" corev1api "k8s.io/api/core/v1" @@ -43,6 +45,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/persistence" "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt" "github.com/vmware-tanzu/velero/pkg/plugin/velero" + "github.com/vmware-tanzu/velero/pkg/util/boolptr" kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube" "github.com/vmware-tanzu/velero/pkg/util/results" ) @@ -291,13 +294,16 @@ type finalizerContext struct { resourceTimeout time.Duration } -func (ctx *finalizerContext) execute() (results.Result, results.Result) { //nolint:unparam //temporarily ignore the lint report: result 0 is always nil (unparam) +func (ctx *finalizerContext) execute() (results.Result, results.Result) { warnings, errs := results.Result{}, results.Result{} // implement finalization tasks pdpErrs := ctx.patchDynamicPVWithVolumeInfo() errs.Merge(&pdpErrs) + vgscWarnings := ctx.cleanupStubVGSC() + warnings.Merge(&vgscWarnings) + rehErrs := ctx.WaitRestoreExecHook() errs.Merge(&rehErrs) @@ -443,6 +449,93 @@ func (ctx *finalizerContext) patchDynamicPVWithVolumeInfo() (errs results.Result return errs } +// cleanupStubVGSC deletes stub VolumeGroupSnapshotContent objects that were +// created during restore to satisfy CSI controller validation. These stubs are +// labeled with velero.io/restore-name for identification. +// Before deleting each VGSC, it waits for all related VolumeSnapshotContents +// to become ReadyToUse, since the CSI controller needs the VGSC during VSC reconciliation. +func (ctx *finalizerContext) cleanupStubVGSC() (warnings results.Result) { + ctx.logger.Info("cleaning up stub VolumeGroupSnapshotContents") + + vgscList := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentList{} + err := ctx.crClient.List( + context.Background(), + vgscList, + client.MatchingLabels{velerov1api.RestoreNameLabel: ctx.restore.Name}, + ) + if err != nil { + // If the CRD is not installed, listing will fail. This is expected + // on clusters without VolumeGroupSnapshot support, so treat as warning. + ctx.logger.WithError(err).Warn("failed to list stub VolumeGroupSnapshotContents, skipping cleanup") + warnings.Add("cluster", errors.Wrap(err, "failed to list stub VolumeGroupSnapshotContents")) + return warnings + } + + if len(vgscList.Items) == 0 { + ctx.logger.Info("no stub VolumeGroupSnapshotContents to clean up") + return warnings + } + + for i := range vgscList.Items { + vgsc := &vgscList.Items[i] + log := ctx.logger.WithField("vgsc", vgsc.Name) + + // Collect the snapshot handles associated with this VGSC + snapshotHandles := map[string]bool{} + if vgsc.Spec.Source.GroupSnapshotHandles != nil { + for _, h := range vgsc.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles { + snapshotHandles[h] = true + } + } + + if len(snapshotHandles) > 0 { + // Wait for related VSCs to become ReadyToUse before deleting the VGSC + log.Infof("waiting for %d related VolumeSnapshotContents to become ReadyToUse", len(snapshotHandles)) + err := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, ctx.resourceTimeout, true, func(context.Context) (bool, error) { + vscList := &snapshotv1api.VolumeSnapshotContentList{} + if err := ctx.crClient.List(context.Background(), vscList, client.MatchingLabels{velerov1api.RestoreNameLabel: ctx.restore.Name}); err != nil { + log.WithError(err).Warn("failed to list VolumeSnapshotContents") + return false, nil + } + + for j := range vscList.Items { + vsc := &vscList.Items[j] + if vsc.Spec.Source.SnapshotHandle == nil { + continue + } + if !snapshotHandles[*vsc.Spec.Source.SnapshotHandle] { + continue + } + // This VSC is related to our VGSC + if vsc.Status == nil || !boolptr.IsSetToTrue(vsc.Status.ReadyToUse) { + log.Debugf("VolumeSnapshotContent %s not yet ReadyToUse", vsc.Name) + return false, nil + } + } + return true, nil + }) + if err != nil { + log.WithError(err).Warn("timed out waiting for related VolumeSnapshotContents to become ReadyToUse, proceeding with VGSC deletion") + warnings.Add("cluster", errors.Wrapf(err, "timed out waiting for VSCs related to VGSC %s", vgsc.Name)) + } + } + + log.Info("deleting stub VolumeGroupSnapshotContent") + if err := ctx.crClient.Delete(context.Background(), vgsc); err != nil { + if apierrors.IsNotFound(err) { + log.Info("stub VolumeGroupSnapshotContent already deleted") + continue + } + log.WithError(err).Warn("failed to delete stub VolumeGroupSnapshotContent") + warnings.Add("cluster", errors.Wrapf(err, "failed to delete stub VolumeGroupSnapshotContent %s", vgsc.Name)) + } else { + log.Info("deleted stub VolumeGroupSnapshotContent") + } + } + + return warnings +} + func needPatch(newPV *corev1api.PersistentVolume, pvInfo *volume.PVInfo) bool { if newPV.Spec.PersistentVolumeReclaimPolicy != corev1api.PersistentVolumeReclaimPolicy(pvInfo.ReclaimPolicy) { return true diff --git a/pkg/controller/restore_finalizer_controller_test.go b/pkg/controller/restore_finalizer_controller_test.go index 56366a360..0f1cc340d 100644 --- a/pkg/controller/restore_finalizer_controller_test.go +++ b/pkg/controller/restore_finalizer_controller_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + volumegroupsnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1beta1" + snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -45,6 +47,7 @@ import ( pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks" "github.com/vmware-tanzu/velero/pkg/plugin/velero" velerotest "github.com/vmware-tanzu/velero/pkg/test" + "github.com/vmware-tanzu/velero/pkg/util/boolptr" pkgUtilKubeMocks "github.com/vmware-tanzu/velero/pkg/util/kube/mocks" "github.com/vmware-tanzu/velero/pkg/util/results" ) @@ -739,3 +742,253 @@ func TestRestoreOperationList(t *testing.T) { }) } } + +func TestCleanupStubVGSC(t *testing.T) { + snapshotHandle1 := "snap-handle-1" + snapshotHandle2 := "snap-handle-2" + + tests := []struct { + name string + restore *velerov1api.Restore + existingVGSCs []*volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent + existingVSCs []*snapshotv1api.VolumeSnapshotContent + expectedRemaining int + expectedWarnings bool + }{ + { + name: "no stub VGSCs to clean up", + restore: builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Result(), + existingVGSCs: nil, + expectedRemaining: 0, + expectedWarnings: false, + }, + { + name: "single stub VGSC deleted after VSCs are ready", + restore: builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Result(), + existingVGSCs: []*volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vgsc-stub-1", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{ + GroupSnapshotHandles: &volumegroupsnapshotv1beta1.GroupSnapshotHandles{ + VolumeGroupSnapshotHandle: "vgs-handle-1", + VolumeSnapshotHandles: []string{snapshotHandle1}, + }, + }, + }, + }, + }, + existingVSCs: []*snapshotv1api.VolumeSnapshotContent{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vsc-1", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + Source: snapshotv1api.VolumeSnapshotContentSource{ + SnapshotHandle: &snapshotHandle1, + }, + VolumeSnapshotRef: corev1api.ObjectReference{ + Name: "vs-1", + Namespace: "ns-1", + }, + }, + Status: &snapshotv1api.VolumeSnapshotContentStatus{ + ReadyToUse: boolptr.True(), + }, + }, + }, + expectedRemaining: 0, + expectedWarnings: false, + }, + { + name: "multiple stub VGSCs deleted", + restore: builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Result(), + existingVGSCs: []*volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vgsc-stub-1", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{ + GroupSnapshotHandles: &volumegroupsnapshotv1beta1.GroupSnapshotHandles{ + VolumeGroupSnapshotHandle: "vgs-handle-1", + VolumeSnapshotHandles: []string{snapshotHandle1}, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vgsc-stub-2", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{ + GroupSnapshotHandles: &volumegroupsnapshotv1beta1.GroupSnapshotHandles{ + VolumeGroupSnapshotHandle: "vgs-handle-2", + VolumeSnapshotHandles: []string{snapshotHandle2}, + }, + }, + }, + }, + }, + existingVSCs: []*snapshotv1api.VolumeSnapshotContent{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vsc-1", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + Source: snapshotv1api.VolumeSnapshotContentSource{ + SnapshotHandle: &snapshotHandle1, + }, + VolumeSnapshotRef: corev1api.ObjectReference{ + Name: "vs-1", + Namespace: "ns-1", + }, + }, + Status: &snapshotv1api.VolumeSnapshotContentStatus{ + ReadyToUse: boolptr.True(), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vsc-2", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + Source: snapshotv1api.VolumeSnapshotContentSource{ + SnapshotHandle: &snapshotHandle2, + }, + VolumeSnapshotRef: corev1api.ObjectReference{ + Name: "vs-2", + Namespace: "ns-1", + }, + }, + Status: &snapshotv1api.VolumeSnapshotContentStatus{ + ReadyToUse: boolptr.True(), + }, + }, + }, + expectedRemaining: 0, + expectedWarnings: false, + }, + { + name: "VGSCs from different restore are not deleted", + restore: builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Result(), + existingVGSCs: []*volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vgsc-stub-mine", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vgsc-stub-other", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-2", + }, + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{}, + }, + }, + }, + expectedRemaining: 1, + expectedWarnings: false, + }, + { + name: "VGSC deleted even when no snapshot handles in spec", + restore: builder.ForRestore(velerov1api.DefaultNamespace, "restore-1").Result(), + existingVGSCs: []*volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vgsc-stub-empty", + Labels: map[string]string{ + velerov1api.RestoreNameLabel: "restore-1", + }, + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: "rbd.csi.ceph.com", + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{}, + }, + }, + }, + expectedRemaining: 0, + expectedWarnings: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeClient := velerotest.NewFakeControllerRuntimeClientBuilder(t).Build() + logger := velerotest.NewLogger() + + ctx := &finalizerContext{ + logger: logger, + crClient: fakeClient, + restore: tc.restore, + resourceTimeout: 10 * time.Second, + } + + for _, vgsc := range tc.existingVGSCs { + require.NoError(t, fakeClient.Create(t.Context(), vgsc)) + } + for _, vsc := range tc.existingVSCs { + require.NoError(t, fakeClient.Create(t.Context(), vsc)) + } + + warnings := ctx.cleanupStubVGSC() + + if tc.expectedWarnings { + assert.False(t, warnings.IsEmpty()) + } else { + assert.True(t, warnings.IsEmpty(), "expected no warnings") + } + + remainingList := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentList{} + require.NoError(t, fakeClient.List(t.Context(), remainingList)) + assert.Len(t, remainingList.Items, tc.expectedRemaining) + + // Verify remaining VGSCs don't belong to this restore + for _, remaining := range remainingList.Items { + assert.NotEqual(t, tc.restore.Name, remaining.Labels[velerov1api.RestoreNameLabel], + "VGSC %s should have been deleted", remaining.Name) + } + }) + } +} diff --git a/pkg/restore/actions/csi/volumesnapshot_action.go b/pkg/restore/actions/csi/volumesnapshot_action.go index 6d4bc1eda..708b40681 100644 --- a/pkg/restore/actions/csi/volumesnapshot_action.go +++ b/pkg/restore/actions/csi/volumesnapshot_action.go @@ -17,11 +17,16 @@ limitations under the License. package csi import ( + "context" "fmt" + volumegroupsnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1beta1" snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" + corev1api "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -65,6 +70,165 @@ func resetVolumeSnapshotAnnotation(vs *snapshotv1api.VolumeSnapshot) { string(snapshotv1api.VolumeSnapshotContentRetain) } +// ensureStubVGSCExists creates a stub VolumeGroupSnapshotContent if the snapshot +// was created as part of a VolumeGroupSnapshot. This is needed for CSI drivers +// like Ceph RBD that populate volumeGroupSnapshotHandle on pre-provisioned snapshots. +// The CSI snapshot controller requires a VGSC with matching handle to exist. +func (p *volumeSnapshotRestoreItemAction) ensureStubVGSCExists( + ctx context.Context, + vs *snapshotv1api.VolumeSnapshot, + restore *velerov1api.Restore, +) error { + vgsh, ok := vs.Annotations[velerov1api.VolumeGroupSnapshotHandleAnnotation] + if !ok || vgsh == "" { + // No VolumeGroupSnapshotHandle, nothing to do + return nil + } + + snapshotHandle, ok := vs.Annotations[velerov1api.VolumeSnapshotHandleAnnotation] + if !ok || snapshotHandle == "" { + p.log.Warnf("VS %s/%s has VolumeGroupSnapshotHandle but no SnapshotHandle annotation", + vs.Namespace, vs.Name) + return nil + } + + driver, ok := vs.Annotations[velerov1api.DriverNameAnnotation] + if !ok || driver == "" { + p.log.Warnf("VS %s/%s has VolumeGroupSnapshotHandle but no Driver annotation", + vs.Namespace, vs.Name) + return nil + } + + // Generate a deterministic name for the stub VGSC based on the group handle + vgscName := util.GenerateSha256FromRestoreUIDAndVsName(string(restore.UID), vgsh) + + // Check if VGSC already exists + existingVGSC := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{} + err := p.crClient.Get(ctx, crclient.ObjectKey{Name: vgscName}, existingVGSC) + if err == nil { + // VGSC already exists, add this snapshot handle if not already present + p.log.Infof("Stub VGSC %s already exists for VolumeGroupSnapshotHandle %s", vgscName, vgsh) + return p.addSnapshotHandleToVGSC(ctx, existingVGSC, snapshotHandle) + } + if !apierrors.IsNotFound(err) { + return errors.Wrapf(err, "failed to check for existing VGSC %s", vgscName) + } + + // Create stub VGSC + p.log.Infof("Creating stub VGSC %s for VolumeGroupSnapshotHandle %s", vgscName, vgsh) + + // Look up VolumeGroupSnapshotClass to get secret annotations + vgscAnnotations := map[string]string{} + vgscList := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotClassList{} + if err := p.crClient.List(ctx, vgscList); err == nil { + for _, vgsClass := range vgscList.Items { + if vgsClass.Driver == driver { + // Found matching class, extract secret parameters + if secretName, ok := vgsClass.Parameters["csi.storage.k8s.io/group-snapshotter-secret-name"]; ok { + vgscAnnotations["groupsnapshot.storage.kubernetes.io/deletion-secret-name"] = secretName + } + if secretNS, ok := vgsClass.Parameters["csi.storage.k8s.io/group-snapshotter-secret-namespace"]; ok { + vgscAnnotations["groupsnapshot.storage.kubernetes.io/deletion-secret-namespace"] = secretNS + } + break + } + } + } + + vgsc := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: vgscName, + Labels: map[string]string{ + velerov1api.RestoreNameLabel: restore.Name, + }, + Annotations: vgscAnnotations, + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + Driver: driver, + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{ + GroupSnapshotHandles: &volumegroupsnapshotv1beta1.GroupSnapshotHandles{ + VolumeGroupSnapshotHandle: vgsh, + VolumeSnapshotHandles: []string{snapshotHandle}, + }, + }, + VolumeGroupSnapshotRef: corev1api.ObjectReference{ + Name: "stub-vgs-" + vgscName[:8], + Namespace: vs.Namespace, + }, + }, + } + + if err := p.crClient.Create(ctx, vgsc); err != nil { + if apierrors.IsAlreadyExists(err) { + // Another VS restore created the VGSC between our Get and Create. + // Re-fetch and add our snapshot handle. + p.log.Infof("Stub VGSC %s was created by another VS restore, adding our handle", vgscName) + raceVGSC := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{} + if getErr := p.crClient.Get(ctx, crclient.ObjectKey{Name: vgscName}, raceVGSC); getErr != nil { + return errors.Wrapf(getErr, "failed to get VGSC %s after race", vgscName) + } + return p.addSnapshotHandleToVGSC(ctx, raceVGSC, snapshotHandle) + } + return errors.Wrapf(err, "failed to create stub VGSC %s", vgscName) + } + + // Re-fetch to get server-assigned metadata (resourceVersion) needed for patching + createdVGSC := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{} + if err := p.crClient.Get(ctx, crclient.ObjectKey{Name: vgscName}, createdVGSC); err != nil { + p.log.Warnf("Failed to fetch stub VGSC %s for status patch: %v", vgscName, err) + return nil + } + + // Set volumeGroupSnapshotHandle in status using Patch to avoid conflicts with the CSI controller. + patchBase := createdVGSC.DeepCopy() + if createdVGSC.Status == nil { + createdVGSC.Status = &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentStatus{} + } + createdVGSC.Status.VolumeGroupSnapshotHandle = &vgsh + if err := p.crClient.Status().Patch(ctx, createdVGSC, crclient.MergeFrom(patchBase)); err != nil { + p.log.Warnf("Failed to patch stub VGSC %s status: %v", vgscName, err) + } + + p.log.Infof("Successfully created stub VGSC %s", vgscName) + return nil +} + +// addSnapshotHandleToVGSC adds a snapshot handle to an existing VGSC if not already present. +// This is needed when multiple VolumeSnapshots from the same VolumeGroupSnapshot are restored. +func (p *volumeSnapshotRestoreItemAction) addSnapshotHandleToVGSC( + ctx context.Context, + vgsc *volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent, + snapshotHandle string, +) error { + // Check if handle is already in the list + if vgsc.Spec.Source.GroupSnapshotHandles != nil { + for _, handle := range vgsc.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles { + if handle == snapshotHandle { + p.log.Infof("Snapshot handle %s already present in VGSC %s", snapshotHandle, vgsc.Name) + return nil + } + } + } + + // Add the snapshot handle to the list + patchBase := vgsc.DeepCopy() + if vgsc.Spec.Source.GroupSnapshotHandles == nil { + vgsc.Spec.Source.GroupSnapshotHandles = &volumegroupsnapshotv1beta1.GroupSnapshotHandles{} + } + vgsc.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles = append( + vgsc.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles, + snapshotHandle, + ) + + if err := p.crClient.Patch(ctx, vgsc, crclient.MergeFrom(patchBase)); err != nil { + return errors.Wrapf(err, "failed to add snapshot handle to VGSC %s", vgsc.Name) + } + + p.log.Infof("Added snapshot handle %s to existing VGSC %s", snapshotHandle, vgsc.Name) + return nil +} + func (p *volumeSnapshotRestoreItemAction) Execute( input *velero.RestoreItemActionExecuteInput, ) (*velero.RestoreItemActionExecuteOutput, error) { @@ -90,6 +254,13 @@ func (p *volumeSnapshotRestoreItemAction) Execute( errors.Wrapf(err, "failed to convert input.Item from unstructured") } + // Create stub VGSC if this snapshot was created via VolumeGroupSnapshot + // This must happen before VSC is created, as the CSI controller requires VGSC to exist + if err := p.ensureStubVGSCExists(context.Background(), &vsFromBackup, input.Restore); err != nil { + p.log.Warnf("Failed to create stub VGSC for VS %s/%s: %v", vsFromBackup.Namespace, vsFromBackup.Name, err) + // Continue with restore, VGSC creation failure should not block restore + } + generatedName := util.GenerateSha256FromRestoreUIDAndVsName(string(input.Restore.UID), vsFromBackup.Name) // Reset Spec to convert the VolumeSnapshot from using diff --git a/pkg/restore/actions/csi/volumesnapshot_action_test.go b/pkg/restore/actions/csi/volumesnapshot_action_test.go index 4fb37e301..9e548b59d 100644 --- a/pkg/restore/actions/csi/volumesnapshot_action_test.go +++ b/pkg/restore/actions/csi/volumesnapshot_action_test.go @@ -17,9 +17,11 @@ limitations under the License. package csi import ( + "context" "fmt" "testing" + volumegroupsnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1beta1" snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -27,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + crclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" @@ -219,3 +222,244 @@ func TestNewVolumeSnapshotRestoreItemAction(t *testing.T) { _, err1 := plugin1(logger) require.NoError(t, err1) } + +func TestEnsureStubVGSCExists(t *testing.T) { + testDriver := "rbd.csi.ceph.com" + testVGSHandle := "vgs-handle-123" + testSnapshotHandle := "snap-handle-456" + + tests := []struct { + name string + vs *snapshotv1api.VolumeSnapshot + restore *velerov1api.Restore + existingVGSC *volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent + expectVGSC bool + expectErr bool + expectedHandle string + }{ + { + name: "VS without VolumeGroupSnapshotHandle annotation - no VGSC created", + vs: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vs", + Namespace: "test-ns", + Annotations: map[string]string{ + velerov1api.VolumeSnapshotHandleAnnotation: testSnapshotHandle, + velerov1api.DriverNameAnnotation: testDriver, + }, + }, + }, + restore: builder.ForRestore("velero", "restore").ObjectMeta(builder.WithUID("restore-uid")).Result(), + expectVGSC: false, + expectErr: false, + }, + { + name: "VS with VolumeGroupSnapshotHandle but no SnapshotHandle - no VGSC created", + vs: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vs", + Namespace: "test-ns", + Annotations: map[string]string{ + velerov1api.VolumeGroupSnapshotHandleAnnotation: testVGSHandle, + velerov1api.DriverNameAnnotation: testDriver, + }, + }, + }, + restore: builder.ForRestore("velero", "restore").ObjectMeta(builder.WithUID("restore-uid")).Result(), + expectVGSC: false, + expectErr: false, + }, + { + name: "VS with VolumeGroupSnapshotHandle but no Driver annotation - no VGSC created", + vs: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vs", + Namespace: "test-ns", + Annotations: map[string]string{ + velerov1api.VolumeGroupSnapshotHandleAnnotation: testVGSHandle, + velerov1api.VolumeSnapshotHandleAnnotation: testSnapshotHandle, + }, + }, + }, + restore: builder.ForRestore("velero", "restore").ObjectMeta(builder.WithUID("restore-uid")).Result(), + expectVGSC: false, + expectErr: false, + }, + { + name: "VS with all required annotations - VGSC should be created", + vs: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vs", + Namespace: "test-ns", + Annotations: map[string]string{ + velerov1api.VolumeGroupSnapshotHandleAnnotation: testVGSHandle, + velerov1api.VolumeSnapshotHandleAnnotation: testSnapshotHandle, + velerov1api.DriverNameAnnotation: testDriver, + }, + }, + }, + restore: builder.ForRestore("velero", "restore").ObjectMeta(builder.WithUID("restore-uid")).Result(), + expectVGSC: true, + expectErr: false, + expectedHandle: testSnapshotHandle, + }, + { + name: "VGSC already exists - should add snapshot handle", + vs: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vs-2", + Namespace: "test-ns", + Annotations: map[string]string{ + velerov1api.VolumeGroupSnapshotHandleAnnotation: testVGSHandle, + velerov1api.VolumeSnapshotHandleAnnotation: "snap-handle-789", + velerov1api.DriverNameAnnotation: testDriver, + }, + }, + }, + restore: builder.ForRestore("velero", "restore").ObjectMeta(builder.WithUID("restore-uid")).Result(), + existingVGSC: &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: util.GenerateSha256FromRestoreUIDAndVsName("restore-uid", testVGSHandle), + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: testDriver, + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + Source: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{ + GroupSnapshotHandles: &volumegroupsnapshotv1beta1.GroupSnapshotHandles{ + VolumeGroupSnapshotHandle: testVGSHandle, + VolumeSnapshotHandles: []string{testSnapshotHandle}, + }, + }, + }, + }, + expectVGSC: true, + expectErr: false, + expectedHandle: "snap-handle-789", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + crClient := velerotest.NewFakeControllerRuntimeClient(t) + + // Create existing VGSC if provided + if tc.existingVGSC != nil { + require.NoError(t, crClient.Create(context.Background(), tc.existingVGSC)) + } + + p := &volumeSnapshotRestoreItemAction{ + log: logrus.StandardLogger(), + crClient: crClient, + } + + err := p.ensureStubVGSCExists(context.Background(), tc.vs, tc.restore) + + if tc.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + // Check if VGSC was created/updated + vgscName := util.GenerateSha256FromRestoreUIDAndVsName(string(tc.restore.UID), tc.vs.Annotations[velerov1api.VolumeGroupSnapshotHandleAnnotation]) + vgsc := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{} + getErr := crClient.Get(context.Background(), crclient.ObjectKey{Name: vgscName}, vgsc) + + if tc.expectVGSC { + require.NoError(t, getErr) + require.NotNil(t, vgsc.Spec.Source.GroupSnapshotHandles) + require.Contains(t, vgsc.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles, tc.expectedHandle) + } else { + // If no VGSC expected, it's okay if Get returns not found or if vgscName is empty + if tc.vs.Annotations[velerov1api.VolumeGroupSnapshotHandleAnnotation] != "" { + require.Error(t, getErr) + } + } + }) + } +} + +func TestAddSnapshotHandleToVGSC(t *testing.T) { + testDriver := "rbd.csi.ceph.com" + testVGSHandle := "vgs-handle-123" + + tests := []struct { + name string + existingHandles []string + nilGroupSnapshotHandles bool + newHandle string + expectedHandles []string + }{ + { + name: "Add new handle to empty list", + existingHandles: []string{}, + newHandle: "snap-1", + expectedHandles: []string{"snap-1"}, + }, + { + name: "Add new handle to existing list", + existingHandles: []string{"snap-1"}, + newHandle: "snap-2", + expectedHandles: []string{"snap-1", "snap-2"}, + }, + { + name: "Handle already exists - no change", + existingHandles: []string{"snap-1", "snap-2"}, + newHandle: "snap-1", + expectedHandles: []string{"snap-1", "snap-2"}, + }, + { + name: "Nil GroupSnapshotHandles - should initialize and add", + nilGroupSnapshotHandles: true, + newHandle: "snap-1", + expectedHandles: []string{"snap-1"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + crClient := velerotest.NewFakeControllerRuntimeClient(t) + + var source volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource + if tc.nilGroupSnapshotHandles { + source = volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{} + } else { + source = volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSource{ + GroupSnapshotHandles: &volumegroupsnapshotv1beta1.GroupSnapshotHandles{ + VolumeGroupSnapshotHandle: testVGSHandle, + VolumeSnapshotHandles: tc.existingHandles, + }, + } + } + + existingVGSC := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vgsc", + }, + Spec: volumegroupsnapshotv1beta1.VolumeGroupSnapshotContentSpec{ + Driver: testDriver, + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + Source: source, + }, + } + require.NoError(t, crClient.Create(context.Background(), existingVGSC)) + + // Re-fetch to get the created object with proper metadata + fetchedVGSC := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{} + require.NoError(t, crClient.Get(context.Background(), crclient.ObjectKey{Name: "test-vgsc"}, fetchedVGSC)) + + p := &volumeSnapshotRestoreItemAction{ + log: logrus.StandardLogger(), + crClient: crClient, + } + + err := p.addSnapshotHandleToVGSC(context.Background(), fetchedVGSC, tc.newHandle) + require.NoError(t, err) + + // Verify the VGSC has expected handles + updatedVGSC := &volumegroupsnapshotv1beta1.VolumeGroupSnapshotContent{} + require.NoError(t, crClient.Get(context.Background(), crclient.ObjectKey{Name: "test-vgsc"}, updatedVGSC)) + require.ElementsMatch(t, tc.expectedHandles, updatedVGSC.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles) + }) + } +} diff --git a/pkg/restore/actions/csi/volumesnapshotcontent_action.go b/pkg/restore/actions/csi/volumesnapshotcontent_action.go index 0a268eab2..00a25c86f 100644 --- a/pkg/restore/actions/csi/volumesnapshotcontent_action.go +++ b/pkg/restore/actions/csi/volumesnapshotcontent_action.go @@ -17,6 +17,8 @@ limitations under the License. package csi import ( + "context" + snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -108,12 +110,23 @@ func (p *volumeSnapshotContentRestoreItemAction) Execute( return nil, errors.Errorf("fail to get snapshot handle from VSC %s status", vsc.Name) } - if vsc.Spec.VolumeSnapshotClassName != nil { - // Delete VolumeSnapshotClass from the VolumeSnapshotContent. - // This is necessary to make the restore independent of the VolumeSnapshotClass. - vsc.Spec.VolumeSnapshotClassName = nil - p.log.Debugf("Deleted VolumeSnapshotClassName from VolumeSnapshotContent %s to make restore independent of VolumeSnapshotClass", - vsc.Name) + // Look up a VolumeSnapshotClass matching the driver for credential lookup. + // Some CSI drivers (e.g., Ceph RBD) need credentials for snapshot verification. + // Instead of keeping the original class name (which may not exist on target cluster), + // we find a matching class by driver to make restore portable. + vsc.Spec.VolumeSnapshotClassName = nil + vscList := &snapshotv1api.VolumeSnapshotClassList{} + if err := p.client.List(context.Background(), vscList); err == nil { + for i := range vscList.Items { + if vscList.Items[i].Driver == vsc.Spec.Driver { + vsc.Spec.VolumeSnapshotClassName = &vscList.Items[i].Name + p.log.Infof("Set VolumeSnapshotClassName to %s for VSC %s based on driver match", + vscList.Items[i].Name, vsc.Name) + break + } + } + } else { + p.log.Warnf("Failed to list VolumeSnapshotClasses: %v", err) } additionalItems := []velero.ResourceIdentifier{} diff --git a/pkg/test/fake_controller_runtime_client.go b/pkg/test/fake_controller_runtime_client.go index e22220404..ec22a3dc6 100644 --- a/pkg/test/fake_controller_runtime_client.go +++ b/pkg/test/fake_controller_runtime_client.go @@ -45,6 +45,7 @@ func NewFakeControllerRuntimeClientBuilder(t *testing.T) *k8sfake.ClientBuilder require.NoError(t, appsv1api.AddToScheme(scheme)) require.NoError(t, snapshotv1api.AddToScheme(scheme)) require.NoError(t, storagev1api.AddToScheme(scheme)) + require.NoError(t, volumegroupsnapshotv1beta1.AddToScheme(scheme)) return k8sfake.NewClientBuilder().WithScheme(scheme) }