From 50ea4eea74949bab513c4a12910ef3179fbeb4b0 Mon Sep 17 00:00:00 2001 From: Adam Zhang Date: Mon, 8 Jun 2026 11:31:37 +0800 Subject: [PATCH 1/6] update codecov-action from v5 to v6 Signed-off-by: Adam Zhang --- .github/workflows/pr-ci-check.yml | 2 +- .github/workflows/push.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-ci-check.yml b/.github/workflows/pr-ci-check.yml index aea136329..ba55e6ab0 100644 --- a/.github/workflows/pr-ci-check.yml +++ b/.github/workflows/pr-ci-check.yml @@ -24,7 +24,7 @@ jobs: - name: Make ci run: make ci - name: Upload test coverage - uses: codecov/codecov-action@v5 + uses: codecov/codecov-action@v6 with: token: ${{ secrets.CODECOV_TOKEN }} files: coverage.out diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 10b191630..511113264 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -45,7 +45,7 @@ jobs: - name: Test run: make test - name: Upload test coverage - uses: codecov/codecov-action@v5 + uses: codecov/codecov-action@v6 with: token: ${{ secrets.CODECOV_TOKEN }} files: coverage.out From 52860f986efe1368f7da7db513bda83ae520e745 Mon Sep 17 00:00:00 2001 From: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com> Date: Tue, 9 Jun 2026 04:05:53 +0800 Subject: [PATCH 2/6] Use "go install" so the download goes through GOPROXY instead of the GitHub. (#9891) Signed-off-by: Xun Jiang --- hack/build-image/Dockerfile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hack/build-image/Dockerfile b/hack/build-image/Dockerfile index eb1d728f5..88dedde95 100644 --- a/hack/build-image/Dockerfile +++ b/hack/build-image/Dockerfile @@ -96,7 +96,9 @@ RUN ARCH=$(go env GOARCH) && \ chmod +x /usr/bin/goreleaser # get golangci-lint -RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.5.0 +# Use "go install" so the download goes through GOPROXY instead of the GitHub +# release API/CDN, which has been returning intermittent/persistent HTTP 504s. +RUN go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.5.0 # install kubectl RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/$(go env GOARCH)/kubectl From dda779de6501412ecbf33249847f98d074a0543e Mon Sep 17 00:00:00 2001 From: Subhramit Basu Date: Tue, 9 Jun 2026 01:40:32 +0530 Subject: [PATCH 3/6] Reject restores from backups not in a completed or partially failed phase (#9792) * Add phase check validations in restore controller Signed-off-by: subhramit * Adapt existing tests Signed-off-by: subhramit * Add tests Signed-off-by: subhramit * Update doc Signed-off-by: subhramit * Add changelog Signed-off-by: Subhramit Basu * Update pkg/controller/restore_controller_test.go Signed-off-by: Subhramit Basu --------- Signed-off-by: subhramit Signed-off-by: Subhramit Basu --- changelogs/unreleased/9792-subhramit | 1 + pkg/controller/restore_controller.go | 11 +++++ pkg/controller/restore_controller_test.go | 52 ++++++++++++++++++--- site/content/docs/main/restore-reference.md | 2 +- 4 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/9792-subhramit diff --git a/changelogs/unreleased/9792-subhramit b/changelogs/unreleased/9792-subhramit new file mode 100644 index 000000000..8868ed627 --- /dev/null +++ b/changelogs/unreleased/9792-subhramit @@ -0,0 +1 @@ +Restores from backups not in a completed or partially failed phase are now rejected. diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 469951f27..285b08281 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -399,6 +399,17 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf return backupInfo{}, nil } + // reject restores from backups that are not in a usable phase + switch info.backup.Status.Phase { + case api.BackupPhaseCompleted, api.BackupPhasePartiallyFailed: + // ok + default: + restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, + fmt.Sprintf("backup %q is in phase %q and cannot be used as a restore source", + info.backup.Name, info.backup.Status.Phase)) + return backupInfo{}, nil + } + // Fill in the ScheduleName so it's easier to consume for metrics. if restore.Spec.ScheduleName == "" { restore.Spec.ScheduleName = info.backup.GetLabels()[api.ScheduleNameLabel] diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index b013ee64d..6f03a6074 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -305,7 +305,7 @@ func TestRestoreReconcile(t *testing.T) { name: "restorer throwing an error causes the restore to fail", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), restorerError: errors.New("blarg"), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), @@ -319,7 +319,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore with none existingresourcepolicy gets executed", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).ExistingResourcePolicy("none").Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), expectedStartTime: ×tamp, @@ -330,7 +330,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore with update existingresourcepolicy gets executed", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).ExistingResourcePolicy("update").Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), expectedStartTime: ×tamp, @@ -352,7 +352,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore gets executed", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), expectedStartTime: ×tamp, @@ -363,7 +363,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore gets executed and only includes pod volume backups from restore namespace", location: defaultStorageLocation, restore: NewRestore("foo", "bar2", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), podVolumeBackups: []*velerov1api.PodVolumeBackup{ builder.ForPodVolumeBackup("foo", "pvb-1").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(), builder.ForPodVolumeBackup("other-ns", "pvb-2").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(), @@ -444,7 +444,7 @@ func TestRestoreReconcile(t *testing.T) { expectedStartTime: ×tamp, expectedCompletedTime: ×tamp, backupStoreGetBackupContentsErr: errors.New("Couldn't download backup"), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), }, { name: "restore attached with an expected finalizer gets cleaned up successfully", @@ -473,7 +473,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore with empty VolumeInfos", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), emptyVolumeInfo: true, expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), @@ -497,6 +497,44 @@ func TestRestoreReconcile(t *testing.T) { backup: defaultBackup().StorageLocation("default").Result(), expectedErr: true, }, + { + name: "restore from backup in Deleting phase fails validation", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseDeleting).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseFailedValidation), + expectedValidationErrors: []string{`backup "backup-1" is in phase "Deleting" and cannot be used as a restore source`}, + }, + { + name: "restore from backup in InProgress phase fails validation", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseInProgress).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseFailedValidation), + expectedValidationErrors: []string{`backup "backup-1" is in phase "InProgress" and cannot be used as a restore source`}, + }, + { + name: "restore from backup in PartiallyFailed phase succeeds", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhasePartiallyFailed).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseInProgress), + expectedStartTime: ×tamp, + expectedCompletedTime: ×tamp, + expectedRestorerCall: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseInProgress).Result(), + }, + { + name: "restore from backup in Failed phase fails validation", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseFailed).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseFailedValidation), + expectedValidationErrors: []string{`backup "backup-1" is in phase "Failed" and cannot be used as a restore source`}, + }, } formatFlag := logging.FormatText diff --git a/site/content/docs/main/restore-reference.md b/site/content/docs/main/restore-reference.md index b14a1dac9..cf9d34b51 100644 --- a/site/content/docs/main/restore-reference.md +++ b/site/content/docs/main/restore-reference.md @@ -27,7 +27,7 @@ The following is an overview of Velero's restore process that starts after you r 1. The Velero client makes a call to the Kubernetes API server to create a [`Restore`](api-types/restore.md) object. -1. The `RestoreController` notices the new Restore object and performs validation. +1. The `RestoreController` notices the new Restore object and performs validation. This includes verifying that the referenced backup is in a usable phase. Only backups in `Completed` or `PartiallyFailed` phase are accepted as restore sources. 1. The `RestoreController` fetches basic information about the backup being restored, like the [BackupStorageLocation](locations.md) (BSL). It also fetches a tarball of the cluster resources in the backup, any volumes that will be restored using File System Backup, and any volume snapshots to be restored. From 2ee99e75cd3dc3cf0b833171384774de11374e44 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Wed, 10 Jun 2026 00:04:25 +0800 Subject: [PATCH 4/6] Update restore-reference.md (#9893) This commit updates the doc to make the order of resources during restore is consistent with the code. Signed-off-by: Daniel Jiang --- site/content/docs/main/restore-reference.md | 25 ++++++++++++++++---- site/content/docs/v1.17/restore-reference.md | 25 ++++++++++++++++---- site/content/docs/v1.18/restore-reference.md | 25 ++++++++++++++++---- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/site/content/docs/main/restore-reference.md b/site/content/docs/main/restore-reference.md index cf9d34b51..eec8ad965 100644 --- a/site/content/docs/main/restore-reference.md +++ b/site/content/docs/main/restore-reference.md @@ -78,26 +78,41 @@ By default, Velero will restore resources in the following order: * VolumeSnapshotClass * VolumeSnapshotContents * VolumeSnapshots +* DataUploads * PersistentVolumes * PersistentVolumeClaims +* ClusterRoles +* Roles +* ServiceAccounts +* ClusterRoleBindings +* RoleBindings * Secrets * ConfigMaps -* ServiceAccounts * LimitRanges +* PriorityClasses * Pods * ReplicaSets +* ClusterClasses +* Endpoints +* Services +* ClusterBootstraps * Clusters * ClusterResourceSets +* Apps (apps.kappctrl.k14s.io) +* PackageInstalls -It's recommended that you use the default order for your restores. You are able to customize this order if you need to by setting the `--restore-resource-priorities` flag on the Velero server and specifying a different resource order. This customized order will apply to all future restores. You don't have to specify all resources in the `--restore-resource-priorities` flag. Velero will append resources not listed to the end of your customized list in alphabetical order. +It's recommended that you use the default order for your restores. You are able to customize this order if you need to by setting the `--restore-resource-priorities` flag on the Velero server and specifying a different resource order. This customized order will apply to all future restores. You don't have to specify all resources in the `--restore-resource-priorities` flag. The priority list contains two parts which are split by the `-` element: resources before the `-` element are restored first as high priorities, resources after the `-` element are restored last as low priorities, and any resource not in the list will be restored alphabetically between the high and low priorities. ```shell velero server \ --restore-resource-priorities=customresourcedefinitions,namespaces,storageclasses,\ volumesnapshotclass.snapshot.storage.k8s.io,volumesnapshotcontents.snapshot.storage.k8s.io,\ -volumesnapshots.snapshot.storage.k8s.io,persistentvolumes,persistentvolumeclaims,secrets,\ -configmaps,serviceaccounts,limitranges,pods,replicasets.apps,clusters.cluster.x-k8s.io,\ -clusterresourcesets.addons.cluster.x-k8s.io +volumesnapshots.snapshot.storage.k8s.io,datauploads.velero.io,persistentvolumes,\ +persistentvolumeclaims,clusterroles,roles,serviceaccounts,clusterrolebindings,rolebindings,\ +secrets,configmaps,limitranges,priorityclasses,pods,replicasets.apps,\ +clusterclasses.cluster.x-k8s.io,endpoints,services,-,clusterbootstraps.run.tanzu.vmware.com,\ +clusters.cluster.x-k8s.io,clusterresourcesets.addons.cluster.x-k8s.io,apps.kappctrl.k14s.io,\ +packageinstalls.packaging.carvel.dev ``` diff --git a/site/content/docs/v1.17/restore-reference.md b/site/content/docs/v1.17/restore-reference.md index b062175cc..92a5f538b 100644 --- a/site/content/docs/v1.17/restore-reference.md +++ b/site/content/docs/v1.17/restore-reference.md @@ -78,26 +78,41 @@ By default, Velero will restore resources in the following order: * VolumeSnapshotClass * VolumeSnapshotContents * VolumeSnapshots +* DataUploads * PersistentVolumes * PersistentVolumeClaims +* ClusterRoles +* Roles +* ServiceAccounts +* ClusterRoleBindings +* RoleBindings * Secrets * ConfigMaps -* ServiceAccounts * LimitRanges +* PriorityClasses * Pods * ReplicaSets +* ClusterClasses +* Endpoints +* Services +* ClusterBootstraps * Clusters * ClusterResourceSets +* Apps (apps.kappctrl.k14s.io) +* PackageInstalls -It's recommended that you use the default order for your restores. You are able to customize this order if you need to by setting the `--restore-resource-priorities` flag on the Velero server and specifying a different resource order. This customized order will apply to all future restores. You don't have to specify all resources in the `--restore-resource-priorities` flag. Velero will append resources not listed to the end of your customized list in alphabetical order. +It's recommended that you use the default order for your restores. You are able to customize this order if you need to by setting the `--restore-resource-priorities` flag on the Velero server and specifying a different resource order. This customized order will apply to all future restores. You don't have to specify all resources in the `--restore-resource-priorities` flag. The priority list contains two parts which are split by the `-` element: resources before the `-` element are restored first as high priorities, resources after the `-` element are restored last as low priorities, and any resource not in the list will be restored alphabetically between the high and low priorities. ```shell velero server \ --restore-resource-priorities=customresourcedefinitions,namespaces,storageclasses,\ volumesnapshotclass.snapshot.storage.k8s.io,volumesnapshotcontents.snapshot.storage.k8s.io,\ -volumesnapshots.snapshot.storage.k8s.io,persistentvolumes,persistentvolumeclaims,secrets,\ -configmaps,serviceaccounts,limitranges,pods,replicasets.apps,clusters.cluster.x-k8s.io,\ -clusterresourcesets.addons.cluster.x-k8s.io +volumesnapshots.snapshot.storage.k8s.io,datauploads.velero.io,persistentvolumes,\ +persistentvolumeclaims,clusterroles,roles,serviceaccounts,clusterrolebindings,rolebindings,\ +secrets,configmaps,limitranges,priorityclasses,pods,replicasets.apps,\ +clusterclasses.cluster.x-k8s.io,endpoints,services,-,clusterbootstraps.run.tanzu.vmware.com,\ +clusters.cluster.x-k8s.io,clusterresourcesets.addons.cluster.x-k8s.io,apps.kappctrl.k14s.io,\ +packageinstalls.packaging.carvel.dev ``` diff --git a/site/content/docs/v1.18/restore-reference.md b/site/content/docs/v1.18/restore-reference.md index b14a1dac9..2726f8a9f 100644 --- a/site/content/docs/v1.18/restore-reference.md +++ b/site/content/docs/v1.18/restore-reference.md @@ -78,26 +78,41 @@ By default, Velero will restore resources in the following order: * VolumeSnapshotClass * VolumeSnapshotContents * VolumeSnapshots +* DataUploads * PersistentVolumes * PersistentVolumeClaims +* ClusterRoles +* Roles +* ServiceAccounts +* ClusterRoleBindings +* RoleBindings * Secrets * ConfigMaps -* ServiceAccounts * LimitRanges +* PriorityClasses * Pods * ReplicaSets +* ClusterClasses +* Endpoints +* Services +* ClusterBootstraps * Clusters * ClusterResourceSets +* Apps (apps.kappctrl.k14s.io) +* PackageInstalls -It's recommended that you use the default order for your restores. You are able to customize this order if you need to by setting the `--restore-resource-priorities` flag on the Velero server and specifying a different resource order. This customized order will apply to all future restores. You don't have to specify all resources in the `--restore-resource-priorities` flag. Velero will append resources not listed to the end of your customized list in alphabetical order. +It's recommended that you use the default order for your restores. You are able to customize this order if you need to by setting the `--restore-resource-priorities` flag on the Velero server and specifying a different resource order. This customized order will apply to all future restores. You don't have to specify all resources in the `--restore-resource-priorities` flag. The priority list contains two parts which are split by the `-` element: resources before the `-` element are restored first as high priorities, resources after the `-` element are restored last as low priorities, and any resource not in the list will be restored alphabetically between the high and low priorities. ```shell velero server \ --restore-resource-priorities=customresourcedefinitions,namespaces,storageclasses,\ volumesnapshotclass.snapshot.storage.k8s.io,volumesnapshotcontents.snapshot.storage.k8s.io,\ -volumesnapshots.snapshot.storage.k8s.io,persistentvolumes,persistentvolumeclaims,secrets,\ -configmaps,serviceaccounts,limitranges,pods,replicasets.apps,clusters.cluster.x-k8s.io,\ -clusterresourcesets.addons.cluster.x-k8s.io +volumesnapshots.snapshot.storage.k8s.io,datauploads.velero.io,persistentvolumes,\ +persistentvolumeclaims,clusterroles,roles,serviceaccounts,clusterrolebindings,rolebindings,\ +secrets,configmaps,limitranges,priorityclasses,pods,replicasets.apps,\ +clusterclasses.cluster.x-k8s.io,endpoints,services,-,clusterbootstraps.run.tanzu.vmware.com,\ +clusters.cluster.x-k8s.io,clusterresourcesets.addons.cluster.x-k8s.io,apps.kappctrl.k14s.io,\ +packageinstalls.packaging.carvel.dev ``` From 92123d3d465d53df2bbf5d45815ba4a08ea4c1d8 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Tue, 9 Jun 2026 10:21:55 -0700 Subject: [PATCH 5/6] Skip VGS cleanup when backup did not use VolumeGroupSnapshots Guard the cleanupStubVGSC() call in restore finalization with a check for VolumeGroupSnapshotHandle in volumeInfo. This avoids a spurious warning on clusters where the v1beta2 VolumeGroupSnapshotContent CRD is not installed, since the List call would fail even though no stubs exist to clean up. Fixes #9882 Signed-off-by: Shubham Pampattiwar --- .../restore_finalizer_controller.go | 15 +++- .../restore_finalizer_controller_test.go | 77 +++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/pkg/controller/restore_finalizer_controller.go b/pkg/controller/restore_finalizer_controller.go index f82216bc3..93652c0c2 100644 --- a/pkg/controller/restore_finalizer_controller.go +++ b/pkg/controller/restore_finalizer_controller.go @@ -301,8 +301,10 @@ func (ctx *finalizerContext) execute() (results.Result, results.Result) { pdpErrs := ctx.patchDynamicPVWithVolumeInfo() errs.Merge(&pdpErrs) - vgscWarnings := ctx.cleanupStubVGSC() - warnings.Merge(&vgscWarnings) + if ctx.hasVolumeGroupSnapshotHandles() { + vgscWarnings := ctx.cleanupStubVGSC() + warnings.Merge(&vgscWarnings) + } rehErrs := ctx.WaitRestoreExecHook() errs.Merge(&rehErrs) @@ -449,6 +451,15 @@ func (ctx *finalizerContext) patchDynamicPVWithVolumeInfo() (errs results.Result return errs } +func (ctx *finalizerContext) hasVolumeGroupSnapshotHandles() bool { + for _, vi := range ctx.volumeInfo { + if vi.CSISnapshotInfo != nil && vi.CSISnapshotInfo.VolumeGroupSnapshotHandle != "" { + return true + } + } + return false +} + // 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. diff --git a/pkg/controller/restore_finalizer_controller_test.go b/pkg/controller/restore_finalizer_controller_test.go index 6fb5ba303..f07d2576c 100644 --- a/pkg/controller/restore_finalizer_controller_test.go +++ b/pkg/controller/restore_finalizer_controller_test.go @@ -743,6 +743,83 @@ func TestRestoreOperationList(t *testing.T) { } } +func TestHasVolumeGroupSnapshotHandles(t *testing.T) { + tests := []struct { + name string + volumeInfo []*volume.BackupVolumeInfo + expected bool + }{ + { + name: "nil volumeInfo", + volumeInfo: nil, + expected: false, + }, + { + name: "empty volumeInfo", + volumeInfo: []*volume.BackupVolumeInfo{}, + expected: false, + }, + { + name: "no CSISnapshotInfo", + volumeInfo: []*volume.BackupVolumeInfo{ + {PVCName: "pvc-1", BackupMethod: volume.NativeSnapshot}, + }, + expected: false, + }, + { + name: "CSISnapshotInfo with empty VolumeGroupSnapshotHandle", + volumeInfo: []*volume.BackupVolumeInfo{ + { + PVCName: "pvc-1", + BackupMethod: volume.CSISnapshot, + CSISnapshotInfo: &volume.CSISnapshotInfo{ + SnapshotHandle: "snap-1", + }, + }, + }, + expected: false, + }, + { + name: "one volume with VolumeGroupSnapshotHandle", + volumeInfo: []*volume.BackupVolumeInfo{ + { + PVCName: "pvc-1", + BackupMethod: volume.CSISnapshot, + CSISnapshotInfo: &volume.CSISnapshotInfo{ + SnapshotHandle: "snap-1", + VolumeGroupSnapshotHandle: "vgs-handle-1", + }, + }, + }, + expected: true, + }, + { + name: "mixed volumes only one with VolumeGroupSnapshotHandle", + volumeInfo: []*volume.BackupVolumeInfo{ + {PVCName: "pvc-1", BackupMethod: volume.NativeSnapshot}, + { + PVCName: "pvc-2", + BackupMethod: volume.CSISnapshot, + CSISnapshotInfo: &volume.CSISnapshotInfo{ + SnapshotHandle: "snap-2", + VolumeGroupSnapshotHandle: "vgs-handle-2", + }, + }, + }, + expected: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := &finalizerContext{ + volumeInfo: tc.volumeInfo, + } + assert.Equal(t, tc.expected, ctx.hasVolumeGroupSnapshotHandles()) + }) + } +} + func TestCleanupStubVGSC(t *testing.T) { snapshotHandle1 := "snap-handle-1" snapshotHandle2 := "snap-handle-2" From 22e31de11b382d84147e53a56e59e023777060d2 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Tue, 9 Jun 2026 10:23:22 -0700 Subject: [PATCH 6/6] Add changelog for PR #9896 Signed-off-by: Shubham Pampattiwar --- changelogs/unreleased/9896-shubham-pampattiwar | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/9896-shubham-pampattiwar diff --git a/changelogs/unreleased/9896-shubham-pampattiwar b/changelogs/unreleased/9896-shubham-pampattiwar new file mode 100644 index 000000000..034507c1f --- /dev/null +++ b/changelogs/unreleased/9896-shubham-pampattiwar @@ -0,0 +1 @@ +Skip VGS cleanup when backup did not use VolumeGroupSnapshots