[eno] Add cross-reconciler deletion ordering fix#571
[eno] Add cross-reconciler deletion ordering fix#571ruinan-liu wants to merge 4 commits intomainfrom
Conversation
| logger.Info("FailOpen - suppressing errors") | ||
| err = nil | ||
| modified = false | ||
| if snap == nil || (!snap.ForegroundDeletion && !resource.HasDeletionGroup()) { |
There was a problem hiding this comment.
If it is ForgroundDeletion or has deletion group, we don't ignore the reconciliation. Does it sound a behavior change?
There was a problem hiding this comment.
Yes, this would be a behavior change.
Our previous fail-open logic was broad and prioritized liveness, making deletion ordering more of a best-effort guarantee. However, ForegroundDeletion and DeletionGroup imply a stricter contract. In those cases, failing open would weaken the intended ordering guarantees.
I don't think we are moving away from deletion faillingOpen globally, but we need to make sure the deletion ordering is respected fully?
| // Don't fail open for resources with deletion ordering contraints - | ||
| // suppressing errors would break deletion group/ foreground deletion ordering | ||
| if failingOpen && snap != nil && (snap.ForegroundDeletion || resource.HasDeletionGroup()) { | ||
| failingOpen = false |
There was a problem hiding this comment.
Better to do it at line 157?
ruinan-liu
left a comment
ruinan-liu
left a comment
There was a problem hiding this comment.
Respond to comments
2 participants
No description provided.