Repository navigation
Retry PackageRepository updates on conflict - #1879
Open
NopeCharon wants to merge 1 commit into
Open
NopeCharon wants to merge 1 commit into
NopeCharon wants to merge 1 commit into
Conversation
Adding and removing the PackageRepository deletion finalizer is done by fetching the latest PackageRepository and immediately updating it. If the resource is modified by someone else between those two calls (e.g. a status update or another controller touching it), the API server rejects the update with "the object has been modified; please apply your changes to the latest version and try again". When that happened while removing the finalizer, deletion failed with "Unblocking for deploy: Updating PackageRepository: ..." and the PackageRepository stayed stuck until the next reconcile. App CRs already retry such updates (see pkg/app/crd_app.go). Wrap the PackageRepository get-and-update in retry.RetryOnConflict so that it is refetched and the change is reapplied on conflicts, while other errors are still returned immediately. Add unit tests verifying that the finalizer is removed after transient conflicts and that non-conflict errors are not retried. Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
The PackageRepository reconciler adds and removes its deletion finalizer by fetching the latest PackageRepository and updating it straight away. If something else changes the resource between those two calls, the update is rejected with
the object has been modified; please apply your changes to the latest version and try again. When this happens while removing the finalizer, deletion fails withUnblocking for deploy: Updating PackageRepository: ....App CRs already retry these updates (
pkg/app/crd_app.go, #1766). This PR wraps the PackageRepository get-and-update inretry.RetryOnConflictfrom client-go, which is already used inpkg/apiserver. On a conflict, the PackageRepository is fetched again and the change is reapplied. Other errors are still returned immediately. The error message returned to callers stays the same (Updating PackageRepository: ...).Which issue(s) this PR fixes:
Fixes #417
Does this PR introduce a user-facing change?
Additional Notes for your reviewer:
Added
pkg/pkgrepository/crd_app_test.gowith two tests:TestUnblockDeletionRetriesOnConflict: the first two updates return a conflict. The test checks that the finalizers are removed after the third attempt. It fails without the fix.TestUnblockDeletionDoesNotRetryOnNonConflictError: a non-conflict error is returned after one attempt.Review Checklist:
a link to that PR
change
Additional documentation e.g., Proposal, usage docs, etc.: