fix(operator): propagate a backup's merge status to StorageBackup.status

[boddumanohar]

The bug

A StorageBackup stopped being polled the moment the control plane reported it completed, because backupTerminal treated completion as the end of the lifecycle. It is not: retention merges a completed backup into its successor at an arbitrary later point, moving it through merging to merged, and the control plane pushes nothing. The operator learns of the transition only by asking again — so it never did, and neither Merging nor Merged ever reached StorageBackup.status.

Consumers could not tell a merged backup from a restorable one. The e2e retention test had no signal at all and inferred the merge from a shrinking backup count.

Two findings worth flagging, because they change the diagnosis:

1. Count-based inference cannot be made to work. A merged backup is retained in the control plane's database with status merged (tasks_runner_backup.py:360 — its own comment says "remove the old backup", but it only rewrites the status), and list_backups applies no status filter. The count never shrinks, regardless of how many new backups arrive. The CR phase is the only viable signal.
2. backupPhaseFromAPIStatus had no merged case, so it fell through to default and reported Pending. Fixing the polling alone would have turned a completed backup into a Pending one the moment it merged — worse than the silence. Both parts have to land together.

Nothing was needed on the control-plane side: BackupDTO already passes the status through raw (_dtos.py:544), and backupAPIResponse.Status already parses it. The data was arriving; the operator had stopped looking.

The change

• completed is no longer terminal. merged and failed are.
• A completed backup is re-polled on a separate slow interval (backupMergeWatchRequeue, 5m) rather than the ten-second progress one: a merge is not urgent, nothing waits on it the way a caller waits for the backup itself, and every poll costs one cluster-wide backup list.
• merged maps to a new terminal BackupPhaseMerged.
• BackupRestore against a merged backup now terminates. Making Merged reachable made it reachable by BackupRestore, which waited for a Done that can never arrive and polled every ten seconds forever, emitting a warning event each time — the same unbounded-retry shape as the capacity-refusal hang fixed in 900d593. A merged backup no longer exists on its own, and the control plane refuses a chain containing anything but completed backups (backup_controller.py:421), so the restore fails with that reason.

status.phase carries no Enum marker, so the CRDs are unchanged and nothing needed regenerating (make -C operator test produced no drift).

Tests

Written first and run red against the unfixed tree:

storagebackup_controller_unit_test.go:188: expected a completed backup to be polled
    again so a later merge is observed, got RequeueAfter=0s
storagebackup_controller_unit_test.go:252: backend status "merged": expected
    Status.Phase="Merged", got "Pending"
storagebackup_controller_unit_test.go:259: backend status "merged": expected
    polling=false, got RequeueAfter=10s
backuprestore_controller_test.go:403: RequeueAfter = 10s, want 0 (a merged backup
    never becomes restorable)

The merging and failed rows of the table test pass on the unfixed tree and act as guards.

Green after the fix; make -C operator test, -shuffle=on, make -C operator lint (0 issues), and all 9 house style gates pass. New matrix in operator/docs/tests/test-plan-backup-lifecycle.md (BL-01, BL-03, BL-06 are the regression rows).

What this does not do

• The e2e half of the retention issue is still open: the test needs to assert status.phase instead of the count. Not touched here.
• Consolidating the poll. StorageBackupSync already lists every backend backup once per cluster per cycle and skips tracked CRs outright (storagebackupsync_controller.go:118). Extending it to refresh status on tracked CRs would get this at one API call per cluster instead of one per completed backup, and would retire cleanly when SSE lands. Left as a follow-up rather than folded in, since it changes that controller's contract. @maxschettler — this is the cheaper alternative to lifetime pollers.
• Snapshot ownership and the backup deletion semantics (the other two items from the thread) are separate and blocked on the control-plane API modes.

A correction on the pool-deletion premise

While tracing this I checked the claim that "the operator deletes all backups in a pool when the pool is deleted". It does not, and neither does the control plane: handleStoragePoolDeletion only calls the pool DELETE, drops the StorageClass, and clears node labels; there are no owner references on StorageBackup; and pool_controller.delete_pool() never reads Backup rows. Pool deletion is also not blocked by a backup — it counts only lvols, and nothing in sbcli refuses an lvol delete because a snapshot or backup exists.

The data loss in the failing test came from deleting the StorageBackup CR, whose finalizer calls the lvol-scoped endpoint and wipes the whole chain for that volume. Both CRDs are Namespaced, so namespace teardown does the same. That fix needs a per-backup delete endpoint, which sbcli does not have today (/{backup_id} is GET-only) — tracked separately.

🤖 Generated with Claude Code