[Docs] Failed/PartiallyFailed Orphaned backup will not be removed by ObjectStorageSync

[kaovilai]

Describe the problem/challenge you have
Failed/PartiallyFailed backups hang on Kubernetes although removed from bucket

Describe the solution you'd like
Backups that are failed or partially failed should be considered for deletion as well.

velero/pkg/apis/velero/v1/backup.go

Line 233 in d627362

BackupPhasePartiallyFailed BackupPhase = "PartiallyFailed"

velero/pkg/apis/velero/v1/backup.go

Line 237 in d627362

BackupPhaseFailed BackupPhase = "Failed"

Anything else you would like to add:

Line of code that may need amending to implement this feature:

velero/pkg/controller/backup_sync_controller.go

Line 342 in d627362

if backup.Status.Phase != velerov1api.BackupPhaseCompleted || backupStoreBackups.Has(backup.Name) {

Alternatively: Object Storage Sync docs should mention that only completed backups are deleted.

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

• 👍 for "The project would be better with this feature added"
• 👎 for "This feature will not enhance the project in a meaningful way"

cc: @mperetzred @dymurray

[reasonerjt]

For aFailed backup , it usually does not exist in the bucket, so the backup sync controller should probably not removes it as part of syncing the data.

IMO we should update the doc to clarify it.

[kaovilai]

Sounds good to me

[eleanor-millman]

@a-mccarthy Do you have enough to do this work, or do you need engineering input? Thanks!

[sseago]

I think part of the concern here gets at the root of what the goal of cleaning up expired backups. Obviously the most important reason is storage space in the BSL. But a secondary purpose may be cleaning up the list of backup resources in kubernetes. Currently failed backups and backups with invalid BSLs are not cleaned up, but this could lead to months worth of failed and/or inaccessible backups still clogging the list of returned backups when listing in-cluster backups. We need a way for these to be automatically cleaned up, either by default, or via some feature or configuration flag to have them cleared out when they expire.

[eleanor-millman]

@sseago to be clear, are you suggested additional feature work? If so, probably best if you open a separate issue, since this docs issue will be closed once we get the docs updated. But if I misunderstood and you are suggesting different docs updates, then please let me know!

[sseago]

@eleanor-millman Ahh -- I think I was mixing up the doc issue (this one) with the related dev issue that we discussed on the call last week. No, I think on the docs side, documenting current behavior is all that this issue needs. The other issue was where we were discussing possible feature work.