From 1b7175394af1592c2a0d04ca109ec238ec4b6729 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Fri, 30 May 2025 22:21:19 +0800 Subject: [PATCH] Skip VS and VSC not created by backup. Signed-off-by: Xun Jiang --- changelogs/unreleased/8990-blackpiglet | 1 + pkg/backup/actions/csi/pvc_action.go | 1 - .../actions/csi/volumesnapshot_action.go | 111 +++++++---------- .../actions/csi/volumesnapshot_action_test.go | 17 --- pkg/controller/backup_controller.go | 94 ++++++++++++-- pkg/controller/backup_controller_test.go | 115 ++++++++++++++++++ pkg/util/csi/volume_snapshot.go | 24 ---- pkg/util/csi/volume_snapshot_test.go | 41 +------ 8 files changed, 244 insertions(+), 160 deletions(-) create mode 100644 changelogs/unreleased/8990-blackpiglet diff --git a/changelogs/unreleased/8990-blackpiglet b/changelogs/unreleased/8990-blackpiglet new file mode 100644 index 000000000..ec805dc58 --- /dev/null +++ b/changelogs/unreleased/8990-blackpiglet @@ -0,0 +1 @@ +Skip VS and VSC not created by backup. diff --git a/pkg/backup/actions/csi/pvc_action.go b/pkg/backup/actions/csi/pvc_action.go index ffe31d90a..d1d8056a7 100644 --- a/pkg/backup/actions/csi/pvc_action.go +++ b/pkg/backup/actions/csi/pvc_action.go @@ -285,7 +285,6 @@ func (p *pvcBackupItemAction) Execute( vs, p.crClient, p.log, - true, backup.Spec.CSISnapshotTimeout.Duration, ) if err != nil { diff --git a/pkg/backup/actions/csi/volumesnapshot_action.go b/pkg/backup/actions/csi/volumesnapshot_action.go index 7a3bc79d7..595606a39 100644 --- a/pkg/backup/actions/csi/volumesnapshot_action.go +++ b/pkg/backup/actions/csi/volumesnapshot_action.go @@ -96,22 +96,6 @@ func (p *volumeSnapshotBackupItemAction) Execute( }, } - // determine if we are backing up a VolumeSnapshot that was created by velero while - // performing backup of a CSI backed PVC. - // For VolumeSnapshots that were created during the backup of a CSI backed PVC, - // we will wait for the VolumeSnapshotContents to be available. - // For VolumeSnapshots created outside of velero, we expect the VolumeSnapshotContent - // to be available prior to backing up the VolumeSnapshot. In case of a failure, - // backup should be re-attempted after the CSI driver has reconciled the VolumeSnapshot. - // existence of the velerov1api.BackupNameLabel indicates that the VolumeSnapshot was - // created while backing up a CSI backed PVC. - - // We want to await reconciliation of only those VolumeSnapshots created during the - // ongoing backup. For this we will wait only if the backup label exists on the - // VolumeSnapshot object and the backup name is the same as that of the value of the - // backup label. - backupOngoing := vs.Labels[velerov1api.BackupNameLabel] == label.GetValidName(backup.Name) - p.log.Infof("Getting VolumesnapshotContent for Volumesnapshot %s/%s", vs.Namespace, vs.Name) @@ -119,7 +103,6 @@ func (p *volumeSnapshotBackupItemAction) Execute( vs, p.crClient, p.log, - backupOngoing, backup.Spec.CSISnapshotTimeout.Duration, ) if err != nil { @@ -171,42 +154,40 @@ func (p *volumeSnapshotBackupItemAction) Execute( } } - if backupOngoing { - p.log.Infof("Patching VolumeSnapshotContent %s with velero BackupNameLabel", - vsc.Name) - // If we created the VolumeSnapshotContent object during this ongoing backup, - // we would have created it with a DeletionPolicy of Retain. - // But, we want to retain these VolumeSnapshotContent ONLY for the lifetime - // of the backup. To that effect, during velero backup - // deletion, we will update the DeletionPolicy of the VolumeSnapshotContent - // and then delete the VolumeSnapshot object which will cascade delete the - // VolumeSnapshotContent and the associated snapshot in the storage - // provider (handled by the CSI driver and the CSI common controller). - // However, in the event that the VolumeSnapshot object is deleted outside - // of the backup deletion process, it is possible that the dynamically created - // VolumeSnapshotContent object will be left as an orphaned and non-discoverable - // resource in the cluster as well as in the storage provider. To avoid piling - // up of such orphaned resources, we will want to discover and delete the - // dynamically created VolumeSnapshotContent. We do that by adding - // the "velero.io/backup-name" label on the VolumeSnapshotContent. - // Further, we want to add this label only on VolumeSnapshotContents that - // were created during an ongoing velero backup. - originVSC := vsc.DeepCopy() - kubeutil.AddLabels( - &vsc.ObjectMeta, - map[string]string{ - velerov1api.BackupNameLabel: label.GetValidName(backup.Name), - }, - ) + p.log.Infof("Patching VolumeSnapshotContent %s with velero BackupNameLabel", + vsc.Name) + // If we created the VolumeSnapshotContent object during this ongoing backup, + // we would have created it with a DeletionPolicy of Retain. + // But, we want to retain these VolumeSnapshotContent ONLY for the lifetime + // of the backup. To that effect, during velero backup + // deletion, we will update the DeletionPolicy of the VolumeSnapshotContent + // and then delete the VolumeSnapshot object which will cascade delete the + // VolumeSnapshotContent and the associated snapshot in the storage + // provider (handled by the CSI driver and the CSI common controller). + // However, in the event that the VolumeSnapshot object is deleted outside + // of the backup deletion process, it is possible that the dynamically created + // VolumeSnapshotContent object will be left as an orphaned and non-discoverable + // resource in the cluster as well as in the storage provider. To avoid piling + // up of such orphaned resources, we will want to discover and delete the + // dynamically created VolumeSnapshotContent. We do that by adding + // the "velero.io/backup-name" label on the VolumeSnapshotContent. + // Further, we want to add this label only on VolumeSnapshotContents that + // were created during an ongoing velero backup. + originVSC := vsc.DeepCopy() + kubeutil.AddLabels( + &vsc.ObjectMeta, + map[string]string{ + velerov1api.BackupNameLabel: label.GetValidName(backup.Name), + }, + ) - if vscPatchError := p.crClient.Patch( - context.TODO(), - vsc, - crclient.MergeFrom(originVSC), - ); vscPatchError != nil { - p.log.Warnf("Failed to patch VolumeSnapshotContent %s: %v", - vsc.Name, vscPatchError) - } + if vscPatchError := p.crClient.Patch( + context.TODO(), + vsc, + crclient.MergeFrom(originVSC), + ); vscPatchError != nil { + p.log.Warnf("Failed to patch VolumeSnapshotContent %s: %v", + vsc.Name, vscPatchError) } } @@ -244,20 +225,18 @@ func (p *volumeSnapshotBackupItemAction) Execute( var itemToUpdate []velero.ResourceIdentifier // Only return Async operation for VSC created for this backup. - if backupOngoing { - // The operationID is of the form // - operationID = vs.Namespace + "/" + vs.Name + "/" + time.Now().Format(time.RFC3339) - itemToUpdate = []velero.ResourceIdentifier{ - { - GroupResource: kuberesource.VolumeSnapshots, - Namespace: vs.Namespace, - Name: vs.Name, - }, - { - GroupResource: kuberesource.VolumeSnapshotContents, - Name: vsc.Name, - }, - } + // The operationID is of the form // + operationID = vs.Namespace + "/" + vs.Name + "/" + time.Now().Format(time.RFC3339) + itemToUpdate = []velero.ResourceIdentifier{ + { + GroupResource: kuberesource.VolumeSnapshots, + Namespace: vs.Namespace, + Name: vs.Name, + }, + { + GroupResource: kuberesource.VolumeSnapshotContents, + Name: vsc.Name, + }, } return &unstructured.Unstructured{Object: vsMap}, diff --git a/pkg/backup/actions/csi/volumesnapshot_action_test.go b/pkg/backup/actions/csi/volumesnapshot_action_test.go index 5abf7ac9e..a9c666123 100644 --- a/pkg/backup/actions/csi/volumesnapshot_action_test.go +++ b/pkg/backup/actions/csi/volumesnapshot_action_test.go @@ -49,23 +49,6 @@ func TestVSExecute(t *testing.T) { expectedAdditionalItems []velero.ResourceIdentifier expectedItemToUpdate []velero.ResourceIdentifier }{ - { - name: "VS not created by backup, has no status. Backup is finalizing", - backup: builder.ForBackup("velero", "backup"). - Phase(velerov1api.BackupPhaseFinalizing).Result(), - vs: builder.ForVolumeSnapshot("velero", "vs"). - VolumeSnapshotClass("class").Result(), - expectedErr: "", - }, - { - name: "VS is not created by the backup, associated VSC not exists", - backup: builder.ForBackup("velero", "backup"). - Phase(velerov1api.BackupPhaseInProgress).Result(), - vs: builder.ForVolumeSnapshot("velero", "vs"). - VolumeSnapshotClass("class").Status(). - BoundVolumeSnapshotContentName("vsc").Result(), - expectedErr: `error getting volume snapshot content from API: volumesnapshotcontents.snapshot.storage.k8s.io "vsc" not found`, - }, { name: "Normal case", backup: builder.ForBackup("velero", "backup"). diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index e2b624b0a..bc296f790 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -21,6 +21,7 @@ import ( "context" "fmt" "os" + "slices" "time" snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v7/apis/volumesnapshot/v1" @@ -31,6 +32,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" kerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" @@ -63,6 +65,20 @@ const ( backupResyncPeriod = time.Minute ) +var autoExcludeNamespaceScopedResources = []string{ + // CSI VolumeSnapshot and VolumeSnapshotContent are intermediate resources. + // Velero only handle the VS and VSC created during backup, + // not during resource collecting. + "volumesnapshots.snapshot.storage.k8s.io", +} + +var autoExcludeClusterScopedResources = []string{ + // CSI VolumeSnapshot and VolumeSnapshotContent are intermediate resources. + // Velero only handle the VS and VSC created during backup, + // not during resource collecting. + "volumesnapshotcontents.snapshot.storage.k8s.io", +} + type backupReconciler struct { ctx context.Context logger logrus.FieldLogger @@ -481,19 +497,51 @@ func (b *backupReconciler) prepareBackupRequest(backup *velerov1api.Backup, logg request.Status.ValidationErrors = append(request.Status.ValidationErrors, validatedError) } - // validate the included/excluded resources - for _, err := range collections.ValidateIncludesExcludes(request.Spec.IncludedResources, request.Spec.ExcludedResources) { - request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded resource lists: %v", err)) - } + if collections.UseOldResourceFilters(request.Spec) { + // validate the included/excluded resources + ieErr := collections.ValidateIncludesExcludes(request.Spec.IncludedResources, request.Spec.ExcludedResources) + if len(ieErr) > 0 { + for _, err := range ieErr { + request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded resource lists: %v", err)) + } + } else { + request.Spec.IncludedResources, request.Spec.ExcludedResources = + modifyResourceIncludeExclude( + request.Spec.IncludedResources, + request.Spec.ExcludedResources, + append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), + ) + } + } else { + // validate the cluster-scoped included/excluded resources + clusterErr := collections.ValidateScopedIncludesExcludes(request.Spec.IncludedClusterScopedResources, request.Spec.ExcludedClusterScopedResources) + if len(clusterErr) > 0 { + for _, err := range clusterErr { + request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid cluster-scoped included/excluded resource lists: %s", err)) + } + } else { + request.Spec.IncludedClusterScopedResources, request.Spec.ExcludedClusterScopedResources = + modifyResourceIncludeExclude( + request.Spec.IncludedClusterScopedResources, + request.Spec.ExcludedClusterScopedResources, + autoExcludeClusterScopedResources, + ) + } - // validate the cluster-scoped included/excluded resources - for _, err := range collections.ValidateScopedIncludesExcludes(request.Spec.IncludedClusterScopedResources, request.Spec.ExcludedClusterScopedResources) { - request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid cluster-scoped included/excluded resource lists: %s", err)) - } - - // validate the namespace-scoped included/excluded resources - for _, err := range collections.ValidateScopedIncludesExcludes(request.Spec.IncludedNamespaceScopedResources, request.Spec.ExcludedNamespaceScopedResources) { - request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid namespace-scoped included/excluded resource lists: %s", err)) + // validate the namespace-scoped included/excluded resources + namespaceErr := collections.ValidateScopedIncludesExcludes(request.Spec.IncludedNamespaceScopedResources, request.Spec.ExcludedNamespaceScopedResources) + if len(namespaceErr) > 0 { + for _, err := range namespaceErr { + request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid namespace-scoped included/excluded resource lists: %s", err)) + } + } else { + request.Spec.IncludedNamespaceScopedResources, request.Spec.ExcludedNamespaceScopedResources = + modifyResourceIncludeExclude( + request.Spec.IncludedNamespaceScopedResources, + request.Spec.ExcludedNamespaceScopedResources, + autoExcludeNamespaceScopedResources, + ) + } } // validate the included/excluded namespaces @@ -932,3 +980,25 @@ func oldAndNewFilterParametersUsedTogether(backupSpec velerov1api.BackupSpec) bo return haveOldResourceFilterParameters && haveNewResourceFilterParameters } + +func modifyResourceIncludeExclude(include, exclude, addedExclude []string) (modifiedInclude, modifiedExclude []string) { + modifiedInclude = include + modifiedExclude = exclude + + excludeStrSet := sets.NewString(exclude...) + for _, ex := range addedExclude { + if !excludeStrSet.Has(ex) { + modifiedExclude = append(modifiedExclude, ex) + } + } + + for _, exElem := range modifiedExclude { + for inIndex, inElem := range modifiedInclude { + if inElem == exElem { + modifiedInclude = slices.Delete(modifiedInclude, inIndex, inIndex+1) + } + } + } + + return modifiedInclude, modifiedExclude +} diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index b5ee3dbf1..e8a28282d 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -708,6 +708,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -745,6 +746,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: "alt-loc", DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -786,6 +788,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: "read-write", DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -824,6 +827,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -862,6 +866,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -901,6 +906,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -940,6 +946,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -979,6 +986,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1018,6 +1026,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1058,6 +1067,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -1098,6 +1108,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -1138,6 +1149,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1179,6 +1191,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1220,6 +1233,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1261,6 +1275,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1303,6 +1318,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1344,6 +1360,105 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), + ExcludedResources: append(autoExcludeNamespaceScopedResources, autoExcludeClusterScopedResources...), + }, + Status: velerov1api.BackupStatus{ + Phase: velerov1api.BackupPhaseFinalizing, + Version: 1, + FormatVersion: "1.1.0", + StartTimestamp: ×tamp, + Expiration: ×tamp, + CSIVolumeSnapshotsAttempted: 0, + CSIVolumeSnapshotsCompleted: 0, + }, + }, + volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").VolumeSnapshotClass("testClass").Status().BoundVolumeSnapshotContentName("testVSC").RestoreSize("10G").SourcePVC("testPVC").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(), + }, + { + name: "backup with namespace-scoped and cluster-scoped resource filters", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1"). + ExcludedClusterScopedResources("clusterroles"). + IncludedClusterScopedResources("storageclasses"). + ExcludedNamespaceScopedResources("secrets"). + IncludedNamespaceScopedResources("pods").Result(), + backupLocation: defaultBackupLocation, + defaultVolumesToFsBackup: false, + defaultSnapshotMoveData: true, + expectedResult: &velerov1api.Backup{ + TypeMeta: metav1.TypeMeta{ + Kind: "Backup", + APIVersion: "velero.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "backup-1", + Annotations: map[string]string{ + "velero.io/source-cluster-k8s-major-version": "1", + "velero.io/source-cluster-k8s-minor-version": "16", + "velero.io/source-cluster-k8s-gitversion": "v1.16.4", + "velero.io/resource-timeout": "0s", + }, + Labels: map[string]string{ + "velero.io/storage-location": "loc-1", + }, + }, + Spec: velerov1api.BackupSpec{ + StorageLocation: defaultBackupLocation.Name, + DefaultVolumesToFsBackup: boolptr.False(), + SnapshotMoveData: boolptr.True(), + IncludedClusterScopedResources: []string{"storageclasses"}, + ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), + IncludedNamespaceScopedResources: []string{"pods"}, + ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...), + }, + Status: velerov1api.BackupStatus{ + Phase: velerov1api.BackupPhaseFinalizing, + Version: 1, + FormatVersion: "1.1.0", + StartTimestamp: ×tamp, + Expiration: ×tamp, + CSIVolumeSnapshotsAttempted: 0, + CSIVolumeSnapshotsCompleted: 0, + }, + }, + volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").VolumeSnapshotClass("testClass").Status().BoundVolumeSnapshotContentName("testVSC").RestoreSize("10G").SourcePVC("testPVC").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(), + }, + { + name: "backup's include filter overlap with default exclude resources", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1"). + ExcludedClusterScopedResources("clusterroles"). + IncludedClusterScopedResources("storageclasses", "volumesnapshotcontents.snapshot.storage.k8s.io"). + ExcludedNamespaceScopedResources("secrets"). + IncludedNamespaceScopedResources("pods", "volumesnapshots.snapshot.storage.k8s.io").Result(), + backupLocation: defaultBackupLocation, + defaultVolumesToFsBackup: false, + defaultSnapshotMoveData: true, + expectedResult: &velerov1api.Backup{ + TypeMeta: metav1.TypeMeta{ + Kind: "Backup", + APIVersion: "velero.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "backup-1", + Annotations: map[string]string{ + "velero.io/source-cluster-k8s-major-version": "1", + "velero.io/source-cluster-k8s-minor-version": "16", + "velero.io/source-cluster-k8s-gitversion": "v1.16.4", + "velero.io/resource-timeout": "0s", + }, + Labels: map[string]string{ + "velero.io/storage-location": "loc-1", + }, + }, + Spec: velerov1api.BackupSpec{ + StorageLocation: defaultBackupLocation.Name, + DefaultVolumesToFsBackup: boolptr.False(), + SnapshotMoveData: boolptr.True(), + IncludedClusterScopedResources: []string{"storageclasses"}, + ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), + IncludedNamespaceScopedResources: []string{"pods"}, + ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, diff --git a/pkg/util/csi/volume_snapshot.go b/pkg/util/csi/volume_snapshot.go index f3d1f1a8b..3363ce632 100644 --- a/pkg/util/csi/volume_snapshot.go +++ b/pkg/util/csi/volume_snapshot.go @@ -588,32 +588,8 @@ func WaitUntilVSCHandleIsReady( volSnap *snapshotv1api.VolumeSnapshot, crClient crclient.Client, log logrus.FieldLogger, - shouldWait bool, csiSnapshotTimeout time.Duration, ) (*snapshotv1api.VolumeSnapshotContent, error) { - if !shouldWait { - if volSnap.Status == nil || - volSnap.Status.BoundVolumeSnapshotContentName == nil { - // volumesnapshot hasn't been reconciled and we're - // not waiting for it. - return nil, nil - } - vsc := new(snapshotv1api.VolumeSnapshotContent) - err := crClient.Get( - context.TODO(), - crclient.ObjectKey{ - Name: *volSnap.Status.BoundVolumeSnapshotContentName, - }, - vsc, - ) - if err != nil { - return nil, - errors.Wrap(err, - "error getting volume snapshot content from API") - } - return vsc, nil - } - // We'll wait 10m for the VSC to be reconciled polling // every 5s unless backup's csiSnapshotTimeout is set interval := 5 * time.Second diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go index ba24a91e7..8c736d482 100644 --- a/pkg/util/csi/volume_snapshot_test.go +++ b/pkg/util/csi/volume_snapshot_test.go @@ -1646,26 +1646,22 @@ func TestWaitUntilVSCHandleIsReady(t *testing.T) { name string volSnap *snapshotv1api.VolumeSnapshot exepctedVSC *snapshotv1api.VolumeSnapshotContent - wait bool expectError bool }{ { name: "waitEnabled should find volumesnapshotcontent for volumesnapshot", volSnap: validVS, exepctedVSC: vscObj, - wait: true, expectError: false, }, { name: "waitEnabled should not find volumesnapshotcontent for volumesnapshot with non-existing snapshotcontent name in status.BoundVolumeSnapshotContentName", volSnap: vsWithVSCNotFound, exepctedVSC: nil, - wait: true, expectError: true, }, { name: "waitEnabled should not find volumesnapshotcontent for a non-existent volumesnapshot", - wait: true, exepctedVSC: nil, expectError: true, volSnap: &snapshotv1api.VolumeSnapshot{ @@ -1678,46 +1674,11 @@ func TestWaitUntilVSCHandleIsReady(t *testing.T) { }, }, }, - { - name: "waitDisabled should not find volumesnapshotcontent when volumesnapshot status is nil", - wait: false, - expectError: false, - exepctedVSC: nil, - volSnap: vsWithNilStatus, - }, - { - name: "waitDisabled should not find volumesnapshotcontent when volumesnapshot status.BoundVolumeSnapshotContentName is nil", - wait: false, - expectError: false, - exepctedVSC: nil, - volSnap: vsWithNilStatusField, - }, - { - name: "waitDisabled should find volumesnapshotcontent when volumesnapshotcontent status is nil", - wait: false, - expectError: false, - exepctedVSC: vscWithNilStatus, - volSnap: vsForNilStatusVsc, - }, - { - name: "waitDisabled should find volumesnapshotcontent when volumesnapshotcontent status.SnapshotHandle is nil", - wait: false, - expectError: false, - exepctedVSC: vscWithNilStatusField, - volSnap: vsForNilStatusFieldVsc, - }, - { - name: "waitDisabled should not find a non-existent volumesnapshotcontent", - wait: false, - exepctedVSC: nil, - expectError: true, - volSnap: vsWithVSCNotFound, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actualVSC, actualError := WaitUntilVSCHandleIsReady(tc.volSnap, fakeClient, logrus.New().WithField("fake", "test"), tc.wait, 0) + actualVSC, actualError := WaitUntilVSCHandleIsReady(tc.volSnap, fakeClient, logrus.New().WithField("fake", "test"), 0) if tc.expectError && actualError == nil { assert.Error(t, actualError) assert.Nil(t, actualVSC)