sync controller: don't delete API objects that are new/in-progress/etc.

[skriss]

backups with .spec.phase of "", New, InProgress should be excluded from the deleteUnused func.

backups with .spec.phase of FailedValidation -- I think it makes sense to exclude them as well. This ensure we don't delete the API obj before a user can see why it failed validation. It will still be GC'ed according to its TTL. We could possibly look at GC'ing backups that failed validation more quickly, but I don't think it's important for now.

backups with .spec.phase of Failed - these should be in object storage (just metadata + logs) -- so the existing code won't delete them, unless the upload of metadata to object storage fails. I'm tempted to exclude these from deleteUnused as well, as it could cause confusion for the user if a backup fails, uploading metadata/logs fail, and then the backup API obj just disappears.

So I think the net result of this is deleteUnused should only process Completed backups.

@carlisia WDYT?

[nrb]

Is the v0.10.0 milestone soon enough? Is it ok to release v0.9.2 without the fix?

[skriss]

this is a fix to carlisia's PR that got merged yesterday

[ncdc]

You'll create a release-0.9 branch based off of v0.9.1 and then get #704 on to it?

[skriss]

release-0.9 already exists (created it in order to do v0.9.1), and yes, will get just #704 onto it to release v0.9.2. Writing changelog up now.

[carlisia]

@skriss this makes sense. I'll add a few test cases that are not BackupPhaseCompleted and add the condition to make them pass.