Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In GuaranteedUpdate, retry on a precondition check failure if we are working with cached data #82303

Merged
merged 2 commits into from Sep 4, 2019

Conversation

@roycaihw
Copy link
Member

commented Sep 4, 2019

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?:

Fix a bug in apiserver that could cause a valid update request to be rejected with a precondition check failure.

/sig api-machinery
/cc @liggitt

roycaihw added 2 commits Sep 4, 2019
…rking with cached data
@liggitt

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Thanks, test looks great as well. Once this merges, please open backports to release branches as well.

/lgtm
/approve

/milestone v1.16
/priority critical-urgent

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

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

[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

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 26a381b into kubernetes:master Sep 4, 2019
23 of 24 checks passed
23 of 24 checks passed
tide Not mergeable. Must be in milestone v1.16.
Details
cla/linuxfoundation roycaihw authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-conformance-kind-ipv6 Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
@@ -274,7 +274,20 @@ func (s *store) GuaranteedUpdate(
transformContext := authenticatedDataString(key)
for {
if err := preconditions.Check(key, origState.obj); err != nil {
return err
// If our data is already up to date, return the error

This comment has been minimized.

Copy link
@tedyu

tedyu Sep 4, 2019

Contributor

Can the new code be put in a helper func so that code is reused between this and the s.updateState case below ?

This comment has been minimized.

Copy link
@liggitt

liggitt Sep 4, 2019

Member

if it doesn't make it harder to follow, that's possible. I wanted to keep this very very simple for backports.

k8s-ci-robot added a commit that referenced this pull request Sep 11, 2019
…03-upstream-release-1.15

Automated cherry pick of #82303: in GuaranteedUpdate, retry on precondition check failure if we are working with cached data
k8s-ci-robot added a commit that referenced this pull request Sep 11, 2019
…03-upstream-release-1.14

Automated cherry pick of #82303: in GuaranteedUpdate, retry on precondition check failure if we are working with cached data
k8s-ci-robot added a commit that referenced this pull request Sep 11, 2019
…03-upstream-release-1.13

Automated cherry pick of #82303: in GuaranteedUpdate, retry on precondition check failure if we are working with cached data
@roycaihw

This comment has been minimized.

Copy link
Member Author

commented Sep 17, 2019

/area custom-resources

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.