This commit makes change to the code, scripts and some template files to
reflect the change of the org after donation to CNCF.
The files under "/site" "/changelogs" and "/design" are deliberately
excluded in this change. I'll make follow up changes in these
directories when needed.
Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>
* fix: defer VGS cleanup until backup finalization
The CSI PVC backup action deleted the VolumeGroupSnapshot and bound
VolumeGroupSnapshotContent after creating and waiting for all group
member VolumeSnapshots. At that point, Velero had not necessarily
completed processing the member VolumeSnapshots, CSI async operations,
or archive finalization.
Deleting the VGS triggered external-snapshotter member cleanup and could
remove VolumeSnapshot API objects still referenced by the Backup
workflow.
Remove immediate VGS/VGSC deletion from getVolumeSnapshotReference and
retain the group until the Backup reaches a terminal phase. Continue
processing VGS member VolumeSnapshots through the existing normal CSI
finalization path.
Add the velero.io/volume-group-snapshot-cleanup Backup finalizer before
VGS creation. Label VGSC resources with the Backup name and UID, enforce
DeletionPolicy=Retain before cleanup, and delete VGS/VGSC resources from
terminal Backup reconciliation only.
Make cleanup idempotent and retryable, including recovery of labeled
orphan VGSC resources. Remove the Backup finalizer only after VGS/VGSC
cleanup verification succeeds. Block Backup deletion while cleanup is
pending.
Add unit and controller integration tests covering finalizer
registration, VGS creation, terminal cleanup, failed-Backup cleanup,
orphan VGSC cleanup, and finalizer removal.
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* add changelog
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* Adress review feedback
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* remove stale VGS finalizer during snapshot restore
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* Improve codecoverage
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* explain VGS member snapshot cleanup timing
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* Second pass at review comment
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* Third pass at review comment
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* Dont swallow get errors
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* Patch backup CR only when annotation is missing
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
---------
Signed-off-by: Prasad Joshi <prajoshi@redhat.com>
* Fix backup-finalizer: do not set backup phase to Completed before PutBackupMetadata succeeds
Previously, the backup finalizer controller set backup.Status.Phase to
Completed/PartiallyFailed in-memory BEFORE calling PutBackupMetadata and
PutBackupContents. When these uploads failed (e.g., due to object lock
or immutability), the deferred patch function still wrote the terminal
phase to the Kubernetes API server, preventing the controller from
retrying the upload on the next reconcile.
This fix moves the phase assignment to AFTER both uploads succeed. A
DeepCopy of the backup is used to encode the JSON with the final phase
for object storage, while the in-memory backup object retains the
Finalizing phase until uploads complete.
Caveats:
- CompletionTimestamp is now captured before upload but only committed to
the API server after upload succeeds. On retry after a transient
failure, a new timestamp is generated, so the completion time reflects
when the upload finally succeeded rather than when finalization
processing completed.
- Metrics (RegisterBackupSuccess/RegisterBackupPartialFailure) are now
recorded after uploads succeed, so they accurately reflect only fully
persisted backups.
- The metadata uploaded to object storage contains the final phase and
completion timestamp via DeepCopy, so storage state is correct even
before the API server is patched.
Fixes#9645
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Add changelog for #9646
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Fix testifylint: use require.Error instead of assert.Error
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Address review feedback on backup-finalizer fix
- Add default guard for unhandled phase values in finalPhase switch
- Add retry with DefaultBackoff for PutBackupMetadata per reviewer request
- Replace brittle framework.BackupItemActionResolverV2{} mock with mock.Anything
- Add FinalizingPartiallyFailed test case for PutBackupContents failure
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Use bounded, object-storage-tuned backoff for backup-finalizer uploads
retry.DefaultBackoff is tuned for API server optimistic-concurrency
conflicts (4 steps, ~1.25s total) and gives up far too quickly for
object storage calls, which can see longer transient outages or
throttling (review feedback from blackpiglet). Replace it with a
dedicated, bounded backoff (1s base, 2x factor, 5 steps, ~31s total)
applied to both PutBackupMetadata and PutBackupContents.
Being bounded (rather than retrying forever) means a persistent
failure, e.g. an object-lock/immutability policy denying every write,
surfaces as an error within a bounded time instead of hanging the
reconcile indefinitely; controller-runtime requeues on error, so
retries continue across reconciles (review feedback from priyansh17).
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Fix PutBackupMetadata retry to re-read backupJSON each attempt
backupJSON is a bytes.Buffer, so passing it directly to
PutBackupMetadata drains it on the first read attempt. A retry after
a transient failure would then upload empty content instead of the
backup metadata. Wrap it in bytes.NewReader(backupJSON.Bytes()) inside
the retry closure so every attempt gets a fresh reader.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
---------
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Happy <yesreply@happy.engineering>
* Modify according to comments.
* Rename the pvSkipTracker and related fields to indicate both PVC and PV are supported.
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
* Add MustIncludeAdditionalItemPVCs structure in backup. It's used to track PVCs returned by BIA with mustIncluded annotaion and PVC is excluded from backup by global filter.
* Modfiy the volumeHelper interface to add a parameter function for ShouldPerformFSBackup.
* Modify to support fine-grained backup filters.
* Modify according to comments. Use a read-only interface to replace the parameter function.
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
Change errors.Cause to errors.Is, because github.com/cockroachdb/errors
New() function create a error with error stack with depth 1, but
github.com/pkg/errors's New() function create error with no depth.
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
Also bumped to support upgraded k8s.io/ deps.
- controller-gen to v0.16.5
- sigs.k8s.io/controller-runtime v0.19.2
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
handleSkippedPVHasRetainPolicy
According to comment, calling executePVAction aims to reset PV's
claimRef, but the reset logic was moved into resetVolumeBindingInfo
since release-1.4.
Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>
Update CSIVolumeSnapshotsCompleted in backup's status and the metric
during backup finalize stage according to async operations content.
Signed-off-by: Xun Jiang <jxun@vmware.com>
Daniel Jiang
Prasad Joshi
Xun Jiang
Xun Jiang/Bruce Jiang
Scott Seago
Xun Jiang
Xun Jiang
Daniel Jiang
Xun Jiang
Lyndon-Li
