fix(release-0.2): retry on Bucket update conflict during BucketAccess reconcile#302
Conversation
… reconcile The BucketAccess sidecar reconciler does a Get -> AddFinalizer -> Update on the parent Bucket to attach BABucketFinalizer. When the Bucket reconciler in the same controller process mutates the Bucket between our Get and Update, the Update fails with: Operation cannot be fulfilled on buckets.objectstorage.k8s.io "<name>": the object has been modified; please apply your changes to the latest version and try again surfaced to users as a FailedGrantAccess event on the BucketAccess. Wrap the Bucket finalizer add in retry.RetryOnConflict and re-Get the Bucket inside the closure so each retry mutates the freshest version, matching the pattern adopted in kubernetes-sigs#197 for the BucketClaim controller. Related to kubernetes-sigs#173. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
|
Welcome @myasnikovdaniil! |
|
Hi @myasnikovdaniil. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: myasnikovdaniil The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@myasnikovdaniil please take a look at the CLA requirements added by the bot |
|
/ok-to-test |
What this PR does
Carries the same
RetryOnConflictpattern adopted in #197 (BucketClaim controller) into the BucketAccess sidecar reconciler, where the same race exists on the parent Bucket finalizer-add path.In
sidecar/pkg/bucketaccess/bucketaccess_controller.go, theAddpath does:Between
GetandUpdate, the Bucket reconciler running in the same controller process can mutate the object and bumpresourceVersion, producing:surfaced to users as a
FailedGrantAccessevent on the BucketAccess.This PR wraps the Bucket finalizer add in
retry.RetryOnConflict(retry.DefaultRetry, ...)and re-Gets the Bucket inside the closure so each retry mutates the freshest version. Same pattern as #197.Why release-0.2 only
The same code path does not exist on
main— the v1alpha2 BucketAccess sidecar (sidecar/pkg/reconciler/bucketaccess.go) only reads Buckets and only mutates theBucketAccessitself, so the cross-object Bucket finalizer race is gone architecturally.Related
object has been modifiedinstances in Controller and Sidecar #166 — Open ask to track these conflicts in CIRelease note
Test plan
go build ./...andgo vet ./...passgo test ./sidecar/pkg/bucketaccess/...passesFailedGrantAccess: ... object has been modifiedevent reproducible against v0.2.2 stops firing once the patch is applied.