In GuaranteedUpdate, retry on a precondition check failure if we are working with cached data #82303
Conversation
…rking with cached data
k8s-ci-robot
added
release-note
|
Thanks, test looks great as well. Once this merges, please open backports to release branches as well. /lgtm /milestone v1.16 This addresses an issue that can affect all resources and cause spurious failures of API requests using preconditions under load, and is suitable for backporting to release branches. |
k8s-ci-robot
added
the
priority/critical-urgent
k8s-ci-robot
added
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, roycaihw The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| for { | ||
| if err := preconditions.Check(key, origState.obj); err != nil { | ||
| return err | ||
| // If our data is already up to date, return the error |
There was a problem hiding this comment.
Can the new code be put in a helper func so that code is reused between this and the s.updateState case below ?
There was a problem hiding this comment.
if it doesn't make it harder to follow, that's possible. I wanted to keep this very very simple for backports.
…03-upstream-release-1.15 Automated cherry pick of #82303: in GuaranteedUpdate, retry on precondition check failure if we are working with cached data
…03-upstream-release-1.14 Automated cherry pick of #82303: in GuaranteedUpdate, retry on precondition check failure if we are working with cached data
…03-upstream-release-1.13 Automated cherry pick of #82303: in GuaranteedUpdate, retry on precondition check failure if we are working with cached data
|
/area custom-resources |
liggitt
added
the
area/deflake
4 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
Preconditions should retry internally on stale watch data instead of surfacing an error.
Which issue(s) this PR fixes:
Fixes #82130
Does this PR introduce a user-facing change?:
/sig api-machinery
/cc @liggitt