fix(task): replace-pod judges staleness per pod, not by equal StatefulSet revisions

[philipsu522]

Summary

replace-pod short-circuited to complete when sts.Status.CurrentRevision == sts.Status.UpdateRevision, without looking at the pods. On an image rollback that wedged the NodeUpdate plan: the StatefulSet controller resolves a rollback to a revision still in history by pointing UpdateRevision back at it, so CurrentRevision == UpdateRevision while the (OnDelete) pod still runs the abandoned revision. replace-pod completed without deleting anything and observe-image then waited forever for a replacement that was never triggered — recovery required manually deleting pods (seen today on brandon-evmonly-400ms after harbor-engineering-workspace#267 reverted #266).

 	if sts.Status.UpdateRevision == "" {
 		return nil
 	}
-	if sts.Status.CurrentRevision == sts.Status.UpdateRevision {
-		e.complete()
-		return nil
-	}

The per-pod loop already skips pods whose controller-revision-hash equals UpdateRevision, so the already-rolled case is still a no-op (TestReplacePod_AlreadyAtUpdateRevision_NoOp unchanged). Added TestReplacePod_RollbackWithEqualRevisions_DeletesStalePod for the rollback shape (current == update == rev-a, pod at rev-b).

Link to Devin session: https://app.devin.ai/sessions/15fbcb9794c54a48a182b0b29c017d7e
Open in Devin Desktop: https://app.devin.ai/desktop/session/15fbcb9794c54a48a182b0b29c017d7e?variant=devin
Requested by: @philipsu522

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[cursor]

PR Summary

Medium Risk
Changes pod-deletion logic on the image-rollout path and could affect recovery from rollbacks; UID-preconditioned deletes reduce accidental wrong-pod deletion but alter reconcile idempotency behavior.

Overview
replace-pod no longer treats CurrentRevision == UpdateRevision as “rollout done.” Stale pods are decided only in the per-pod loop by comparing controller-revision-hash to UpdateRevision, which fixes rollback cases where both revision fields match a historical rev while the OnDelete pod still runs the abandoned image.

Pod deletion now uses a UID precondition on delete and treats Conflict like NotFound so a stale cache entry cannot delete a recreated pod with the same name.

Adds TestReplacePod_RollbackWithEqualRevisions_DeletesStalePod and updates envtest StatefulSet status faker comments to match the new revision-gate behavior (non-empty UpdateRevision only; envtest pods without revision hashes stay no-ops).

^{Reviewed by Cursor Bugbot for commit 29b3140. Bugbot is set up for automated code reviews on this repo. Configure here.}

[seidroid]

Drops replace-pod's CurrentRevision == UpdateRevision short-circuit so pod staleness is judged per pod against UpdateRevision, fixing the rollback shape where equal revisions hid a pod still running an abandoned revision, and adds a test for it. The logic is correct — the per-pod loop still no-ops the already-rolled case and the recreated pod lands at UpdateRevision under OnDelete, so nothing blocks; codex's diff-only reading reported no findings and contributed none to carry.

Non-blocking

1 finding on the changed lines, as inline comments.

• internal/controller/seinetwork/envtest/sts_status_faker.go documents its updateRevision == currentRevision stub as being "so ReplacePod's 'rollout complete' branch fires immediately" — that branch no longer exists. Behaviour in envtest is unchanged (no pods carry a revision hash there, so the loop completes anyway), but the comment now explains a rationale the code has dropped.

1 nit, not posted on the code

• internal/task/replace_pod.go:73 — The removed short-circuit also sat in front of this guard, so an already-rolled StatefulSet with no selector or replicas > 1 used to complete as a no-op and now returns a Terminal error that fails the plan. Self-healing in practice (the controller re-applies replicas=1), and TestReplacePod_MultiReplica_TerminalError only covers the unequal-revision shape.

_{seidroid review · decision approve · session 682bdab9954749d2bcc3803060767165 · turn resp_claude_e776fd65a912cf0b3bf3d89a825bc899 · item 368a6b6b46fb5040bede24c1da9eb7bd}

Findings: 0 blocking | 2 non-blocking | 1 posted inline

[seidroid]

review found nothing blocking.