diff --git a/changelogs/unreleased/2411-ashish-amarnath b/changelogs/unreleased/2411-ashish-amarnath new file mode 100644 index 000000000..a61e377cf --- /dev/null +++ b/changelogs/unreleased/2411-ashish-amarnath @@ -0,0 +1 @@ +during backup deletion also delete CSI volumesnapshots that were created as a part of the backup diff --git a/hack/test.sh b/hack/test.sh index deaf5ce3c..b54143363 100755 --- a/hack/test.sh +++ b/hack/test.sh @@ -35,5 +35,5 @@ if [[ -n "${GOFLAGS:-}" ]]; then echo "GOFLAGS: ${GOFLAGS}" fi -go test -installsuffix "static" -timeout 60s "${TARGETS[@]}" +go test -installsuffix "static" -timeout 60s -v "${TARGETS[@]}" echo "Success!" diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 76810af93..1d3be4b4e 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -28,6 +28,7 @@ import ( "sync" "time" + snapshotterClientSet "github.com/kubernetes-csi/external-snapshotter/v2/pkg/client/clientset/versioned" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/sirupsen/logrus" @@ -228,6 +229,7 @@ type server struct { dynamicClient dynamic.Interface sharedInformerFactory informers.SharedInformerFactory csiSnapshotterSharedInformerFactory *CSIInformerFactoryWrapper + csiSnapshotClient *snapshotterClientSet.Clientset ctx context.Context cancelFunc context.CancelFunc logger logrus.FieldLogger @@ -300,6 +302,7 @@ func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*s dynamicClient: dynamicClient, sharedInformerFactory: informers.NewSharedInformerFactoryWithOptions(veleroClient, 0, informers.WithNamespace(f.Namespace())), csiSnapshotterSharedInformerFactory: NewCSIInformerFactoryWrapper(csiSnapClient), + csiSnapshotClient: csiSnapClient, ctx: ctx, cancelFunc: cancelFunc, logger: logger, @@ -551,6 +554,34 @@ func (s *server) initRestic() error { return nil } +func (s *server) getCSISnapshotListers() (snapshotv1beta1listers.VolumeSnapshotLister, snapshotv1beta1listers.VolumeSnapshotContentLister) { + // Make empty listers that will only be populated if CSI is properly enabled. + var vsLister snapshotv1beta1listers.VolumeSnapshotLister + var vscLister snapshotv1beta1listers.VolumeSnapshotContentLister + var err error + + // If CSI is enabled, check for the CSI groups and generate the listers + // If CSI isn't enabled, return empty listers. + if features.IsEnabled(api.CSIFeatureFlag) { + _, err = s.discoveryClient.ServerResourcesForGroupVersion(snapshotv1beta1api.SchemeGroupVersion.String()) + switch { + case apierrors.IsNotFound(err): + // CSI is enabled, but the required CRDs aren't installed, so halt. + s.logger.Fatalf("The '%s' feature flag was specified, but CSI API group [%s] was not found.", api.CSIFeatureFlag, snapshotv1beta1api.SchemeGroupVersion.String()) + case err == nil: + // CSI is enabled, and the resources were found. + // Instantiate the listers fully + s.logger.Debug("Creating CSI listers") + // Access the wrapped factory directly here since we've already done the feature flag check above to know it's safe. + vsLister = s.csiSnapshotterSharedInformerFactory.factory.Snapshot().V1beta1().VolumeSnapshots().Lister() + vscLister = s.csiSnapshotterSharedInformerFactory.factory.Snapshot().V1beta1().VolumeSnapshotContents().Lister() + case err != nil: + cmd.CheckError(err) + } + } + return vsLister, vscLister +} + func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string) error { s.logger.Info("Starting controllers") @@ -573,6 +604,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string newPluginManager := func(logger logrus.FieldLogger) clientmgmt.Manager { return clientmgmt.NewManager(logger, s.logLevel, s.pluginRegistry) } + csiVSLister, csiVSCLister := s.getCSISnapshotListers() backupSyncControllerRunInfo := func() controllerRunInfo { backupSyncContoller := controller.NewBackupSyncController( @@ -606,30 +638,6 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string ) cmd.CheckError(err) - // Make empty listers that will only be populated if CSI is properly enabled. - var vsLister snapshotv1beta1listers.VolumeSnapshotLister - var vscLister snapshotv1beta1listers.VolumeSnapshotContentLister - - // If CSI is enabled, check for the CSI groups and generate the listers - // If CSI isn't enabled, proceed normally. - if features.IsEnabled(api.CSIFeatureFlag) { - _, err = s.discoveryClient.ServerResourcesForGroupVersion(snapshotv1beta1api.SchemeGroupVersion.String()) - switch { - case apierrors.IsNotFound(err): - // CSI is enabled, but the required CRDs aren't installed, so halt. - s.logger.Fatalf("The '%s' feature flag was specified, but CSI API group [%s] was not found.", api.CSIFeatureFlag, snapshotv1beta1api.SchemeGroupVersion.String()) - case err == nil: - // CSI is enabled, and the resources were found. - // Instantiate the listers fully - s.logger.Debug("Creating CSI listers") - // Access the wrapped factory directly here since we've already done the feature flag check above to know it's safe. - vsLister = s.csiSnapshotterSharedInformerFactory.factory.Snapshot().V1beta1().VolumeSnapshots().Lister() - vscLister = s.csiSnapshotterSharedInformerFactory.factory.Snapshot().V1beta1().VolumeSnapshotContents().Lister() - case err != nil: - cmd.CheckError(err) - } - } - backupController := controller.NewBackupController( s.sharedInformerFactory.Velero().V1().Backups(), s.veleroClient.VeleroV1(), @@ -646,8 +654,8 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string defaultVolumeSnapshotLocations, s.metrics, s.config.formatFlag.Parse(), - vsLister, - vscLister, + csiVSLister, + csiVSCLister, ) return controllerRunInfo{ @@ -700,6 +708,8 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string s.sharedInformerFactory.Velero().V1().PodVolumeBackups().Lister(), s.sharedInformerFactory.Velero().V1().BackupStorageLocations().Lister(), s.sharedInformerFactory.Velero().V1().VolumeSnapshotLocations().Lister(), + csiVSLister, + s.csiSnapshotClient, newPluginManager, s.metrics, ) diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index 2f2db4574..735ced87a 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -23,6 +23,9 @@ import ( "time" jsonpatch "github.com/evanphx/json-patch" + snapshotterClientSet "github.com/kubernetes-csi/external-snapshotter/v2/pkg/client/clientset/versioned" + snapshotter "github.com/kubernetes-csi/external-snapshotter/v2/pkg/client/clientset/versioned/typed/volumesnapshot/v1beta1" + snapshotv1beta1listers "github.com/kubernetes-csi/external-snapshotter/v2/pkg/client/listers/volumesnapshot/v1beta1" "github.com/pkg/errors" "github.com/sirupsen/logrus" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -33,8 +36,9 @@ import ( kubeerrs "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/cache" - v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" pkgbackup "github.com/vmware-tanzu/velero/pkg/backup" + "github.com/vmware-tanzu/velero/pkg/features" velerov1client "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1" velerov1informers "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions/velero/v1" velerov1listers "github.com/vmware-tanzu/velero/pkg/generated/listers/velero/v1" @@ -62,10 +66,12 @@ type backupDeletionController struct { podvolumeBackupLister velerov1listers.PodVolumeBackupLister backupLocationLister velerov1listers.BackupStorageLocationLister snapshotLocationLister velerov1listers.VolumeSnapshotLocationLister - processRequestFunc func(*v1.DeleteBackupRequest) error + csiSnapshotLister snapshotv1beta1listers.VolumeSnapshotLister + csiSnapshotClient *snapshotterClientSet.Clientset + processRequestFunc func(*velerov1api.DeleteBackupRequest) error clock clock.Clock newPluginManager func(logrus.FieldLogger) clientmgmt.Manager - newBackupStore func(*v1.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) + newBackupStore func(*velerov1api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) metrics *metrics.ServerMetrics } @@ -82,6 +88,8 @@ func NewBackupDeletionController( podvolumeBackupLister velerov1listers.PodVolumeBackupLister, backupLocationLister velerov1listers.BackupStorageLocationLister, snapshotLocationLister velerov1listers.VolumeSnapshotLocationLister, + csiSnapshotLister snapshotv1beta1listers.VolumeSnapshotLister, + csiSnapshotClient *snapshotterClientSet.Clientset, newPluginManager func(logrus.FieldLogger) clientmgmt.Manager, metrics *metrics.ServerMetrics, ) Interface { @@ -97,6 +105,8 @@ func NewBackupDeletionController( podvolumeBackupLister: podvolumeBackupLister, backupLocationLister: backupLocationLister, snapshotLocationLister: snapshotLocationLister, + csiSnapshotLister: csiSnapshotLister, + csiSnapshotClient: csiSnapshotClient, metrics: metrics, // use variables to refer to these functions so they can be // replaced with fakes for testing. @@ -140,7 +150,7 @@ func (c *backupDeletionController) processQueueItem(key string) error { } switch req.Status.Phase { - case v1.DeleteBackupRequestPhaseProcessed: + case velerov1api.DeleteBackupRequestPhaseProcessed: // Don't do anything because it's already been processed default: // Don't mutate the shared cache @@ -151,7 +161,7 @@ func (c *backupDeletionController) processQueueItem(key string) error { return nil } -func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) error { +func (c *backupDeletionController) processRequest(req *velerov1api.DeleteBackupRequest) error { log := c.logger.WithFields(logrus.Fields{ "namespace": req.Namespace, "name": req.Name, @@ -162,8 +172,8 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e // Make sure we have the backup name if req.Spec.BackupName == "" { - _, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + _, err = c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + r.Status.Phase = velerov1api.DeleteBackupRequestPhaseProcessed r.Status.Errors = []string{"spec.backupName is required"} }) return err @@ -177,8 +187,8 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e // Don't allow deleting an in-progress backup if c.backupTracker.Contains(req.Namespace, req.Spec.BackupName) { - _, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + _, err = c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + r.Status.Phase = velerov1api.DeleteBackupRequestPhaseProcessed r.Status.Errors = []string{"backup is still in progress"} }) @@ -189,8 +199,8 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e backup, err := c.backupClient.Backups(req.Namespace).Get(req.Spec.BackupName, metav1.GetOptions{}) if apierrors.IsNotFound(err) { // Couldn't find backup - update status to Processed and record the not-found error - req, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + req, err = c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + r.Status.Phase = velerov1api.DeleteBackupRequestPhaseProcessed r.Status.Errors = []string{"backup not found"} }) @@ -203,8 +213,8 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e // Don't allow deleting backups in read-only storage locations location, err := c.backupLocationLister.BackupStorageLocations(backup.Namespace).Get(backup.Spec.StorageLocation) if apierrors.IsNotFound(err) { - _, err := c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + _, err := c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + r.Status.Phase = velerov1api.DeleteBackupRequestPhaseProcessed r.Status.Errors = append(r.Status.Errors, fmt.Sprintf("backup storage location %s not found", backup.Spec.StorageLocation)) }) return err @@ -213,9 +223,9 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e return errors.Wrap(err, "error getting backup storage location") } - if location.Spec.AccessMode == v1.BackupStorageLocationAccessModeReadOnly { - _, err := c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + if location.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly { + _, err := c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + r.Status.Phase = velerov1api.DeleteBackupRequestPhaseProcessed r.Status.Errors = append(r.Status.Errors, fmt.Sprintf("cannot delete backup because backup storage location %s is currently in read-only mode", location.Name)) }) return err @@ -228,11 +238,11 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e } // Update status to InProgress and set backup-name label if needed - req, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseInProgress + req, err = c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + r.Status.Phase = velerov1api.DeleteBackupRequestPhaseInProgress - if req.Labels[v1.BackupNameLabel] == "" { - req.Labels[v1.BackupNameLabel] = label.GetValidName(req.Spec.BackupName) + if req.Labels[velerov1api.BackupNameLabel] == "" { + req.Labels[velerov1api.BackupNameLabel] = label.GetValidName(req.Spec.BackupName) } }) if err != nil { @@ -240,9 +250,9 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e } // Set backup-uid label if needed - if req.Labels[v1.BackupUIDLabel] == "" { - req, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - req.Labels[v1.BackupUIDLabel] = string(backup.UID) + if req.Labels[velerov1api.BackupUIDLabel] == "" { + req, err = c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + req.Labels[velerov1api.BackupUIDLabel] = string(backup.UID) }) if err != nil { return err @@ -250,15 +260,15 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e } // Set backup status to Deleting - backup, err = c.patchBackup(backup, func(b *v1.Backup) { - b.Status.Phase = v1.BackupPhaseDeleting + backup, err = c.patchBackup(backup, func(b *velerov1api.Backup) { + b.Status.Phase = velerov1api.BackupPhaseDeleting }) if err != nil { log.WithError(errors.WithStack(err)).Error("Error setting backup phase to deleting") return err } - backupScheduleName := backup.GetLabels()[v1.ScheduleNameLabel] + backupScheduleName := backup.GetLabels()[velerov1api.ScheduleNameLabel] c.metrics.RegisterBackupDeletionAttempt(backupScheduleName) var errs []string @@ -312,6 +322,15 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e } } + if features.IsEnabled(velerov1api.CSIFeatureFlag) { + log.Info("Removing CSI volumesnapshots") + if csiErrs := deleteCSIVolumeSnapshots(backup, c.csiSnapshotLister, c.csiSnapshotClient.SnapshotV1beta1(), log); len(csiErrs) > 0 { + for _, err := range csiErrs { + errs = append(errs, err.Error()) + } + } + } + log.Info("Removing restores") if restores, err := c.restoreLister.Restores(backup.Namespace).List(labels.Everything()); err != nil { log.WithError(errors.WithStack(err)).Error("Error listing restore API objects") @@ -352,8 +371,8 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e } // Update status to processed and record errors - req, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + req, err = c.patchDeleteBackupRequest(req, func(r *velerov1api.DeleteBackupRequest) { + r.Status.Phase = velerov1api.DeleteBackupRequestPhaseProcessed r.Status.Errors = errs }) if err != nil { @@ -395,10 +414,10 @@ func volumeSnapshotterForSnapshotLocation( return volumeSnapshotter, nil } -func (c *backupDeletionController) deleteExistingDeletionRequests(req *v1.DeleteBackupRequest, log logrus.FieldLogger) []error { +func (c *backupDeletionController) deleteExistingDeletionRequests(req *velerov1api.DeleteBackupRequest, log logrus.FieldLogger) []error { log.Info("Removing existing deletion requests for backup") selector := labels.SelectorFromSet(labels.Set(map[string]string{ - v1.BackupNameLabel: label.GetValidName(req.Spec.BackupName), + velerov1api.BackupNameLabel: label.GetValidName(req.Spec.BackupName), })) dbrs, err := c.deleteBackupRequestLister.DeleteBackupRequests(req.Namespace).List(selector) if err != nil { @@ -419,7 +438,7 @@ func (c *backupDeletionController) deleteExistingDeletionRequests(req *v1.Delete return errs } -func (c *backupDeletionController) deleteResticSnapshots(backup *v1.Backup) []error { +func (c *backupDeletionController) deleteResticSnapshots(backup *velerov1api.Backup) []error { if c.resticMgr == nil { return nil } @@ -442,6 +461,39 @@ func (c *backupDeletionController) deleteResticSnapshots(backup *v1.Backup) []er return errs } +func deleteCSIVolumeSnapshots(backup *velerov1api.Backup, csiSnapshotLister snapshotv1beta1listers.VolumeSnapshotLister, + csiClient snapshotter.SnapshotV1beta1Interface, log *logrus.Entry) []error { + errs := []error{} + + selector := labels.SelectorFromSet(map[string]string{velerov1api.BackupNameLabel: backup.Name}) + csiVolSnaps, err := csiSnapshotLister.List(selector) + if err != nil { + return []error{err} + } + + log.Infof("Deleting %d CSI volumesnapshots", len(csiVolSnaps)) + for _, csiVS := range csiVolSnaps { + log.Infof("Deleting CSI volumesnapshot %s/%s", csiVS.Namespace, csiVS.Name) + if csiVS.Status != nil && csiVS.Status.BoundVolumeSnapshotContentName != nil { + // we patch the DeletionPolicy of the volumesnapshotcontent to set it to Delete. + // This ensures that the volume snapshot in the storage provider is also deleted. + log.Infof("Setting DeletionPolicy of CSI volumesnapshotcontent %s to Delete", *csiVS.Status.BoundVolumeSnapshotContentName) + pb := []byte(`{"spec":{"deletionPolicy":"Delete"}}`) + _, err := csiClient.VolumeSnapshotContents().Patch(*csiVS.Status.BoundVolumeSnapshotContentName, types.MergePatchType, pb) + if err != nil && !apierrors.IsNotFound(err) { + errs = append(errs, errors.Wrapf(err, "failed to update deletion policy on volumesnapshotcontent %s skipping deletion of volumesnapshot %s/%s", + *csiVS.Status.BoundVolumeSnapshotContentName, csiVS.Namespace, csiVS.Name)) + continue + } + } + err := csiClient.VolumeSnapshots(csiVS.Namespace).Delete(csiVS.Name, &metav1.DeleteOptions{}) + if err != nil { + errs = append(errs, err) + } + } + return errs +} + const deleteBackupRequestMaxAge = 24 * time.Hour func (c *backupDeletionController) deleteExpiredRequests() { @@ -458,7 +510,7 @@ func (c *backupDeletionController) deleteExpiredRequests() { now := c.clock.Now() for _, req := range requests { - if req.Status.Phase != v1.DeleteBackupRequestPhaseProcessed { + if req.Status.Phase != velerov1api.DeleteBackupRequestPhaseProcessed { continue } @@ -475,7 +527,7 @@ func (c *backupDeletionController) deleteExpiredRequests() { } } -func (c *backupDeletionController) patchDeleteBackupRequest(req *v1.DeleteBackupRequest, mutate func(*v1.DeleteBackupRequest)) (*v1.DeleteBackupRequest, error) { +func (c *backupDeletionController) patchDeleteBackupRequest(req *velerov1api.DeleteBackupRequest, mutate func(*velerov1api.DeleteBackupRequest)) (*velerov1api.DeleteBackupRequest, error) { // Record original json oldData, err := json.Marshal(req) if err != nil { @@ -504,7 +556,7 @@ func (c *backupDeletionController) patchDeleteBackupRequest(req *v1.DeleteBackup return req, nil } -func (c *backupDeletionController) patchBackup(backup *v1.Backup, mutate func(*v1.Backup)) (*v1.Backup, error) { +func (c *backupDeletionController) patchBackup(backup *velerov1api.Backup, mutate func(*velerov1api.Backup)) (*velerov1api.Backup, error) { // Record original json oldData, err := json.Marshal(backup) if err != nil { diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index 2f3016c53..317cce8b0 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -21,6 +21,9 @@ import ( "testing" "time" + snapshotv1beta1api "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1" + snapshotFake "github.com/kubernetes-csi/external-snapshotter/v2/pkg/client/clientset/versioned/fake" + snapshotv1beta1informers "github.com/kubernetes-csi/external-snapshotter/v2/pkg/client/informers/externalversions" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -32,7 +35,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" core "k8s.io/client-go/testing" - v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" pkgbackup "github.com/vmware-tanzu/velero/pkg/backup" "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/fake" @@ -62,6 +65,8 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) { sharedInformers.Velero().V1().PodVolumeBackups().Lister(), sharedInformers.Velero().V1().BackupStorageLocations().Lister(), sharedInformers.Velero().V1().VolumeSnapshotLocations().Lister(), + nil, // csiSnapshotLister + nil, // csiSnapshotClient nil, // new plugin manager func metrics.NewServerMetrics(), ).(*backupDeletionController) @@ -78,21 +83,21 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) { req := pkgbackup.NewDeleteBackupRequest("foo", "uid") req.Namespace = "foo" req.Name = "foo-abcde" - req.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + req.Status.Phase = velerov1.DeleteBackupRequestPhaseProcessed err = controller.processQueueItem("foo/bar") assert.NoError(t, err) // Invoke processRequestFunc - for _, phase := range []v1.DeleteBackupRequestPhase{"", v1.DeleteBackupRequestPhaseNew, v1.DeleteBackupRequestPhaseInProgress} { + for _, phase := range []velerov1.DeleteBackupRequestPhase{"", velerov1.DeleteBackupRequestPhaseNew, velerov1.DeleteBackupRequestPhaseInProgress} { t.Run(fmt.Sprintf("phase=%s", phase), func(t *testing.T) { req.Status.Phase = phase sharedInformers.Velero().V1().DeleteBackupRequests().Informer().GetStore().Add(req) var errorToReturn error - var actual *v1.DeleteBackupRequest + var actual *velerov1.DeleteBackupRequest var called bool - controller.processRequestFunc = func(r *v1.DeleteBackupRequest) error { + controller.processRequestFunc = func(r *velerov1.DeleteBackupRequest) error { called = true actual = r return errorToReturn @@ -119,7 +124,7 @@ type backupDeletionControllerTestData struct { volumeSnapshotter *velerotest.FakeVolumeSnapshotter backupStore *persistencemocks.BackupStore controller *backupDeletionController - req *v1.DeleteBackupRequest + req *velerov1.DeleteBackupRequest } func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletionControllerTestData { @@ -152,6 +157,8 @@ func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletio sharedInformers.Velero().V1().PodVolumeBackups().Lister(), sharedInformers.Velero().V1().BackupStorageLocations().Lister(), sharedInformers.Velero().V1().VolumeSnapshotLocations().Lister(), + nil, // csiSnapshotLister + nil, // csiSnapshotClient func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager }, metrics.NewServerMetrics(), ).(*backupDeletionController), @@ -159,7 +166,7 @@ func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletio req: req, } - data.controller.newBackupStore = func(*v1.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) { + data.controller.newBackupStore = func(*velerov1.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error) { return backupStore, nil } @@ -179,7 +186,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, @@ -199,15 +206,15 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { require.NoError(t, td.sharedInformers.Velero().V1().DeleteBackupRequests().Informer().GetStore().Add(td.req)) - existing := &v1.DeleteBackupRequest{ + existing := &velerov1.DeleteBackupRequest{ ObjectMeta: metav1.ObjectMeta{ Namespace: td.req.Namespace, Name: "bar", Labels: map[string]string{ - v1.BackupNameLabel: td.req.Spec.BackupName, + velerov1.BackupNameLabel: td.req.Spec.BackupName, }, }, - Spec: v1.DeleteBackupRequestSpec{ + Spec: velerov1.DeleteBackupRequestSpec{ BackupName: td.req.Spec.BackupName, }, } @@ -216,15 +223,15 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { require.NoError(t, err) require.NoError(t, td.sharedInformers.Velero().V1().DeleteBackupRequests().Informer().GetStore().Add( - &v1.DeleteBackupRequest{ + &velerov1.DeleteBackupRequest{ ObjectMeta: metav1.ObjectMeta{ Namespace: td.req.Namespace, Name: "bar-2", Labels: map[string]string{ - v1.BackupNameLabel: "some-other-backup", + velerov1.BackupNameLabel: "some-other-backup", }, }, - Spec: v1.DeleteBackupRequestSpec{ + Spec: velerov1.DeleteBackupRequestSpec{ BackupName: "some-other-backup", }, }, @@ -233,7 +240,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { assert.NoError(t, td.controller.processRequest(td.req)) expectedDeleteAction := core.NewDeleteAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, "bar", ) @@ -255,7 +262,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, @@ -267,7 +274,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { }) t.Run("patching to InProgress fails", func(t *testing.T) { - backup := builder.ForBackup(v1.DefaultNamespace, "foo").StorageLocation("default").Result() + backup := builder.ForBackup(velerov1.DefaultNamespace, "foo").StorageLocation("default").Result() location := builder.ForBackupStorageLocation("velero", "default").Result() td := setupBackupDeletionControllerTest(backup) @@ -283,12 +290,12 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewGetAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), backup.Namespace, backup.Name, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, @@ -299,7 +306,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { }) t.Run("patching backup to Deleting fails", func(t *testing.T) { - backup := builder.ForBackup(v1.DefaultNamespace, "foo").StorageLocation("default").Result() + backup := builder.ForBackup(velerov1.DefaultNamespace, "foo").StorageLocation("default").Result() location := builder.ForBackupStorageLocation("velero", "default").Result() td := setupBackupDeletionControllerTest(backup) @@ -318,19 +325,19 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewGetAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), backup.Namespace, backup.Name, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, []byte(`{"status":{"phase":"InProgress"}}`), ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), backup.Namespace, backup.Name, types.MergePatchType, @@ -348,12 +355,12 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewGetAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, @@ -365,7 +372,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { }) t.Run("unable to find backup storage location", func(t *testing.T) { - backup := builder.ForBackup(v1.DefaultNamespace, "foo").StorageLocation("default").Result() + backup := builder.ForBackup(velerov1.DefaultNamespace, "foo").StorageLocation("default").Result() td := setupBackupDeletionControllerTest(backup) @@ -374,12 +381,12 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewGetAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, @@ -391,8 +398,8 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { }) t.Run("backup storage location is in read-only mode", func(t *testing.T) { - backup := builder.ForBackup(v1.DefaultNamespace, "foo").StorageLocation("default").Result() - location := builder.ForBackupStorageLocation("velero", "default").AccessMode(v1.BackupStorageLocationAccessModeReadOnly).Result() + backup := builder.ForBackup(velerov1.DefaultNamespace, "foo").StorageLocation("default").Result() + location := builder.ForBackupStorageLocation("velero", "default").AccessMode(velerov1.BackupStorageLocationAccessModeReadOnly).Result() td := setupBackupDeletionControllerTest(backup) @@ -403,12 +410,12 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewGetAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, @@ -420,13 +427,13 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { }) t.Run("full delete, no errors", func(t *testing.T) { - backup := builder.ForBackup(v1.DefaultNamespace, "foo").Result() + backup := builder.ForBackup(velerov1.DefaultNamespace, "foo").Result() backup.UID = "uid" backup.Spec.StorageLocation = "primary" - restore1 := builder.ForRestore("velero", "restore-1").Phase(v1.RestorePhaseCompleted).Backup("foo").Result() - restore2 := builder.ForRestore("velero", "restore-2").Phase(v1.RestorePhaseCompleted).Backup("foo").Result() - restore3 := builder.ForRestore("velero", "restore-3").Phase(v1.RestorePhaseCompleted).Backup("some-other-backup").Result() + restore1 := builder.ForRestore("velero", "restore-1").Phase(velerov1.RestorePhaseCompleted).Backup("foo").Result() + restore2 := builder.ForRestore("velero", "restore-2").Phase(velerov1.RestorePhaseCompleted).Backup("foo").Result() + restore3 := builder.ForRestore("velero", "restore-3").Phase(velerov1.RestorePhaseCompleted).Backup("some-other-backup").Result() td := setupBackupDeletionControllerTest(backup, restore1, restore2, restore3) @@ -434,15 +441,15 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore2) td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore3) - location := &v1.BackupStorageLocation{ + location := &velerov1.BackupStorageLocation{ ObjectMeta: metav1.ObjectMeta{ Namespace: backup.Namespace, Name: backup.Spec.StorageLocation, }, - Spec: v1.BackupStorageLocationSpec{ + Spec: velerov1.BackupStorageLocationSpec{ Provider: "objStoreProvider", - StorageType: v1.StorageType{ - ObjectStorage: &v1.ObjectStorageLocation{ + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ Bucket: "bucket", }, }, @@ -450,12 +457,12 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { } require.NoError(t, td.sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(location)) - snapshotLocation := &v1.VolumeSnapshotLocation{ + snapshotLocation := &velerov1.VolumeSnapshotLocation{ ObjectMeta: metav1.ObjectMeta{ Namespace: backup.Namespace, Name: "vsl-1", }, - Spec: v1.VolumeSnapshotLocationSpec{ + Spec: velerov1.VolumeSnapshotLocationSpec{ Provider: "provider-1", }, } @@ -505,55 +512,55 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, []byte(`{"metadata":{"labels":{"velero.io/backup-name":"foo"}},"status":{"phase":"InProgress"}}`), ), core.NewGetAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, []byte(`{"metadata":{"labels":{"velero.io/backup-uid":"uid"}}}`), ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, types.MergePatchType, []byte(`{"status":{"phase":"Deleting"}}`), ), core.NewDeleteAction( - v1.SchemeGroupVersion.WithResource("restores"), + velerov1.SchemeGroupVersion.WithResource("restores"), td.req.Namespace, "restore-1", ), core.NewDeleteAction( - v1.SchemeGroupVersion.WithResource("restores"), + velerov1.SchemeGroupVersion.WithResource("restores"), td.req.Namespace, "restore-2", ), core.NewDeleteAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, []byte(`{"status":{"phase":"Processed"}}`), ), core.NewDeleteCollectionAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, pkgbackup.NewDeleteBackupRequestListOptions(td.req.Spec.BackupName, "uid"), ), @@ -575,15 +582,15 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { backup.Spec.StorageLocation = "primary" restore1 := builder.ForRestore("velero", "restore-1"). - Phase(v1.RestorePhaseCompleted). + Phase(velerov1.RestorePhaseCompleted). Backup("the-really-long-backup-name-that-is-much-more-than-63-characters"). Result() restore2 := builder.ForRestore("velero", "restore-2"). - Phase(v1.RestorePhaseCompleted). + Phase(velerov1.RestorePhaseCompleted). Backup("the-really-long-backup-name-that-is-much-more-than-63-characters"). Result() restore3 := builder.ForRestore("velero", "restore-3"). - Phase(v1.RestorePhaseCompleted). + Phase(velerov1.RestorePhaseCompleted). Backup("some-other-backup"). Result() @@ -595,15 +602,15 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore2) td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore3) - location := &v1.BackupStorageLocation{ + location := &velerov1.BackupStorageLocation{ ObjectMeta: metav1.ObjectMeta{ Namespace: backup.Namespace, Name: backup.Spec.StorageLocation, }, - Spec: v1.BackupStorageLocationSpec{ + Spec: velerov1.BackupStorageLocationSpec{ Provider: "objStoreProvider", - StorageType: v1.StorageType{ - ObjectStorage: &v1.ObjectStorageLocation{ + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ Bucket: "bucket", }, }, @@ -611,12 +618,12 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { } require.NoError(t, td.sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(location)) - snapshotLocation := &v1.VolumeSnapshotLocation{ + snapshotLocation := &velerov1.VolumeSnapshotLocation{ ObjectMeta: metav1.ObjectMeta{ Namespace: backup.Namespace, Name: "vsl-1", }, - Spec: v1.VolumeSnapshotLocationSpec{ + Spec: velerov1.VolumeSnapshotLocationSpec{ Provider: "provider-1", }, } @@ -664,55 +671,55 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { expectedActions := []core.Action{ core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, []byte(`{"metadata":{"labels":{"velero.io/backup-name":"the-really-long-backup-name-that-is-much-more-than-63-cha6ca4bc"}},"status":{"phase":"InProgress"}}`), ), core.NewGetAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, []byte(`{"metadata":{"labels":{"velero.io/backup-uid":"uid"}}}`), ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, types.MergePatchType, []byte(`{"status":{"phase":"Deleting"}}`), ), core.NewDeleteAction( - v1.SchemeGroupVersion.WithResource("restores"), + velerov1.SchemeGroupVersion.WithResource("restores"), td.req.Namespace, "restore-1", ), core.NewDeleteAction( - v1.SchemeGroupVersion.WithResource("restores"), + velerov1.SchemeGroupVersion.WithResource("restores"), td.req.Namespace, "restore-2", ), core.NewDeleteAction( - v1.SchemeGroupVersion.WithResource("backups"), + velerov1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, td.req.Spec.BackupName, ), core.NewPatchAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, td.req.Name, types.MergePatchType, []byte(`{"status":{"phase":"Processed"}}`), ), core.NewDeleteCollectionAction( - v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, pkgbackup.NewDeleteBackupRequestListOptions(td.req.Spec.BackupName, "uid"), ), @@ -734,7 +741,7 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { tests := []struct { name string - requests []*v1.DeleteBackupRequest + requests []*velerov1.DeleteBackupRequest expectedDeletions []string }{ { @@ -742,14 +749,14 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { }, { name: "older than max age, phase = '', don't delete", - requests: []*v1.DeleteBackupRequest{ + requests: []*velerov1.DeleteBackupRequest{ { ObjectMeta: metav1.ObjectMeta{ Namespace: "ns", Name: "name", CreationTimestamp: metav1.Time{Time: expired1}, }, - Status: v1.DeleteBackupRequestStatus{ + Status: velerov1.DeleteBackupRequestStatus{ Phase: "", }, }, @@ -757,45 +764,45 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { }, { name: "older than max age, phase = New, don't delete", - requests: []*v1.DeleteBackupRequest{ + requests: []*velerov1.DeleteBackupRequest{ { ObjectMeta: metav1.ObjectMeta{ Namespace: "ns", Name: "name", CreationTimestamp: metav1.Time{Time: expired1}, }, - Status: v1.DeleteBackupRequestStatus{ - Phase: v1.DeleteBackupRequestPhaseNew, + Status: velerov1.DeleteBackupRequestStatus{ + Phase: velerov1.DeleteBackupRequestPhaseNew, }, }, }, }, { name: "older than max age, phase = InProcess, don't delete", - requests: []*v1.DeleteBackupRequest{ + requests: []*velerov1.DeleteBackupRequest{ { ObjectMeta: metav1.ObjectMeta{ Namespace: "ns", Name: "name", CreationTimestamp: metav1.Time{Time: expired1}, }, - Status: v1.DeleteBackupRequestStatus{ - Phase: v1.DeleteBackupRequestPhaseInProgress, + Status: velerov1.DeleteBackupRequestStatus{ + Phase: velerov1.DeleteBackupRequestPhaseInProgress, }, }, }, }, { name: "some expired, some not", - requests: []*v1.DeleteBackupRequest{ + requests: []*velerov1.DeleteBackupRequest{ { ObjectMeta: metav1.ObjectMeta{ Namespace: "ns", Name: "unexpired-1", CreationTimestamp: metav1.Time{Time: unexpired1}, }, - Status: v1.DeleteBackupRequestStatus{ - Phase: v1.DeleteBackupRequestPhaseProcessed, + Status: velerov1.DeleteBackupRequestStatus{ + Phase: velerov1.DeleteBackupRequestPhaseProcessed, }, }, { @@ -804,8 +811,8 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { Name: "expired-1", CreationTimestamp: metav1.Time{Time: expired1}, }, - Status: v1.DeleteBackupRequestStatus{ - Phase: v1.DeleteBackupRequestPhaseProcessed, + Status: velerov1.DeleteBackupRequestStatus{ + Phase: velerov1.DeleteBackupRequestPhaseProcessed, }, }, { @@ -814,8 +821,8 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { Name: "unexpired-2", CreationTimestamp: metav1.Time{Time: unexpired2}, }, - Status: v1.DeleteBackupRequestStatus{ - Phase: v1.DeleteBackupRequestPhaseProcessed, + Status: velerov1.DeleteBackupRequestStatus{ + Phase: velerov1.DeleteBackupRequestPhaseProcessed, }, }, { @@ -824,8 +831,8 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { Name: "expired-2", CreationTimestamp: metav1.Time{Time: expired2}, }, - Status: v1.DeleteBackupRequestStatus{ - Phase: v1.DeleteBackupRequestPhaseProcessed, + Status: velerov1.DeleteBackupRequestStatus{ + Phase: velerov1.DeleteBackupRequestPhaseProcessed, }, }, }, @@ -850,6 +857,8 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { sharedInformers.Velero().V1().PodVolumeBackups().Lister(), sharedInformers.Velero().V1().BackupStorageLocations().Lister(), sharedInformers.Velero().V1().VolumeSnapshotLocations().Lister(), + nil, // csiSnapshotLister + nil, // csiSnapshotClient nil, // new plugin manager func metrics.NewServerMetrics(), ).(*backupDeletionController) @@ -866,10 +875,187 @@ func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { expectedActions := []core.Action{} for _, name := range test.expectedDeletions { - expectedActions = append(expectedActions, core.NewDeleteAction(v1.SchemeGroupVersion.WithResource("deletebackuprequests"), "ns", name)) + expectedActions = append(expectedActions, core.NewDeleteAction(velerov1.SchemeGroupVersion.WithResource("deletebackuprequests"), "ns", name)) } velerotest.CompareActions(t, expectedActions, client.Actions()) }) } } + +func TestDeleteCSIVolumeSnapshots(t *testing.T) { + //Backup1 + ns1VS1VSCName := "ns1vs1vsc" + ns1VS1VSC := snapshotv1beta1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: ns1VS1VSCName, + }, + Spec: snapshotv1beta1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1beta1api.VolumeSnapshotContentRetain, + }, + } + ns1VS1 := snapshotv1beta1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs1", + Namespace: "ns1", + Labels: map[string]string{ + velerov1.BackupNameLabel: "backup1", + }, + }, + Status: &snapshotv1beta1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &ns1VS1VSCName, + }, + } + + ns1VS2VSCName := "ns1vs2vsc" + ns1VS2VSC := snapshotv1beta1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: ns1VS2VSCName, + }, + Spec: snapshotv1beta1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1beta1api.VolumeSnapshotContentRetain, + }, + } + ns1VS2 := snapshotv1beta1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs2", + Namespace: "ns1", + Labels: map[string]string{ + velerov1.BackupNameLabel: "backup1", + }, + }, + Status: &snapshotv1beta1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &ns1VS2VSCName, + }, + } + + ns2VS1VSCName := "ns2vs1vsc" + ns2VS1VSC := snapshotv1beta1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: ns2VS1VSCName, + }, + Spec: snapshotv1beta1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1beta1api.VolumeSnapshotContentRetain, + }, + } + ns2VS1 := snapshotv1beta1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs1", + Namespace: "ns2", + Labels: map[string]string{ + velerov1.BackupNameLabel: "backup1", + }, + }, + Status: &snapshotv1beta1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &ns2VS1VSCName, + }, + } + + ns2VS2VSCName := "ns2vs2vsc" + ns2VS2VSC := snapshotv1beta1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: ns2VS2VSCName, + }, + Spec: snapshotv1beta1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1beta1api.VolumeSnapshotContentRetain, + }, + } + ns2VS2 := snapshotv1beta1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs2", + Namespace: "ns2", + Labels: map[string]string{ + velerov1.BackupNameLabel: "backup1", + }, + }, + Status: &snapshotv1beta1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &ns2VS2VSCName, + }, + } + + // Backup2 + ns1NilStatusVS := snapshotv1beta1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1NilStatusVS", + Namespace: "ns2", + Labels: map[string]string{ + velerov1.BackupNameLabel: "backup2", + }, + }, + Status: nil, + } + + // Backup3 + ns1NilBoundVSCVS := snapshotv1beta1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1NilBoundVSCVS", + Namespace: "ns2", + Labels: map[string]string{ + velerov1.BackupNameLabel: "backup3", + }, + }, + Status: &snapshotv1beta1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: nil, + }, + } + + // Backup4 + notFound := "not-found" + ns1NonExistentVSCVS := snapshotv1beta1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1NonExistentVSCVS", + Namespace: "ns2", + Labels: map[string]string{ + velerov1.BackupNameLabel: "backup3", + }, + }, + Status: &snapshotv1beta1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: ¬Found, + }, + } + + objs := []runtime.Object{&ns1VS1, &ns1VS1VSC, &ns1VS2, &ns1VS2VSC, &ns2VS1, &ns2VS1VSC, &ns2VS2, &ns2VS2VSC, &ns1NilStatusVS, &ns1NilBoundVSCVS, &ns1NonExistentVSCVS} + fakeClient := snapshotFake.NewSimpleClientset(objs...) + fakeSharedInformer := snapshotv1beta1informers.NewSharedInformerFactoryWithOptions(fakeClient, 0) + for _, o := range objs { + fakeSharedInformer.Snapshot().V1beta1().VolumeSnapshots().Informer().GetStore().Add(o) + } + + testCases := []struct { + name string + backup *velerov1.Backup + }{ + { + name: "should delete volumesnapshots bound to existing volumesnapshotcontent", + backup: builder.ForBackup("velero", "backup1").Result(), + }, + { + name: "should delete volumesnapshots with nil status", + backup: builder.ForBackup("velero", "backup2").Result(), + }, + { + name: "should delete volumesnapshots with nil BoundVolumeSnapshotContentName", + backup: builder.ForBackup("velero", "backup3").Result(), + }, + { + name: "should delete volumesnapshots bound to non-existent volumesnapshotcontents", + backup: builder.ForBackup("velero", "backup4").Result(), + }, + { + name: "should be a no-op when there are no volumesnapshots to delete", + backup: builder.ForBackup("velero", "backup-no-vs").Result(), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + log := velerotest.NewLogger().WithFields( + logrus.Fields{ + "unit-test": "unit-test", + }, + ) + errs := deleteCSIVolumeSnapshots(tc.backup, fakeSharedInformer.Snapshot().V1beta1().VolumeSnapshots().Lister(), fakeClient.SnapshotV1beta1(), log) + assert.Empty(t, errs) + }) + } +}