Repository navigation
fix(objectstorage-controller): propagate Bucket readiness to BucketClaim - #2792
Conversation
The vendored COSI controller (v0.2.2) sets only BucketClaim.status.bucketName in the dynamic-provisioning path and hardcodes bucketReady=false, never re-reading the Bucket afterwards. The controller does not watch Buckets, so the readiness transition the sidecar performs (false->true) is never reflected on the BucketClaim: bucketReady stays false forever and BucketAccess is never granted, leaving provisioned buckets without credentials. Re-read the live Bucket after create and propagate its readiness, so the periodic resync converges the BucketClaim to ready. This matches the behavior the rewritten upstream controller already has on its main branch. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates the ChangesBucketReady Status Propagation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
Verified the patch against the actual upstream source: it applies cleanly to v0.2.2 on top of the existing 91-bucketaccess-conflict-retry.diff in the Dockerfile's glob order, and the semantics check out — the fresh-create path is unchanged (a just-created Bucket is bucketReady=false anyway), while the AlreadyExists path now picks up the readiness the sidecar set in the meantime. Convergence is fast: the controller runtime's default resync is 30s, and once the claim is ready provisionBucketClaimOperation early-returns, so there's no churn after convergence. The fix also mirrors what upstream main does (claim.Status.ReadyToUse = bucket.Status.ReadyToUse), so the vendored patch won't fight a future controller bump.
|
Successfully created backport PR for |
What this PR does
The vendored COSI
objectstorage-controller(built from upstreamv0.2.2) leaves dynamically-provisioned buckets without credentials.In the dynamic-provisioning path,
provisionBucketClaimOperationsetsBucketClaim.status.bucketNamebut hardcodesbucketReady=falseand never re-reads theBucket. The controller does not watchBucketobjects, so the readiness transition the sidecar performs afterDriverCreateBucket(Bucket.status.bucketReadyfalse→true) is never propagated back to theBucketClaim. Each periodic resync re-enters the same branch (Buckets().CreatereturnsAlreadyExists, which is ignored) and re-setsbucketReady=false. As a resultBucketClaim.status.bucketReadystaysfalseforever, the sidecar refuses to grant theBucketAccess("BucketAccess can't be granted to Bucket not in Ready state"), and the credentials secret is never created — even though the bucket exists in the backend and theBucketCR already reportsbucketReady: true.This adds a vendored patch that re-reads the live
Bucketafter create and propagates its readiness onto theBucketClaim, so the existing periodic resync converges the claim to ready and access can be granted.The fix mirrors what the rewritten upstream controller already does on
main(claim.Status.ReadyToUse = bucket.Status.ReadyToUse). Upstreammainis av1alpha2rewrite and the maintainedrelease-0.2branch still carries the bug, so this stays a vendored patch againstv0.2.2until the controller is bumped to a release that includes the fix.Verified: reproduced the stuck-
falseclaim with a unit test against the upstreamv0.2.2source and confirmed the patch resolves it, existing controller tests pass, the patch applies cleanly tov0.2.2, and the controller image builds.Release note
Summary by CodeRabbit